This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[Patch, PR 88253, combine] Prevent combine simplification to zero if side_effects_p
- From: <SenthilKumar dot Selvaraj at microchip dot com>
- To: <gcc-patches at gcc dot gnu dot org>
- Date: Fri, 14 Dec 2018 09:21:26 +0000
- Subject: [Patch, PR 88253, combine] Prevent combine simplification to zero if side_effects_p
Hi,
This patch fixes PR 88253 by preventing combine_simplify_rtx from
folding an rtx with side_effects_p to const0_rtx.
Without the patch, combine deletes volatile loads from memory in certain
cases. When combine_simplify_rtx is called with op0_mode = E_HImode and
rtx like
(subreg:QI (ashift:HI (zero_extend:HI (mem/v:QI (reg/f:HI 53) [0 MEM[(volatile uint8_t *)121B]+0 S1 A8]))
(const_int 8 [0x8])) 0)
it simplifies it to (const_int 0) because it figures out that none of
the bits set by x are nonzero in the insn's mode (ie
nonzero_bits(SUBREG_REG(x)) & mode-mask-for-QI) is zero).
This ends up removing the volatile load too, causing PR 882253, where
the load is on an SFR register and has side effects.
I found that force_to_mode has similar code to fold to const0_rtx, but was
modified in https://gcc.gnu.org/r116827 to do the fold only if !side_effects_p.
--- trunk/gcc/combine.c 2006/05/23 15:07:00 114019
+++ trunk/gcc/combine.c 2006/09/10 21:27:36 116827
@@ -6860,7 +6860,7 @@
nonzero = nonzero_bits (x, mode);
/* If none of the bits in X are needed, return a zero. */
- if (! just_select && (nonzero & mask) == 0)
+ if (!just_select && (nonzero & mask) == 0 && !side_effects_p (x))
x = const0_rtx;
This patch does the same to combine_simplify_rtx.
Reg test passed for the avr target (for which PR 88253 was filed). Will
run for x86_64 as well, if the patch looks ok.
Does this look good?
Regards
Senthil
gcc/ChangeLog:
2018-12-14 Senthil Kumar Selvaraj <senthilkumar.selvaraj@microchip.com>
PR 88253.c
* combine.c (combine_simplify_rtx): Test for side-effects before
substituting by zero.
gcc/testsuite/ChangeLog:
2018-12-14 Senthil Kumar Selvaraj <senthilkumar.selvaraj@microchip.com>
* gcc.target/avr/pr88253.c: New test.
diff --git gcc/combine.c gcc/combine.c
index 7e611399f2c..e634e8f5a7e 100644
--- gcc/combine.c
+++ gcc/combine.c
@@ -5978,6 +5978,7 @@ combine_simplify_rtx (rtx x, machine_mode op0_mode, int in_dest,
&& known_eq (subreg_lowpart_offset (int_mode, int_op0_mode),
SUBREG_BYTE (x))
&& HWI_COMPUTABLE_MODE_P (int_op0_mode)
+ && !side_effects_p (SUBREG_REG (x))
&& (nonzero_bits (SUBREG_REG (x), int_op0_mode)
& GET_MODE_MASK (int_mode)) == 0)
return CONST0_RTX (int_mode);
diff --git gcc/testsuite/gcc.target/avr/pr88253.c gcc/testsuite/gcc.target/avr/pr88253.c
new file mode 100644
index 00000000000..7fa7e4efc69
--- /dev/null
+++ gcc/testsuite/gcc.target/avr/pr88253.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+/* { dg-options "-Os -w" } */
+
+static int aRead() __attribute__((always_inline));
+static int aRead() {
+ unsigned char h,l;
+ l = (*(volatile unsigned char *)(0x78)) ;
+ h = (*(volatile unsigned char *)(0x79)) ;
+ return (h<<8) | l;
+}
+
+int main() {
+ volatile unsigned char x;
+ x = aRead()^42;
+ }
+ /* { dg-final { scan-assembler "lds r\\d+,121" } } */