[Bug target/69946] [6 Regression] Invalid ppc64 assembly emitted starting with r226005
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69946 Jakub Jelinek changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED --- Comment #4 from Jakub Jelinek --- Fixed.
[Bug target/69946] [6 Regression] Invalid ppc64 assembly emitted starting with r226005
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69946 --- Comment #3 from Segher Boessenkool --- Author: segher Date: Fri Feb 26 18:49:18 2016 New Revision: 233755 URL: https://gcc.gnu.org/viewcvs?rev=233755=gcc=rev Log: powerpc: Handle DImode rotatert implemented with rlwinm (PR69946) Some DImode rotate-right-and-mask can be implemented best with a rlwinm instruction: those that could be a lshiftrt instead of a rotatert, while the mask is not right-aligned. Why the rotate in the testcase is not optimised to a plain shift is another question, but we need to handle it here anyway. We compute the shift amount for a 64-bit rotate. This is 32 too high in this case; if we print using %h that is masked out (and this doesn't silently let through invalid instructions, everything is checked by rs6000_is_valid_shift_mask which is much more thorough). PR target/69946 * config/rs6000/rs6000.c (rs6000_insn_for_shift_mask): Print rlwinm shift amount using %h. Add comment. gcc/testsuite/ * gcc.target/powerpc/pr69946.c: New file. Added: trunk/gcc/testsuite/gcc.target/powerpc/pr69946.c Modified: trunk/gcc/ChangeLog trunk/gcc/config/rs6000/rs6000.c trunk/gcc/testsuite/ChangeLog
[Bug target/69946] [6 Regression] Invalid ppc64 assembly emitted starting with r226005
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69946 --- Comment #2 from Jakub Jelinek --- Created attachment 37803 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=37803=edit gcc6-pr69946.patch What about this untested patch? Ideas for better name of the new helper function? Seems that both rs6000_is_valid_shift_mask and rs6000_insn_for_shift_mask were doing pretty much the same thing in their first halves (preparation), except that for insert we assume that the shift count is always CONST_INT. So, this patch moves all that preparation into the helper, and uses it in all the 4 functions.
[Bug target/69946] [6 Regression] Invalid ppc64 assembly emitted starting with r226005
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69946 Segher Boessenkool changed: What|Removed |Added Status|NEW |ASSIGNED Assignee|unassigned at gcc dot gnu.org |segher at gcc dot gnu.org
[Bug target/69946] [6 Regression] Invalid ppc64 assembly emitted starting with r226005
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69946
--- Comment #1 from Jakub Jelinek ---
To me this looks like the bug is in the analysis function not matching what the
output function does.
rs6000_is_valid_shift_mask
has code like:
17346 /* Convert any shift by 0 to a rotate, to simplify below code. */
17347 if (sh == 0)
17348 code = ROTATE;
17349
17350 /* Convert rotate to simple shift if we can, to make analysis
simpler. */
17351 if (code == ROTATE && sh >= 0 && nb >= ne && ne >= sh)
17352 code = ASHIFT;
17353 if (code == ROTATE && sh >= 0 && nb >= ne && nb < sh)
17354 {
17355 code = LSHIFTRT;
17356 sh = n - sh;
17357 }
but rs6000_insn_for_shift_mask doesn't do any of those transformations (neither
shifts to ROTATE, nor ROTATE to ASHIFT, nor ROTATE to LSHIFTRT.
The last one is what triggers on the testcase.
If I try to match what the analysis function does more closely:
--- gcc/config/rs6000/rs6000.c.jj 2016-02-12 00:50:55.0 +0100
+++ gcc/config/rs6000/rs6000.c 2016-02-24 18:55:19.945977868 +0100
@@ -17403,9 +17403,32 @@ rs6000_insn_for_shift_mask (machine_mode
if (!rs6000_is_valid_mask (operands[3], , , mode))
gcc_unreachable ();
+ int n = GET_MODE_PRECISION (mode);
+ int sh = -1;
+
+ if (CONST_INT_P (operands[2]))
+sh = INTVAL (operands[2]);
+
+ rtx_code code = GET_CODE (operands[4]);
+
+ /* Convert any shift by 0 to a rotate, to match what the analysis
+ code did. */
+ if (sh == 0)
+code = ROTATE;
+
+ /* Convert rotate to simple shift if we can. */
+ if (code == ROTATE && sh >= 0 && nb >= ne && ne >= sh)
+code = ASHIFT;
+ if (code == ROTATE && sh >= 0 && nb >= ne && nb < sh)
+{
+ code = LSHIFTRT;
+ sh = n - sh;
+ operands[2] = GEN_INT (sh);
+}
+
if (mode == DImode && ne == 0)
{
- if (GET_CODE (operands[4]) == LSHIFTRT && INTVAL (operands[2]))
+ if (code == LSHIFTRT && INTVAL (operands[2]))
operands[2] = GEN_INT (64 - INTVAL (operands[2]));
operands[3] = GEN_INT (63 - nb);
if (dot)
@@ -17422,7 +17445,7 @@ rs6000_insn_for_shift_mask (machine_mode
}
if (mode == DImode
- && GET_CODE (operands[4]) != LSHIFTRT
+ && code != LSHIFTRT
&& CONST_INT_P (operands[2])
&& ne == INTVAL (operands[2]))
{
@@ -17434,7 +17457,7 @@ rs6000_insn_for_shift_mask (machine_mode
if (nb < 32 && ne < 32)
{
- if (GET_CODE (operands[4]) == LSHIFTRT && INTVAL (operands[2]))
+ if (code == LSHIFTRT && INTVAL (operands[2]))
operands[2] = GEN_INT (32 - INTVAL (operands[2]));
operands[3] = GEN_INT (31 - nb);
operands[4] = GEN_INT (31 - ne);
then the change in generated assembly is:
- rlwinm 3,3,44,20,23
+ rlwinm 3,3,12,20,23
which I bet is correct. If this is the way to go, then
rs6000_insn_for_insert_mask has similar problem (at least, doesn't match the
analysis function, whether it actually makes any difference for that or not is
unclear; but for this one we have a proof).
[Bug target/69946] [6 Regression] Invalid ppc64 assembly emitted starting with r226005
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69946 Jakub Jelinek changed: What|Removed |Added Priority|P3 |P1 Status|UNCONFIRMED |NEW Last reconfirmed||2016-02-24 CC||dje at gcc dot gnu.org, ||segher at gcc dot gnu.org Target Milestone|--- |6.0 Ever confirmed|0 |1
