[AARCH64 PATCH] Fix shift+rotate patterns with masking (PR target/84845)

Jakub Jelinek jakub@redhat.com
Wed Mar 14 08:44:00 GMT 2018


Hi!

The following testcase ICEs on aarch64-linux, because combiner matches the
(insn 25 24 26 2 (set (reg:DI 114)
        (rotatert:DI (reg:DI 115 [ d ])
            (subreg:QI (neg:SI (and:SI (reg:SI 112)
                        (const_int 65535 [0xffff]))) 0))) "pr84845.c":8 664 {*aarch64_reg_di3_neg_mask2}
     (expr_list:REG_DEAD (reg:SI 112)
        (expr_list:REG_DEAD (reg:DI 115 [ d ])
            (nil))))
pattern, but what we split it into doesn't recog, because there are just
patterns that use the same mode for the shift/rotate as for AND and use
proper
                     (match_operand 3 "const_int_operand" "n"))])))]
  "(~INTVAL (operands[3]) & (GET_MODE_BITSIZE (<MODE>mode) - 1)) == 0"
and then there is another pattern for the mixed DImode shift/rotate and
SImode mask, but that one uses for the mask instead
                     (match_operand 3 "aarch64_shift_imm_di" "Usd"))])))]
  "((~INTVAL (operands[3]) & (GET_MODE_BITSIZE (DImode)-1)) == 0)"
which due to the predicate (and constraint) never matches.

The following patch fixes that bug (uses "const_int_operand" "n" like in
similar patterns), also fixes another bug in the *aarch64_reg_<mode>3_neg_mask2
splitter, where if it isn't split before reload it would emit invalid
negation (SImode negation from SImode input into DImode output).

And the last change is that I find these patterns to be misnamed, the most
important on these patterns is that they are shifts/rotates, but the names
of the pattern don't indicate that at all; some other patterns around do
indicate that with <optab>_ part, which the patch changes them to, and
one pattern has that part in inconsistent spot to the others.

Bootstrapped/regtested on aarch64-linux, ok for trunk?

2018-03-14  Jakub Jelinek  <jakub@redhat.com>

	PR target/84845
	* config/aarch64/aarch64.md (*aarch64_reg_<mode>3_neg_mask2): Rename
	to ...
	(*aarch64_<optab>_reg_<mode>3_neg_mask2): ... this.  If pseudos can't
	be created, use lowpart_subreg of operands[0] rather than operands[0]
	itself.
	(*aarch64_reg_<mode>3_minus_mask): Rename to ...
	(*aarch64_ashl_reg_<mode>3_minus_mask): ... this.
	(*aarch64_<optab>_reg_di3_mask2): Use const_int_operand predicate
	and n constraint instead of aarch64_shift_imm_di and Usd.
	(*aarch64_reg_<optab>_minus<mode>3): Rename to ...
	(*aarch64_<optab>_reg_minus<mode>3): ... this.

	* gcc.c-torture/compile/pr84845.c: New test.

--- gcc/config/aarch64/aarch64.md.jj	2018-03-13 00:38:26.000000000 +0100
+++ gcc/config/aarch64/aarch64.md	2018-03-13 18:33:47.657719021 +0100
@@ -4262,7 +4262,7 @@ (define_insn "*aarch64_<optab>_reg_<mode
   [(set_attr "type" "shift_reg")]
 )
 
-(define_insn_and_split "*aarch64_reg_<mode>3_neg_mask2"
+(define_insn_and_split "*aarch64_<optab>_reg_<mode>3_neg_mask2"
   [(set (match_operand:GPI 0 "register_operand" "=&r")
 	(SHIFT:GPI
 	  (match_operand:GPI 1 "register_operand" "r")
@@ -4275,7 +4275,7 @@ (define_insn_and_split "*aarch64_reg_<mo
   [(const_int 0)]
   {
     rtx tmp = (can_create_pseudo_p () ? gen_reg_rtx (SImode)
-	       : operands[0]);
+	       : lowpart_subreg (SImode, operands[0], <MODE>mode));
     emit_insn (gen_negsi2 (tmp, operands[2]));
 
     rtx and_op = gen_rtx_AND (SImode, tmp, operands[3]);
@@ -4286,7 +4286,7 @@ (define_insn_and_split "*aarch64_reg_<mo
   }
 )
 
-(define_insn_and_split "*aarch64_reg_<mode>3_minus_mask"
+(define_insn_and_split "*aarch64_ashl_reg_<mode>3_minus_mask"
   [(set (match_operand:GPI 0 "register_operand" "=&r")
 	(ashift:GPI
 	  (match_operand:GPI 1 "register_operand" "r")
@@ -4320,8 +4320,8 @@ (define_insn "*aarch64_<optab>_reg_di3_m
 	  (match_operand:DI 1 "register_operand" "r")
 	  (match_operator 4 "subreg_lowpart_operator"
 	   [(and:SI (match_operand:SI 2 "register_operand" "r")
-		     (match_operand 3 "aarch64_shift_imm_di" "Usd"))])))]
-  "((~INTVAL (operands[3]) & (GET_MODE_BITSIZE (DImode)-1)) == 0)"
+		    (match_operand 3 "const_int_operand" "n"))])))]
+  "((~INTVAL (operands[3]) & (GET_MODE_BITSIZE (DImode) - 1)) == 0)"
 {
   rtx xop[3];
   xop[0] = operands[0];
@@ -4333,7 +4333,7 @@ (define_insn "*aarch64_<optab>_reg_di3_m
   [(set_attr "type" "shift_reg")]
 )
 
-(define_insn_and_split "*aarch64_reg_<optab>_minus<mode>3"
+(define_insn_and_split "*aarch64_<optab>_reg_minus<mode>3"
   [(set (match_operand:GPI 0 "register_operand" "=&r")
 	(ASHIFT:GPI
 	  (match_operand:GPI 1 "register_operand" "r")
--- gcc/testsuite/gcc.c-torture/compile/pr84845.c.jj	2018-03-13 18:42:46.841004726 +0100
+++ gcc/testsuite/gcc.c-torture/compile/pr84845.c	2018-03-13 18:42:16.491988632 +0100
@@ -0,0 +1,12 @@
+/* PR target/84845 */
+
+int a, b, c;
+unsigned long d;
+
+void
+foo (void)
+{
+  b = -1;
+  b <<= c >= 0;
+  d = d << (63 & (short)-b) | d >> (63 & -(short)-b);
+}

	Jakub



More information about the Gcc-patches mailing list