[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&root=gcc&view=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&action=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], &nb, &ne, 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