| Message ID | YrF/itA0wivs3iXh@tucnak |
|---|---|
| State | New |
| Headers | show |
| Series | ifcvt: Don't introduce trapping or faulting reads in noce_try_sign_mask [PR106032] | expand |
On Tue, 21 Jun 2022, Jakub Jelinek wrote: > Hi! > > noce_try_sign_mask as documented will optimize > if (c < 0) > x = t; > else > x = 0; > into x = (c >> bitsm1) & t; > The optimization is done if either t is unconditional > (e.g. for > x = t; > if (c >= 0) > x = 0; > ) or if it is cheap. We already check that t doesn't have side-effects, > but if t is conditional, we need to punt also if it may trap or fault, > as we make it unconditional. > > I've briefly skimmed other noce_try* optimizations and didn't find one that > would suffer from the same problem. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? OK. Thanks, Richard. > 2022-06-21 Jakub Jelinek <jakub@redhat.com> > > PR rtl-optimization/106032 > * ifcvt.cc (noce_try_sign_mask): Punt if !t_unconditional, and > t may_trap_or_fault_p, even if it is cheap. > > * gcc.c-torture/execute/pr106032.c: New test. > > --- gcc/ifcvt.cc.jj 2022-04-26 10:11:51.951558338 +0200 > +++ gcc/ifcvt.cc 2022-06-20 17:44:18.638394338 +0200 > @@ -2833,18 +2833,19 @@ noce_try_sign_mask (struct noce_if_info > return FALSE; > > /* This is only profitable if T is unconditionally executed/evaluated in the > - original insn sequence or T is cheap. The former happens if B is the > - non-zero (T) value and if INSN_B was taken from TEST_BB, or there was no > - INSN_B which can happen for e.g. conditional stores to memory. For the > - cost computation use the block TEST_BB where the evaluation will end up > - after the transformation. */ > + original insn sequence or T is cheap and can't trap or fault. The former > + happens if B is the non-zero (T) value and if INSN_B was taken from > + TEST_BB, or there was no INSN_B which can happen for e.g. conditional > + stores to memory. For the cost computation use the block TEST_BB where > + the evaluation will end up after the transformation. */ > t_unconditional > = (t == if_info->b > && (if_info->insn_b == NULL_RTX > || BLOCK_FOR_INSN (if_info->insn_b) == if_info->test_bb)); > if (!(t_unconditional > - || (set_src_cost (t, mode, if_info->speed_p) > - < COSTS_N_INSNS (2)))) > + || ((set_src_cost (t, mode, if_info->speed_p) > + < COSTS_N_INSNS (2)) > + && !may_trap_or_fault_p (t)))) > return FALSE; > > if (!noce_can_force_operand (t)) > --- gcc/testsuite/gcc.c-torture/execute/pr106032.c.jj 2022-06-20 18:00:01.064352904 +0200 > +++ gcc/testsuite/gcc.c-torture/execute/pr106032.c 2022-06-20 17:59:41.714600349 +0200 > @@ -0,0 +1,21 @@ > +/* PR rtl-optimization/106032 */ > + > +__attribute__((noipa)) int > +foo (int x, int *y) > +{ > + int a = 0; > + if (x < 0) > + a = *y; > + return a; > +} > + > +int > +main () > +{ > + int a = 42; > + if (foo (0, 0) != 0 || foo (1, 0) != 0) > + __builtin_abort (); > + if (foo (-1, &a) != 42 || foo (-42, &a) != 42) > + __builtin_abort (); > + return 0; > +} > > Jakub > >
--- gcc/ifcvt.cc.jj 2022-04-26 10:11:51.951558338 +0200 +++ gcc/ifcvt.cc 2022-06-20 17:44:18.638394338 +0200 @@ -2833,18 +2833,19 @@ noce_try_sign_mask (struct noce_if_info return FALSE; /* This is only profitable if T is unconditionally executed/evaluated in the - original insn sequence or T is cheap. The former happens if B is the - non-zero (T) value and if INSN_B was taken from TEST_BB, or there was no - INSN_B which can happen for e.g. conditional stores to memory. For the - cost computation use the block TEST_BB where the evaluation will end up - after the transformation. */ + original insn sequence or T is cheap and can't trap or fault. The former + happens if B is the non-zero (T) value and if INSN_B was taken from + TEST_BB, or there was no INSN_B which can happen for e.g. conditional + stores to memory. For the cost computation use the block TEST_BB where + the evaluation will end up after the transformation. */ t_unconditional = (t == if_info->b && (if_info->insn_b == NULL_RTX || BLOCK_FOR_INSN (if_info->insn_b) == if_info->test_bb)); if (!(t_unconditional - || (set_src_cost (t, mode, if_info->speed_p) - < COSTS_N_INSNS (2)))) + || ((set_src_cost (t, mode, if_info->speed_p) + < COSTS_N_INSNS (2)) + && !may_trap_or_fault_p (t)))) return FALSE; if (!noce_can_force_operand (t)) --- gcc/testsuite/gcc.c-torture/execute/pr106032.c.jj 2022-06-20 18:00:01.064352904 +0200 +++ gcc/testsuite/gcc.c-torture/execute/pr106032.c 2022-06-20 17:59:41.714600349 +0200 @@ -0,0 +1,21 @@ +/* PR rtl-optimization/106032 */ + +__attribute__((noipa)) int +foo (int x, int *y) +{ + int a = 0; + if (x < 0) + a = *y; + return a; +} + +int +main () +{ + int a = 42; + if (foo (0, 0) != 0 || foo (1, 0) != 0) + __builtin_abort (); + if (foo (-1, &a) != 42 || foo (-42, &a) != 42) + __builtin_abort (); + return 0; +}
Hi! noce_try_sign_mask as documented will optimize if (c < 0) x = t; else x = 0; into x = (c >> bitsm1) & t; The optimization is done if either t is unconditional (e.g. for x = t; if (c >= 0) x = 0; ) or if it is cheap. We already check that t doesn't have side-effects, but if t is conditional, we need to punt also if it may trap or fault, as we make it unconditional. I've briefly skimmed other noce_try* optimizations and didn't find one that would suffer from the same problem. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2022-06-21 Jakub Jelinek <jakub@redhat.com> PR rtl-optimization/106032 * ifcvt.cc (noce_try_sign_mask): Punt if !t_unconditional, and t may_trap_or_fault_p, even if it is cheap. * gcc.c-torture/execute/pr106032.c: New test. Jakub