rs6000: Workaround for new ifcvt behavior [PR104335]
Commit Message
Hi,
since r12-6747-gaa8cfe785953a0 ifcvt not only passes real comparisons
but also "cc comparisons" (i.e. the representation of the result of a
comparison) to the backend. rs6000_emit_int_cmove () is not prepared to
handle this. Therefore, this patch makes it return false in such a case
in order to avoid an ICE.
I bootstrapped (with --enable-languages=all on P10,
--enable-languages="c, c++, fortran, go, lto, objc, obj-c++" otherwise)
and regtested on Power7, Power8, Power9 and Power10.
Testsuite is unchanged on P7 and P9. On P8 I hit some different FAILs
vs master but they look unrelated and seem to be caused by "spawn
failed" i.e. out of memory or so.
On P10 I compared the testsuite of the last commit before the breaking
one (r12-6746-ge9ebb86799fd77, but commenting out a line that would
still result in a "-Wformat-diag" bootstrap error then) vs. the patched
master: No regressions.
Is it OK?
Regards
Robin
--
PR target/104335
gcc/ChangeLog:
* config/rs6000/rs6000.cc (rs6000_emit_int_cmove): Return false
if the expected comparison's first operand is of mode MODE_CC.
Comments
Hi!
On Wed, Feb 16, 2022 at 08:11:17PM +0100, Robin Dapp wrote:
> since r12-6747-gaa8cfe785953a0 ifcvt not only passes real comparisons
> but also "cc comparisons" (i.e. the representation of the result of a
> comparison) to the backend. rs6000_emit_int_cmove () is not prepared to
> handle this. Therefore, this patch makes it return false in such a case
> in order to avoid an ICE.
> On P10 I compared the testsuite of the last commit before the breaking
> one (r12-6746-ge9ebb86799fd77, but commenting out a line that would
> still result in a "-Wformat-diag" bootstrap error then)
I have used --disable-werror for weeks already :-(
> PR target/104335
>
> gcc/ChangeLog:
>
> * config/rs6000/rs6000.cc (rs6000_emit_int_cmove): Return false
> if the expected comparison's first operand is of mode MODE_CC.
Please send patches as plain text, not as base64.
> --- a/gcc/config/rs6000/rs6000.cc
> +++ b/gcc/config/rs6000/rs6000.cc
> @@ -16175,6 +16175,12 @@ rs6000_emit_int_cmove (rtx dest, rtx op, rtx true_cond, rtx false_cond)
> if (mode != SImode && (!TARGET_POWERPC64 || mode != DImode))
> return false;
>
> + /* PR104335: We now need to expect CC-mode "comparisons"
> + coming from ifcvt. The following code expects proper
> + comparisons so better abort here. */
> + if (XEXP (op, 0) && GET_MODE_CLASS (GET_MODE (XEXP (op, 0))) == MODE_CC)
> + return false;
Why that first test? XEXP (op, 0) is required to not be nil.
The patch is okay without that (if it passes testing of course :-) )
Thanks!
Segher
> Please send patches as plain text, not as base64.
It seems like Thunderbird does not support this anymore since later
versions, grml. Probably need to look for another mail client.
> Why that first test? XEXP (op, 0) is required to not be nil.
>
> The patch is okay without that (if it passes testing of course :-) )
> Thanks!
Pushed without the XEXP test. Hope this fixes things for you.
Regards
Robin
On 2/17/22 1:04 PM, Robin Dapp via Gcc-patches wrote:
>> Please send patches as plain text, not as base64.
>
> It seems like Thunderbird does not support this anymore since later
> versions, grml. Probably need to look for another mail client.
I use Thunderbird with no problems. I use the Quicktext addon/extension
and it gives me a toolbar option "Other" which has a "Insert file as text"
which allows adding a patch to your email inline with no whitespace munging.
Peter
From b9f053bf2222266bd1518e0eac36509ebde57266 Mon Sep 17 00:00:00 2001
From: Robin Dapp <rdapp@linux.ibm.com>
Date: Thu, 10 Feb 2022 09:01:51 -0600
Subject: [PATCH] rs6000: Workaround for new ifcvt behavior [PR104335].
Since r12-6747-gaa8cfe785953a0 ifcvt passes a "cc comparison"
i.e. the representation of the result of a comparison to the
backend. rs6000_emit_int_cmove () is not prepared to handle this.
Therefore, this patch makes it return false in such a case.
PR target/104335
gcc/ChangeLog:
* config/rs6000/rs6000.cc (rs6000_emit_int_cmove): Return false
if the expected comparison's first operand is of mode MODE_CC.
---
gcc/config/rs6000/rs6000.cc | 6 ++++++
1 file changed, 6 insertions(+)
@@ -16175,6 +16175,12 @@ rs6000_emit_int_cmove (rtx dest, rtx op, rtx true_cond, rtx false_cond)
if (mode != SImode && (!TARGET_POWERPC64 || mode != DImode))
return false;
+ /* PR104335: We now need to expect CC-mode "comparisons"
+ coming from ifcvt. The following code expects proper
+ comparisons so better abort here. */
+ if (XEXP (op, 0) && GET_MODE_CLASS (GET_MODE (XEXP (op, 0))) == MODE_CC)
+ return false;
+
/* We still have to do the compare, because isel doesn't do a
compare, it just looks at the CRx bits set by a previous compare
instruction. */
--
2.31.1