[PATCH] [ARM] generate VMAXNM/VMINNM for a compare followed by a select (in safe math mode too)

Renato Golin renato.golin at linaro.org
Tue May 5 12:33:14 PDT 2015


On 5 May 2015 at 19:58, Artyom Skrobov <artyom.skrobov at arm.com> wrote:
> That's correct for VMAX/VMIN; VMAXNM/VMINNM differ from them by returning the non-NaN operand in the case when the other operand is NaN.

Ha! Sorry. You're right.


> My point is that there aren't, in fact, dependencies between the three switch blocks -- each of them operates on different data.
> Therefore, it's enough to observe that each of the switch blocks, in isolation, behaves correctly; thence I can conclude that the combination of the three behaves correctly, too.

That's not what I meant. It's to do with the exact semantics in both
VMAXNM/VMINNM and the DAG it's coming from, to make sure that all edge
cases are covered.

>From what I saw, I believe they are, like you say, the
negative/positive zeros, and NaN. I also believe the swapping of the
arguments is correct by getting the inverse CC (something that wasn't
in the original code). I think that should be safe, but I haven't
looked thoroughly.


> On top of that, I think that the new test cases cover all of the possible combinations (non-zeroes and zeroes of different signs, on different sides of the comparison, with different comparison operators).

You do seem to be covering all bases, so yes, that helps a lot.

Another question.

You're using DAG.isKnownNeverNaN(LHS) to mean "is LHS a non-NaN
constant". For now, that's ok because the function only recognise
constants, but there's a TODO in there to recognise more cases, which
could very well get non-constant cases (if they can be traced back).
Would that affect your premises?

cheers,
--renato



More information about the llvm-commits mailing list