Invalid comparison instruction generation

I have a simple program that generates correct intermediate
representation. However, when working on my backend, and my lowering
function gets called. The comparison operation is flipped via an invalid
transformation. i.e. gt ==> le, lt ==> ge etc..

define void @test_fc_if_gt(double %x, double %y, double addrspace(11)*
%result) {

entry:

        %x.addr = alloca double ; <double*> [#uses=3]

        %y.addr = alloca double ; <double*> [#uses=2]

        %result.addr = alloca double addrspace(11)* ;
<double addrspace(11)**> [#uses=2]

        store double %x, double* %x.addr

        store double %y, double* %y.addr

        store double addrspace(11)* %result, double addrspace(11)**
%result.addr

        %tmp = load double* %x.addr ; <double> [#uses=1]

        %tmp1 = load double* %y.addr ; <double> [#uses=1]

        %cmp = fcmp ogt double %tmp, %tmp1 ; <i1> [#uses=1]

        br i1 %cmp, label %ifthen, label %ifend

ifthen: ; preds = %entry

        %tmp2 = load double addrspace(11)** %result.addr
; <double addrspace(11)*> [#uses=1]

        %tmp3 = load double* %x.addr ; <double> [#uses=1]

        store double %tmp3, double addrspace(11)* %tmp2

        br label %ifend

ifend: ; preds = %ifthen, %entry

        ret void

}

With the above kernel run through llc with -march=x86
-view-dag-combine1-dags I still see the ogt as the comparison operation,
but when I run it with llc -march=x86 -view-legalize-dags the ogt node
has been transformed into a ule.

So, my question is, how do I get llvm to stop doing invalid translation
of comparison instructions? This problem affects my custom backend and I
have reproduced it with the x86 backend.

Micah Villmow

Systems Engineer

Advanced Technology & Performance

Advanced Micro Devices Inc.

4555 Great America Pkwy,

Santa Clara, CA. 95054

P: 408-572-6219

F: 408-572-6596

good-graph.dot (8.75 KB)

bad-graph.dot (7.33 KB)

With the above kernel run through llc with -march=x86
-view-dag-combine1-dags I still see the ogt as the comparison operation, but
when I run it with llc -march=x86 -view-legalize-dags the ogt node has been
transformed into a ule.

Okay... I can see that in the attached graph.

So, my question is, how do I get llvm to stop doing invalid translation of
comparison instructions? This problem affects my custom backend and I have
reproduced it with the x86 backend.

It seems valid to me... what makes you think it's invalid?

-Eli

Eli,
Using the variables from the original IR,
assuming tmp == tmp1 and assume the value is not nan
ogt(tmp, tmp1) is !isnan(tmp) && !isnan(tmp1) && tmp > tmp1, or false
ule(tmp, tmp1) is isnan(tmp) || isnan(tmp1) || tmp <= tmp1, or true

So, this is invalid, or am I misunderstanding what ogt and ule stand
for?

Assuming this is valid, why convert comparison instructions instead of
just passing them through as they originally exist? The backend I am
targeting does not support all comparison instructions and trying to
guess which instruction LLVM converted the current comparison
instruction from and then converting to a supported instruction is not
as simple as it can be.
For example, I need to convert all ogt instructions to an olt
instruction with LHS and RHS swapped, but since ogt is converted to ule,
do I convert all ule into olt and swap?

Thanks,

Micah

Eli,
Using the variables from the original IR,
assuming tmp == tmp1 and assume the value is not nan
ogt(tmp, tmp1) is !isnan(tmp) && !isnan(tmp1) && tmp > tmp1, or false
ule(tmp, tmp1) is isnan(tmp) || isnan(tmp1) || tmp <= tmp1, or true

So, this is invalid, or am I misunderstanding what ogt and ule stand
for?

You'll notice in the "good.dot" graph that there's an "xor" after the
"setcc". So you basically have this in the original graph:

  xor (setcc ogt %tmp1, %tmp2), 1

which is equivalent to

  setcc ule %tmp1, %tmp2

There is a change from "ordered" to "unordered" comparison. But I
don't know if that will cause any troubles here.

Assuming this is valid, why convert comparison instructions instead of
just passing them through as they originally exist?

I'm assuming that it's trying to get rid of the "xor" instruction as
an optimization.

The backend I am
targeting does not support all comparison instructions and trying to
guess which instruction LLVM converted the current comparison
instruction from and then converting to a supported instruction is not
as simple as it can be.
For example, I need to convert all ogt instructions to an olt
instruction with LHS and RHS swapped, but since ogt is converted to ule,
do I convert all ule into olt and swap?

I would think that you would need your own custom lowering code to do
this. Have you declared these invalid operators as "Custom" in your
*ISelLowering.cpp file?

-bw

Eli,
Using the variables from the original IR,
assuming tmp == tmp1 and assume the value is not nan
ogt(tmp, tmp1) is !isnan(tmp) && !isnan(tmp1) && tmp > tmp1, or false
ule(tmp, tmp1) is isnan(tmp) || isnan(tmp1) || tmp <= tmp1, or true

Correct; in fact, ogt and ule are exact opposites.

So, this is invalid, or am I misunderstanding what ogt and ule stand
for?

What you're missing is the xor in the pre-combine graph; I assume it
got added during branch lowering.

Assuming this is valid, why convert comparison instructions instead of
just passing them through as they originally exist? The backend I am
targeting does not support all comparison instructions and trying to
guess which instruction LLVM converted the current comparison
instruction from and then converting to a supported instruction is not
as simple as it can be.

It's not a matter of guessing... you need to able to support all of
the possible comparisons, since they can be introduced by optimizers
at multiple levels.

For example, I need to convert all ogt instructions to an olt
instruction with LHS and RHS swapped, but since ogt is converted to ule,
do I convert all ule into olt and swap?

The existing legalization infrastructure for condition codes is in
SelectionDAGLegalize::LegalizeSetCCCondCode. If this isn't flexible
enough, you might need to modify it a bit. You should be able to just
do something like "setCondCodeAction(ISD::SETOGT, MVT::f64, Expand);"
for the unsupported comparisons, and let Legalize should take care of
the rest.

Oh, and it looks like there's a legitimate bug in
DAGCombiner::visitXOR: it needs to check whether condition codes are
legal before transforming them.

-Eli

Hi Eli,

Oh, and it looks like there's a legitimate bug in
DAGCombiner::visitXOR: it needs to check whether condition codes are
legal before transforming them.

Please write a bugzilla for it.

Thanks!
-bw

Eli/Bill,
Thanks for clearing this up. For some reason the xor is not being
propagated through my backend and I need to figure out why. I'll look
into the files mentioned and see if I can get llvm to do what I need.

Thanks as always,
Micah