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

[Ulrich Weigand]

Chandler Carruth <chandlerc at google.com> wrote on 22.10.2012 20:53:54:

> So, I'm not really sure if this is the right approach. I'd like some
> folks from the LLVM side of things to chime in.
>
> In general, I'm not certain we want to continue growing our dependence
> on the signext and zeroext attributes on return types, or whether we
> want to do the extension in the frontend instead.

I don't really have any strong opinion one way or the other whether it
is preferable to do the extension in the front end or the back end; I'd
be happy to implement whatever you feel best.  However, clang currently
generates code that is definitely incompatible with ABI requirements on
PPC64 (only in some, but rather frequent, special cases), so I was hoping
to find a way to fix this specific codegen bug without requiring too much
of a redesign of the whole thing :-)


> Most of the targets in Clang currently eagerly zext or sext the return
> value to make it conform to the ABI. You can look at some of the other
> classify*Type methods in Clang for how.

Would you mind elaborating where to look, specifically?  What I'm seeing
is that some classify*Type methods select different types, like e.g.:

    return ABIArgInfo::getDirect(llvm::Type::getInt64Ty(getVMContext()));

I was hoping this could be used to implement extension for ABI purposes,
but it doesn't look like this will work (with the current infrastructure).

If the type specified in the classify*Type method is larger than the
actual parameter/return value type, CGCall.cpp will in fact create
extend operations, but those will always be zero-extends:
/// CoerceIntOrPtrToIntOrPtr - Convert a value Val to the specific Ty where
both
/// are either integers or pointers.  This does a truncation of the value
if it
/// is too large or a zero extension if it is too small.

Since the PPC64 ABI specifies sign- or zero-extension (depending on
the signedness of the original argument type at the source level),
this doesn't seem to help.

Did I miss some other method that would allow for sign-extends?


However, in any case, even if the extension were implemented fully in
the front-end, I'd *still* run into the very same test case problem:
"int" return/argument types would then be represented as "i64" in the
IR, which still wouldn't match test cases that are hard-coded to
expect "i32".  In fact, no matter what, those tests would fail, since
just plain "i32" with no attributes cannot express the semantics
required by the ABI, and any change from plain "i32" will cause the
test cases to break ...   So it seems to me the question of how to
deal with the tests is independent of the question whether to extend
in the front end or the back end.

[ B.t.w. doesn't LLVM support (or plan to support) targets where
"int" isn't a 32-bit type in the first place?  On such targets all
those tests would already break as well, in any case. ]

> On Mon, Oct 22, 2012 at 11:23 AM, Rafael Espíndola
> <rafael.espindola at gmail.com> wrote:
> >> 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 ...
> >
> > Not sure as well. It looks easy to forget to add this to new tests and
> > break a ppc64 bot. What abound adding some -triple=  to the tests?
> > LGTM with that change if you are ok with it. Just give it a day to see
> > if anyone else has another idea.
>
> I'd vote for not adding optional matching to the tests. Instead, we
> should specify an exact triple and match the expected IR exactly.
> However, let's figure out what the IR should look like first.

Well, I guess the "exact triple" would specify Intel, right?  In which
case the IR wouldn't really have to change ...

However, this would mean a whole bunch of test cases either wouldn't
be executed at all on non-Intel platforms, or else we'd have to have
(near-)duplicates of the tests for different platforms.  Neither option
looks really desirable to me.


If we want to check for exact matches, what about the following approach:
Have the test case dynamically determine which IR type an "int" source-
level type corresponds to on the target (e.g. by compiling a dummy
routine and capturing the output in a FileCheck variable), and then check
for that same result in the other tests in the file.


Thanks,
Ulrich