[PATCH] rs6000: Support UN[GL][ET] in rs6000_maybe_emit_maxc_minc [PR105002]

[PATCH] rs6000: Support UN[GL][ET] in rs6000_maybe_emit_maxc_minc [PR105002]

[Kewen.Lin]

Hi,

Commit r12-7687 exposed one miss optimization chance in function
rs6000_maybe_emit_maxc_minc, for now it only considers comparison
codes GE/GT/LE/LT, but it can support more variants with codes
UNLT/UNLE/UNGT/UNGE by reversing them into the equivalent ones
with GE/GT/LE/LT.

Bootstrapped and regtested on powerpc64-linux-gnu P8 and
powerpc64le-linux-gnu P9 and P10.

Is it ok for trunk?

BR,
Kewen
-----
	PR target/105002

gcc/ChangeLog:

	* config/rs6000/rs6000.cc (rs6000_maybe_emit_maxc_minc): Support more
	comparison codes UNLT/UNLE/UNGT/UNGE.

---
 gcc/config/rs6000/rs6000.cc | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
index 283e8306ff7..b6a2509788f 100644
--- a/gcc/config/rs6000/rs6000.cc
+++ b/gcc/config/rs6000/rs6000.cc
@@ -15872,6 +15872,13 @@ rs6000_maybe_emit_maxc_minc (rtx dest, rtx op, rtx true_cond, rtx false_cond)
   if (result_mode != compare_mode)
     return false;

+  /* Canonicalize UN[GL][ET] to [LG][TE].  */
+  if (code == UNGE || code == UNGT || code == UNLE || code == UNLT)
+    {
+      code = reverse_condition_maybe_unordered (code);
+      std::swap (true_cond, false_cond);
+    }
+
   if (code == GE || code == GT)
     max_p = true;
   else if (code == LE || code == LT)
--
2.27.0

Re: [PATCH] rs6000: Support UN[GL][ET] in rs6000_maybe_emit_maxc_minc [PR105002]

[Segher Boessenkool]

Hi!

On Thu, Mar 24, 2022 at 10:00:43AM +0800, Kewen.Lin wrote:
> Commit r12-7687 exposed one miss optimization chance in function
> rs6000_maybe_emit_maxc_minc, for now it only considers comparison
> codes GE/GT/LE/LT, but it can support more variants with codes
> UNLT/UNLE/UNGT/UNGE by reversing them into the equivalent ones
> with GE/GT/LE/LT.

> +  /* Canonicalize UN[GL][ET] to [LG][TE].  */
> +  if (code == UNGE || code == UNGT || code == UNLE || code == UNLT)
> +    {
> +      code = reverse_condition_maybe_unordered (code);
> +      std::swap (true_cond, false_cond);
> +    }

Typically you would have to generate code to compensate for the reversed
comparison.  It works out fine here, but could you please restructure
the code to make that less tricky / more obvious?  Or at least add a
comment?

I wouldn't call it "canonicalisation" btw, LT is by no means more
standard than UNGE is.  You can say you are folding things so you later
have to support fewer cases, or similar?

Thanks,


Segher

Re: [PATCH] rs6000: Support UN[GL][ET] in rs6000_maybe_emit_maxc_minc [PR105002]

[Kewen.Lin]

Hi Segher,

on 2022/3/27 11:02 PM, Segher Boessenkool wrote:
> Hi!
> 
> On Thu, Mar 24, 2022 at 10:00:43AM +0800, Kewen.Lin wrote:
>> Commit r12-7687 exposed one miss optimization chance in function
>> rs6000_maybe_emit_maxc_minc, for now it only considers comparison
>> codes GE/GT/LE/LT, but it can support more variants with codes
>> UNLT/UNLE/UNGT/UNGE by reversing them into the equivalent ones
>> with GE/GT/LE/LT.
> 
>> +  /* Canonicalize UN[GL][ET] to [LG][TE].  */
>> +  if (code == UNGE || code == UNGT || code == UNLE || code == UNLT)
>> +    {
>> +      code = reverse_condition_maybe_unordered (code);
>> +      std::swap (true_cond, false_cond);
>> +    }
> 
> Typically you would have to generate code to compensate for the reversed
> comparison.  It works out fine here, but could you please restructure
> the code to make that less tricky / more obvious?  Or at least add a
> comment?
> 
> I wouldn't call it "canonicalisation" btw, LT is by no means more
> standard than UNGE is.  You can say you are folding things so you later
> have to support fewer cases, or similar?
> 

Thanks for the review!  Sorry for the late reply (I'm just back from
vacation).  I just posted v2 by adding more comments to describe the
reversions and changing the bad "canonicalisation" word, hope it looks
better to you.  :)

v2: https://gcc.gnu.org/pipermail/gcc-patches/2022-April/592624.html

BR,
Kewen