[PATCH] D21739: [TTI] Add functions determining if int parameters/returns should be zeroext/signext.

[Daniel Sanders via llvm-commits]

dsanders added inline comments.

================
Comment at: lib/Target/Mips/MipsTargetTransformInfo.h:42
@@ +41,3 @@
+
+  bool shouldExtI32Param() const { return ST->isABI_N64(); }
+};
----------------
koriakin wrote:
> dsanders wrote:
> > This doesn't look correct to me. On our target we unconditionally add the signext attribute to i32 in the frontend. This only has a significant effect for the 64-bit ABI's (N32 and N64) but it's still useful for the 32-bit ABI's (O32) as a reminder of how 32-bit code operates when run on a 64-bit processor (every instruction from the 32-bit subset sign-extends the result value from 32-bit to register width).
> > 
> > Returns also behave the same way as parameters.
> So, for Mips, we'd need a third predicate that forces a signext even for unsigned parameters, right?
> 
> Returns don't seem to get any ext attribute, though:
> 
> unsigned f(unsigned x) {
>         return x;
> }
> 
> gets compiled by clang to:
> 
> define i32 @f(i32 signext) #0 { ... }
> So, for Mips, we'd need a third predicate that forces a signext even for unsigned parameters, right?
 
That's right except it's just 'unsigned int' that ignores the signedness rather than all unsigned parameters. The other unsigned integers use a sign-dependant extension. For example, unsigned short gets zeroext because it's equivalent to the two-step extension (zero-extend to i32, sign-extend to register width) described by our calling convention.

> Returns don't seem to get any ext attribute, though:
> 
> unsigned f(unsigned x) {
> 
> return x;
> }
> 
> gets compiled by clang to:
> 
> define i32 @f(i32 signext) #0 { ... }

That's worrying, there's probably a subtle sign/zero-extension bug lurking somewhere. It's often difficult to identify them since CPU's tend to be more lenient than our ISA (e.g. a 32-bit addition using register values 0x1_80000000 and 0x0 is 'unpredictable' according to our ISA but most implementations will give the expected 0xffffffff_80000000).

I'll mention this likely bug to the people working on the sanitizers since they're tracking down a sign/zero extension bug on MIPS64 at the moment.


Repository:
  rL LLVM

http://reviews.llvm.org/D21739