On Thu, Feb 24, 2022 at 08:07:28AM +0100, Robin Dapp wrote:
> Hi,
> 
> > Robin's patch has the effct making rs6000_emit_int_cmove return false for
> > floating point comparisons, so I marked the bug as being a duplicate of PR
> > target/104335.
> 
> Didn't I just return false for MODE_CC?  This should not affect proper
> floating-point comparisons.  It looks like the problem was indeed caused
> by my original patch but then I wonder why I did not catch it in the
> first place despite running a Power9 bootstrap and regtest (with Fortran
> of course) that looked unchanged.

It only showed up with some specific options (-O1 -fnon-call-exceptions and
-mcpu set to power9 or power10).  If you use -O0, -O2, or -O3 it doesn't show
up, and if you don't use -fnon-call-exceptions it doesn't show up.  In
particular, you needed ISEL enabled (power8 doesn't enable it due to
performance reasons).  Bill Seuer reported the bug.  Maybe he has more details.

Returning false is fine in this case.  It just says that we can't do
conditional move (i.e. use the ISEL instruction).  The compiler will fall back
to doing a jump around a move.

However, in doing the patch, I tried to get the compiler to generate cmoves for
integer values with floating point conditions, and I just don't see it normally
being generated.

I suspect if we want to do it, it will be a much larger project for the GCC 13
time frame.  As I recall, there was one Spec test that really wanted to do
something like this, but to re-architect this will be a large undertaking.

At the moment, we allow mixed floating point test and conditional move:

        float f1, f2;
        double d1, d2, d3;

        d1 = (f1 == f2) ? d2 : d3;

When I added the IEEE 128-bit support, it because a combinatorial explosion to
try and handle all possible compares and moves.  Eventually, we decided just to
only do the IEEE 128-bit cmove if the test and values being moved were the same
mode (i.e. KFmode or TFmode), and you couldn't mix float/double and __float128.

Int cmoves have traditionally just not thought of having floating point
comparisons.  They could, it just wasn't thought of.

> Shouldn't this have come up? I vaguely recall seeing maxloc FAILs at
> some point but not in the final runs.  Going to re-check because this
> would have helped not introduce the problem that late.

As I said, it needed some specific cases to get it to fail.

-- 
Michael Meissner, IBM
PO Box 98, Ayer, Massachusetts, USA, 01432
email: meiss...@linux.ibm.com

Reply via email to