[v2] rs6000: Support UN[GL][ET] in rs6000_maybe_emit_maxc_minc [PR105002]
Commit Message
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(-)
--
2.27.0
Comments
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
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
@@ -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)