[cfe-commits] [PATCH/RFC, PowerPC] Extend 32-bit function arguments / return values

[Ulrich Weigand]

Hello,

in tracking down a failure in the tramp3d-v4 test case on PowerPC Linux, I
discovered that clang generates code that is incompatible with the PowerPC
Linux ABI in some cases.  The ABI states that integer quantities passed to
or returned from a function need to be zero- or sign-extended to 64-bit,
depending on whether the underlying type is unsigned or signed.

This is actually implemented in the llvm target back-end in
PPCTargetLowering::LowerCall_Darwin_Or_64SVR4.  However, at this point we
don't actually have type information from the original source any more, so
we wouldn't know whether to sign- or zero-extend.  Fortunately, the LLVM IR
provides for attributes that can be attached to function return values and
arguments to provide exactly this information ("signext" or "zeroext").

Unfortunately, these attributes are currently computed by clang using the
Type::isPromotableIntegerType predicate.  This appears to checks whether
the type needs to undergo the default promotions according to the C
standard.  Now for PPC64, this isn't quite the same, since we need to
promote 32-bit types (int, unsigned int) to 64-bit as well, and
Type::isPromotableIntegerType always returns false for those.  Thus those
types don't get a signext/zeroext attribute, and the back-end doesn't know
how to promote them.  (It actually always zero-extends them, which happens
to be wrong in the test case that fails for me.)

To fix this, I've implemented new versions of classifyArgumentType and
classifyReturnType for PPC64_SVR4_ABIInfo, which are the same as the
default versions except that they also classify "int" and "unsigned int" as
types needing extending.   This does in fact fix the problem I'm seeing.

Unfortunately, this change also causes about 20 spurious regression test
failures on PowerPC, since IR output now frequently (i.e. for plain "int"
types) contains an extra "signext" attribute, which throws off strict
text-matching in various tests.  I've fixed those by optionally accepting
"signext" (or "zeroext" as the case may be) via a regexp.   Not sure if
this is really the best way of handling this ...

To simplify review, I've attached two patches, one with the actual fix (and
a new test for it), and one that contains all the signext/zeroext test
changes.

Would this be OK to commit?

Bye,
Ulrich
(See attached file: diff-clang-ppc64-extend)(See attached file:
diff-clang-ppc64-extend-testfixes)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: diff-clang-ppc64-extend
Type: application/octet-stream
Size: 3418 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121022/67086839/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: diff-clang-ppc64-extend-testfixes
Type: application/octet-stream
Size: 21310 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121022/67086839/attachment-0001.obj>