Hello!
> > > [ I see i386 also does PUT_CODE in a few more splitters, hrm. ]
> >
> > I think the splitters need to be fixed but I also think that
> > shallow_copy_rtx should
> > be enough, no? combine will not actually end up generating more references
> > to the original RTX so we won't end up with invalid RTX sharing (the same
> > logic
> > is why the splitters didn't care).
>
> That probably is enough; I haven't thought about it too much, all other
> splitters doing a copy do a deep copy. I don't think it is worth being
> too tricky here, those splitters do not match often at all.
>
> > It would be cleaner to write the splitters in a way that doesnt copy the RTX
> > but creates a new RTX with proper code/mode and operands (but that's more
> > work on your side - maybe Uros wants to help here).
>
> I think people use "copy_rtx; PUT_*" because it is less typing work ;-)
> If that is the only thing you change about the rtx, it even is easier
> to understand than creating a new one.
Attached patch implements both suggestions. It uses shallow_copy_rtx
before all PUT_* calls in the splitters, to avoid nasty surprises like
this one in the future. shallow_ropy_rtx copies just one level of RTX,
and this is all what we need. Also, the patch changes existing uses of
copy_rtx to shallow_copy_rtx.
2015-06-26 Uros Bizjak <ubiz...@gmail.com>
Segher Boessenkool <seg...@kernel.crashing.org>
* config/i386/i386.md (various splitters): Use shallow_copy_rtx
before doing PUT_MODE or PUT_CODE on operands to avoid
in-place RTX modification.
testsuite/ChangeLog:
2015-06-26 Uros Bizjak <ubiz...@gmail.com>
* gcc.target/i386/pr66412.c: New test.
Patch was bootstrapped and regresison tested on x85_64-linux-gnu
{,-m32}. Patch was committed to mainline SVN and will be backported to
all release branches (once they open).
Uros.
Index: config/i386/i386.md
===================================================================
--- config/i386/i386.md (revision 224993)
+++ config/i386/i386.md (working copy)
@@ -10796,6 +10796,7 @@
[(set (match_dup 2) (match_dup 1))
(set (match_dup 0) (zero_extend:DI (match_dup 2)))]
{
+ operands[1] = shallow_copy_rtx (operands[1]);
PUT_MODE (operands[1], QImode);
operands[2] = gen_lowpart (QImode, operands[0]);
})
@@ -10813,6 +10814,7 @@
(parallel [(set (match_dup 0) (zero_extend:SI (match_dup 2)))
(clobber (reg:CC FLAGS_REG))])]
{
+ operands[1] = shallow_copy_rtx (operands[1]);
PUT_MODE (operands[1], QImode);
operands[2] = gen_lowpart (QImode, operands[0]);
})
@@ -10828,6 +10830,7 @@
[(set (match_dup 2) (match_dup 1))
(set (match_dup 0) (zero_extend:SI (match_dup 2)))]
{
+ operands[1] = shallow_copy_rtx (operands[1]);
PUT_MODE (operands[1], QImode);
operands[2] = gen_lowpart (QImode, operands[0]);
})
@@ -10865,7 +10868,10 @@
(const_int 0)))]
""
[(set (match_dup 0) (match_dup 1))]
- "PUT_MODE (operands[1], QImode);")
+{
+ operands[1] = shallow_copy_rtx (operands[1]);
+ PUT_MODE (operands[1], QImode);
+})
(define_split
[(set (strict_low_part (match_operand:QI 0 "nonimmediate_operand"))
@@ -10874,7 +10880,10 @@
(const_int 0)))]
""
[(set (match_dup 0) (match_dup 1))]
- "PUT_MODE (operands[1], QImode);")
+{
+ operands[1] = shallow_copy_rtx (operands[1]);
+ PUT_MODE (operands[1], QImode);
+})
(define_split
[(set (match_operand:QI 0 "nonimmediate_operand")
@@ -10884,15 +10893,15 @@
""
[(set (match_dup 0) (match_dup 1))]
{
- rtx new_op1 = copy_rtx (operands[1]);
- operands[1] = new_op1;
- PUT_MODE (new_op1, QImode);
- PUT_CODE (new_op1, ix86_reverse_condition (GET_CODE (new_op1),
- GET_MODE (XEXP (new_op1, 0))));
+ operands[1] = shallow_copy_rtx (operands[1]);
+ PUT_MODE (operands[1], QImode);
+ PUT_CODE (operands[1],
+ ix86_reverse_condition (GET_CODE (operands[1]),
+ GET_MODE (XEXP (operands[1], 0))));
/* Make sure that (a) the CCmode we have for the flags is strong
enough for the reversed compare or (b) we have a valid FP compare. */
- if (! ix86_comparison_operator (new_op1, VOIDmode))
+ if (! ix86_comparison_operator (operands[1], VOIDmode))
FAIL;
})
@@ -10904,15 +10913,15 @@
""
[(set (match_dup 0) (match_dup 1))]
{
- rtx new_op1 = copy_rtx (operands[1]);
- operands[1] = new_op1;
- PUT_MODE (new_op1, QImode);
- PUT_CODE (new_op1, ix86_reverse_condition (GET_CODE (new_op1),
- GET_MODE (XEXP (new_op1, 0))));
+ operands[1] = shallow_copy_rtx (operands[1]);
+ PUT_MODE (operands[1], QImode);
+ PUT_CODE (operands[1],
+ ix86_reverse_condition (GET_CODE (operands[1]),
+ GET_MODE (XEXP (operands[1], 0))));
/* Make sure that (a) the CCmode we have for the flags is strong
enough for the reversed compare or (b) we have a valid FP compare. */
- if (! ix86_comparison_operator (new_op1, VOIDmode))
+ if (! ix86_comparison_operator (operands[1], VOIDmode))
FAIL;
})
@@ -11031,7 +11040,10 @@
(if_then_else (match_dup 0)
(label_ref (match_dup 1))
(pc)))]
- "PUT_MODE (operands[0], VOIDmode);")
+{
+ operands[0] = shallow_copy_rtx (operands[0]);
+ PUT_MODE (operands[0], VOIDmode);
+})
(define_split
[(set (pc)
@@ -11046,15 +11058,15 @@
(label_ref (match_dup 1))
(pc)))]
{
- rtx new_op0 = copy_rtx (operands[0]);
- operands[0] = new_op0;
- PUT_MODE (new_op0, VOIDmode);
- PUT_CODE (new_op0, ix86_reverse_condition (GET_CODE (new_op0),
- GET_MODE (XEXP (new_op0, 0))));
+ operands[0] = shallow_copy_rtx (operands[0]);
+ PUT_MODE (operands[0], VOIDmode);
+ PUT_CODE (operands[0],
+ ix86_reverse_condition (GET_CODE (operands[0]),
+ GET_MODE (XEXP (operands[0], 0))));
/* Make sure that (a) the CCmode we have for the flags is strong
enough for the reversed compare or (b) we have a valid FP compare. */
- if (! ix86_comparison_operator (new_op0, VOIDmode))
+ if (! ix86_comparison_operator (operands[0], VOIDmode))
FAIL;
})
@@ -11091,7 +11103,7 @@
(pc)))]
{
operands[2] = simplify_gen_subreg (<MODE>mode, operands[2], QImode, 0);
-
+ operands[0] = shallow_copy_rtx (operands[0]);
PUT_CODE (operands[0], reverse_condition (GET_CODE (operands[0])));
})
@@ -11124,7 +11136,7 @@
(pc)))]
{
operands[2] = simplify_gen_subreg (<MODE>mode, operands[2], SImode, 0);
-
+ operands[0] = shallow_copy_rtx (operands[0]);
PUT_CODE (operands[0], reverse_condition (GET_CODE (operands[0])));
})
@@ -11160,7 +11172,7 @@
(pc)))]
{
operands[2] = simplify_gen_subreg (<MODE>mode, operands[2], SImode, 0);
-
+ operands[0] = shallow_copy_rtx (operands[0]);
PUT_CODE (operands[0], reverse_condition (GET_CODE (operands[0])));
})
@@ -11192,7 +11204,7 @@
(pc)))]
{
operands[2] = simplify_gen_subreg (SImode, operands[2], QImode, 0);
-
+ operands[0] = shallow_copy_rtx (operands[0]);
PUT_CODE (operands[0], reverse_condition (GET_CODE (operands[0])));
})
@@ -11228,7 +11240,10 @@
(if_then_else (match_op_dup 0 [(reg:CCC FLAGS_REG) (const_int 0)])
(label_ref (match_dup 4))
(pc)))]
- "PUT_CODE (operands[0], reverse_condition (GET_CODE (operands[0])));")
+{
+ operands[0] = shallow_copy_rtx (operands[0]);
+ PUT_CODE (operands[0], reverse_condition (GET_CODE (operands[0])));
+})
;; Define combination compare-and-branch fp compare instructions to help
;; combine.
@@ -17298,6 +17313,7 @@
operands[1] = gen_lowpart (SImode, operands[1]);
if (GET_CODE (operands[3]) != ASHIFT)
operands[2] = gen_lowpart (SImode, operands[2]);
+ operands[3] = shallow_copy_rtx (operands[3]);
PUT_MODE (operands[3], SImode);
})
Index: testsuite/gcc.target/i386/pr66412.c
===================================================================
--- testsuite/gcc.target/i386/pr66412.c (revision 0)
+++ testsuite/gcc.target/i386/pr66412.c (working copy)
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -g" } */
+
+int a, b, c, d;
+
+void
+fn1 ()
+{
+ short e;
+ unsigned short g;
+
+ for (c = 0; c < 1; c++)
+ d = 0;
+ g = ((a == 0) ^ d) % 8;
+ e = g << 1;
+ b = e && 1;
+}
