[PATCH] Make it possible for ints/floats to return different values from getBooleanContents()

[Daniel Sanders]

> -----Original Message-----
> From: Tom Stellard [mailto:tom at stellard.net]
> Sent: 04 July 2014 16:30
> To: reviews+D4389+public+c5772e8a79ffe4c5 at reviews.llvm.org
> Cc: Daniel Sanders; llvm-commits at cs.uiuc.edu
> Subject: Re: [PATCH] Make it possible for ints/floats to return different
> values from getBooleanContents()
> 
> On Fri, Jul 04, 2014 at 09:32:30AM +0000, Daniel Sanders wrote:
> > On MIPS32r6/MIPS64r6, floating point comparisons return 0 or -1 but
> > integer comparisons return 0 or 1.
> >
> 
> On the older R600 subtargets, floating-point comparisons can return either
> 0.0f and 1.0f  or 0 and -1 depending on the instruction.
> 
> We currently implement this by lowering SETCC to SELECT_CC and setting the
> True/False operands to use the correct boolean values.
> 
> Do you have any sense for how your changes may affect these subtargets?
> I'm guessing we probably want to use ZeroOrNegativeOneBoolean contents
> for floating-point comparisons, but I'm not sure.
> 
> -Tom

There should be no change to R600's behaviour as a result of this patch. If there are any, then it's probably a bug. At the moment, the R600 backend says that it uses 0 or -1 for all comparisons (grep for setBooleanContents) and the existing behaviour is supposed to be preserved except for when float and non-float boolean contents differ which should only be the case for MIPS32r6/MIPS64r6 at the moment. The patch should allow you to add a ZeroOrFloatingPointOneBoolean in the future (assuming you can use the EVT to tell whether to use 0/-1 or 0.0f/1.0f) which would be helpful if other targets also use 0.0f and 1.0f and want to share the simplification code.

Whether R600 can hit a bad case mostly depends on when you are lowering SETCC. The case that tripped up MIPS was in the pi benchmark and looked something like this:
		; Given %a, %b, and %n
	start:
		...
		%0 = fcmp olt float %a, %b
		br i1 %0, label %T, label %F
	T:
		%n.1 = add i32 %n, 1
		br label %F
	F:
		%n.2 = phi i32 [ %n.1, %T ], [ %n, %start ]
And was simplified to this DAG because one of the simplifications believed the comparison produced 0 or 1.
	(ADD (SETCC $a, $b, SETOLT), $n)
Unfortunately, the comparison really produced -1 and the counter decremented instead of incrementing.

If you are lowering SETCC to SELECT_CC before type legalization then I wouldn't expect it to be possible to hit a similar case for R600. This is because the target-dependent boolean never appears as a value in the SelectionDAG and is completely contained inside the SELECT_CC node. 
However, if you are lowering after type legalization then it may be possible to trigger a bad case. Simplifying the above example to (ADD (AND (SETCC $a, $b, SETOLT), 1, $n) would seem to be valid based on the current values of the getBooleanContents() family but would generate incorrect code if the comparison really produced 1.0f or 0.0f.

I guess you must be lowering before type legalization since it looks like the common code would have already lowered into (SELECT_CC $a, $b, -1, 0) if you weren't.