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.selva...@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.selva...@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" } } */

Reply via email to