This is an archive of the discontinued LLVM Phabricator instance.

[ARM] GlobalISel: Support i8/i16 ABI extensions
ClosedPublic

Authored by rovka on Dec 13 2016, 2:22 AM.

Details

Summary

At the moment, this means supporting the signext/zeroext attribute on the return
type of the function. For function arguments, signext/zeroext should be handled
by the caller, so there's nothing for us to do until we start lowering calls.

Note that this does not include support for other extensions (i8 to i16), those will
be added later.

Diff Detail

Event Timeline

rovka updated this revision to Diff 81201.Dec 13 2016, 2:22 AM
rovka retitled this revision from to [ARM] GlobalISel: Support i8/i16 ABI extensions.
rovka updated this object.
rovka added reviewers: ab, qcolombet, t.p.northover.
rovka added subscribers: llvm-commits, rengolin, dsanders.
t.p.northover added inline comments.Dec 16 2016, 11:35 AM
lib/Target/ARM/ARMInstructionSelector.cpp
78

If we don't already, it would be a very good idea to make sure we bail completely in Thumb mode. Incidentally, is there any particular reason you decided to go for ARM first? I thought most things were compiled in Thumb these days.

rovka updated this revision to Diff 81944.Dec 19 2016, 6:52 AM

Bail out if we're in Thumb mode (I'll commit that part separately, I'm just adding it here to make sure it looks ok).

rovka added inline comments.Dec 19 2016, 6:58 AM
lib/Target/ARM/ARMInstructionSelector.cpp
78

I'm mostly going with ARM first because it seems simpler than Thumb.
At the moment I'm trying to focus on the call lowering + what it needs to get some simple examples through the entire pipeline. I don't want to beef up the instruction selector too much, because that will go away anyway when we move to TableGen. So for instance we can select the extensions that we need for the ABI zeroext/signext, but I'm not really in any rush right now to add other extensions (e.g. from i8 to i16 or whatever).
I'm open to suggestions if this sounds like a bad plan :)

rengolin accepted this revision.Jan 23 2017, 8:48 AM

Hi Diana,

There's nothing really complicated in this patch, and it looks obvious to me. LGTM. Thanks!

This revision is now accepted and ready to land.Jan 23 2017, 8:48 AM
This revision was automatically updated to reflect the committed changes.