This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[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" } } */


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]