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