llvm dev - Nov 2008 - [LLVMdev] Invalid comparison instruction generation

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

-----Original Message-----
From: llvmdev-bounces at cs.uiuc.edu [mailto:llvmdev-bounces at cs.uiuc.edu]
On Behalf Of Eli Friedman
Sent: Monday, November 10, 2008 4:38 PM
To: LLVM Developers Mailing List
Subject: Re: [LLVMdev] Invalid comparison instruction generation

On Mon, Nov 10, 2008 at 3:06 PM, Villmow, Micah <Micah.Villmow at amd.com>
wrote:
> 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
_______________________________________________
LLVM Developers mailing list
LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

On Mon, Nov 10, 2008 at 5:00 PM, Villmow, Micah <Micah.Villmow at amd.com>
wrote:
> 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

On Mon, Nov 10, 2008 at 5:00 PM, Villmow, Micah <Micah.Villmow at amd.com>
wrote:
> 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

-----Original Message-----
From: llvmdev-bounces at cs.uiuc.edu [mailto:llvmdev-bounces at cs.uiuc.edu]
On Behalf Of Eli Friedman
Sent: Monday, November 10, 2008 5:35 PM
To: LLVM Developers Mailing List
Subject: Re: [LLVMdev] Invalid comparison instruction generation

On Mon, Nov 10, 2008 at 5:00 PM, Villmow, Micah <Micah.Villmow at amd.com>
wrote:
> 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
_______________________________________________
LLVM Developers mailing list
LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev