Commit Message
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.
---
gcc/ifcvt.c | 3 ++-
gcc/testsuite/gcc.dg/pr103028.c | 16 ++++++++++++++++
2 files changed, 18 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.dg/pr103028.c
Comments
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
@@ -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;
new file mode 100644
@@ -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;
+}