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

Reply via email to