[PATCH] rs6000: Workaround for new ifcvt behavior [PR104335]

[PATCH] rs6000: Workaround for new ifcvt behavior [PR104335]

[Robin Dapp]

[-- Attachment #1: Type: text/plain, Size: 1098 bytes --]

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.

[-- Attachment #2: 0001-rs6000-Workaround-for-new-ifcvt-behavior-PR104335.patch --]
[-- Type: text/x-patch, Size: 1504 bytes --]

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(+)

diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
index eaba9a2d698..ebc5b0cefdc 100644
--- 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;
+
   /* 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

Re: [PATCH] rs6000: Workaround for new ifcvt behavior [PR104335]

[Segher Boessenkool]

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

Re: [PATCH] rs6000: Workaround for new ifcvt behavior [PR104335]

[Robin Dapp]

> 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

Re: [PATCH] rs6000: Workaround for new ifcvt behavior [PR104335]

[Peter Bergner]

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