* [PATCH v2] rs6000: Support UN[GL][ET] in rs6000_maybe_emit_maxc_minc [PR105002]
@ 2022-04-01 6:27 Kewen.Lin
2022-04-01 14:49 ` Segher Boessenkool
0 siblings, 1 reply; 3+ messages in thread
From: Kewen.Lin @ 2022-04-01 6:27 UTC (permalink / raw)
To: GCC Patches; +Cc: Segher Boessenkool, David Edelsohn, Peter Bergner
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?
v1: https://gcc.gnu.org/pipermail/gcc-patches/2022-March/592204.html
v2: Add more comments to provide better readability as Segher suggested.
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 | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
index f41b8f740ba..40db13e6498 100644
--- a/gcc/config/rs6000/rs6000.cc
+++ b/gcc/config/rs6000/rs6000.cc
@@ -15838,11 +15838,26 @@ rs6000_maybe_emit_maxc_minc (rtx dest, rtx op, rtx true_cond, rtx false_cond)
rtx op1 = XEXP (op, 1);
machine_mode compare_mode = GET_MODE (op0);
machine_mode result_mode = GET_MODE (dest);
- bool max_p = false;
+ bool max_p;
if (result_mode != compare_mode)
return false;
+ /* See the comments of this function, it simply expects GE/GT/LE/LT in
+ the checks, but for the reversible equivalent UNLT/UNLE/UNGT/UNGE,
+ we need to do the reversions first to make the following checks
+ support fewer cases, like:
+
+ (a UNLT b) ? op1 : op2 => (a >= b) ? op2 : op1;
+ (a UNLE b) ? op1 : op2 => (a > b) ? op2 : op1;
+ (a UNGT b) ? op1 : op2 => (a <= b) ? op2 : op1;
+ (a UNGE b) ? op1 : op2 => (a < b) ? op2 : op1; */
+ 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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] rs6000: Support UN[GL][ET] in rs6000_maybe_emit_maxc_minc [PR105002]
2022-04-01 6:27 [PATCH v2] rs6000: Support UN[GL][ET] in rs6000_maybe_emit_maxc_minc [PR105002] Kewen.Lin
@ 2022-04-01 14:49 ` Segher Boessenkool
2022-04-06 5:53 ` Kewen.Lin
0 siblings, 1 reply; 3+ messages in thread
From: Segher Boessenkool @ 2022-04-01 14:49 UTC (permalink / raw)
To: Kewen.Lin; +Cc: GCC Patches, David Edelsohn, Peter Bergner
Hi!
On Fri, Apr 01, 2022 at 02:27:14PM +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.
You may want to add somewhere (in the comment in the code perhaps?)
that if we see e.g. UNLT it guarantees that we have 4-way condition
codes (LT/GT/EQ/UN), so we do not have to check for fast-math or the
like. This is always true of course, but it doesn't hurt to remind
the reader :-)
The PR marker goes here:
PR target/105002
> * config/rs6000/rs6000.cc (rs6000_maybe_emit_maxc_minc): Support more
> comparison codes UNLT/UNLE/UNGT/UNGE.
> - bool max_p = false;
> + bool max_p;
Please move this to later, since you touch it anyway:
bool max_p;
> if (code == GE || code == GT)
> max_p = true;
> else if (code == LE || code == LT)
Okay for trunk with those finishing touches. Thanks!
Segher
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] rs6000: Support UN[GL][ET] in rs6000_maybe_emit_maxc_minc [PR105002]
2022-04-01 14:49 ` Segher Boessenkool
@ 2022-04-06 5:53 ` Kewen.Lin
0 siblings, 0 replies; 3+ messages in thread
From: Kewen.Lin @ 2022-04-06 5:53 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: GCC Patches, David Edelsohn, Peter Bergner
on 2022/4/1 10:49 PM, Segher Boessenkool wrote:
> Hi!
>
> On Fri, Apr 01, 2022 at 02:27:14PM +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.
>
> You may want to add somewhere (in the comment in the code perhaps?)
> that if we see e.g. UNLT it guarantees that we have 4-way condition
> codes (LT/GT/EQ/UN), so we do not have to check for fast-math or the
> like. This is always true of course, but it doesn't hurt to remind
> the reader :-)
>
> The PR marker goes here:
>
> PR target/105002
>> * config/rs6000/rs6000.cc (rs6000_maybe_emit_maxc_minc): Support more
>> comparison codes UNLT/UNLE/UNGT/UNGE.
>
The current daily bump seems smart and as verified with gcc-verify that
it's able to add the PR marker into the (each if there are multiple) place
that needs it.
But anyway I updated the commit log as you suggested. :)
>> - bool max_p = false;
>> + bool max_p;
>
> Please move this to later, since you touch it anyway:
>
> bool max_p;
>> if (code == GE || code == GT)
>> max_p = true;
>> else if (code == LE || code == LT)
>
> Okay for trunk with those finishing touches. Thanks!
>
Thanks for the review! Addressed the comments and committed as r12-8010.
BR,
Kewen
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-04-06 5:53 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-01 6:27 [PATCH v2] rs6000: Support UN[GL][ET] in rs6000_maybe_emit_maxc_minc [PR105002] Kewen.Lin
2022-04-01 14:49 ` Segher Boessenkool
2022-04-06 5:53 ` Kewen.Lin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).