Message ID | 20211105101945.42071-1-krebbel@linux.ibm.com |
---|---|
State | New |
Headers | show |
Series | Fix PR103028 | expand |
On 11/5/2021 4:19 AM, Andreas Krebbel via Gcc-patches wrote: > This prevents find_cond_trap from being invoked after reload. It may > generate compares which would require reloading. > > Bootstrapped and regression tested on s390x. > > Ok for mainline? > > gcc/ChangeLog: > > PR rtl-optimization/103028 > * ifcvt.c (find_if_header): Invoke find_cond_trap only before > reload. > > gcc/testsuite/ChangeLog: > > PR rtl-optimization/103028 > * gcc.dg/pr103028.c: New test. Shouldn't this be handled by the target by rejecting creating the trap after reload has completed since the target seems to need new pseudos to generate a conditional trap? Otherwise we're penalizing targets which don't need new pseudos to generate conditional traps. jeff
On 11/5/21 20:34, Jeff Law wrote: > > > On 11/5/2021 4:19 AM, Andreas Krebbel via Gcc-patches wrote: >> This prevents find_cond_trap from being invoked after reload. It may >> generate compares which would require reloading. >> >> Bootstrapped and regression tested on s390x. >> >> Ok for mainline? >> >> gcc/ChangeLog: >> >> PR rtl-optimization/103028 >> * ifcvt.c (find_if_header): Invoke find_cond_trap only before >> reload. >> >> gcc/testsuite/ChangeLog: >> >> PR rtl-optimization/103028 >> * gcc.dg/pr103028.c: New test. > Shouldn't this be handled by the target by rejecting creating the trap > after reload has completed since the target seems to need new pseudos to > generate a conditional trap? Otherwise we're penalizing targets which > don't need new pseudos to generate conditional traps. In this case we do not explicitely create a new pseudo. It is rather that we emit a pattern which would need to be handled be reload. I think passes which run after reload are not allowed to emit patterns which would require reloading and it cannot be up to the backend to prevent this. Instead of disabling this path after reload we could also try to check all the to be emitted insns with constrain_operands to make sure at least one of the alternatives is an immediate match. This should only reject cases which are really broken. I didn't try this because I haven't seen anything like this in ifcvt.c while I have seen several places where we just bail out once reload_completed is true. Andreas
diff --git a/gcc/ifcvt.c b/gcc/ifcvt.c index 017944f4f79..1f5b9476ac2 100644 --- a/gcc/ifcvt.c +++ b/gcc/ifcvt.c @@ -4341,7 +4341,8 @@ find_if_header (basic_block test_bb, int pass) && cond_exec_find_if_block (&ce_info)) goto success; - if (targetm.have_trap () + if (!reload_completed + && targetm.have_trap () && optab_handler (ctrap_optab, word_mode) != CODE_FOR_nothing && find_cond_trap (test_bb, then_edge, else_edge)) goto success; diff --git a/gcc/testsuite/gcc.dg/pr103028.c b/gcc/testsuite/gcc.dg/pr103028.c new file mode 100644 index 00000000000..e299ac5d5b5 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr103028.c @@ -0,0 +1,16 @@ +/* PR rtl-optimization/103028 */ +/* { dg-do compile } */ +/* { dg-options "-Og -fif-conversion2 -fharden-conditional-branches" } */ + +/* This used to fail on s390x only with -march=z9-109 and -march=z9-ec */ +/* { dg-additional-options "-march=z9-ec" { target s390*-*-* } } */ + +unsigned char x; +int foo(void) +{ + unsigned long long i = x; + i = i + 0x80000000; + if (i > 0xffffffff) + return x; + return 0; +}