This is an archive of the discontinued LLVM Phabricator instance.

[ARM] Rewrite isLegalT2AddressImmediate
ClosedPublic

Authored by dmgreen on Apr 15 2019, 1:32 AM.

Details

Summary

This does two main things, firstly adding some at least basic addressing modes for i64 types, and secondly treats floats and doubles sensibly when there is no fpu. The floating point change can help codesize in some cases, especially with D60294.

Most backends seems to not consider the exact VT in isLegalAddressingMode, instead switching on type size. That is now what this does when the target does not have an fpu (as the float data will be loaded using LDR's). i64's currently use the address range of an LDRD (even though they may be legalised and loaded with an LDR). This is at least better than marking them all as illegal addressing modes.

The test case has been heavily worked, I'm just showing the differences here for clarity. I have not attempted to do anything with vectors yet. That will need changing once MVE is added.

Diff Detail

Repository
rL LLVM

Event Timeline

dmgreen created this revision.Apr 15 2019, 1:32 AM
t.p.northover added inline comments.
llvm/lib/Target/ARM/ARMISelLowering.cpp
13299 ↗(On Diff #195100)

Variables start with capitals for now.

13305–13307 ↗(On Diff #195100)

Early exit would probably be nicer here (if invalid type, return false immediately), and I think it can go at the very top of the function too.

Also, I believe this is incorrect for i1 (ends up with NumBytes == 0).

Finally, this expands the function to cover vector types too. Is that intentional?

13314 ↗(On Diff #195100)

While you're here we should switch this to use isUInt<8> from MathExtras.h. Same for the later ones.

dmgreen updated this revision to Diff 195208.Apr 15 2019, 9:38 AM
dmgreen marked 4 inline comments as done.
dmgreen added inline comments.
llvm/lib/Target/ARM/ARMISelLowering.cpp
13305–13307 ↗(On Diff #195100)

I was originally going to say that MVE will be coming soon (you may have seen), and I would follow up then, but my understanding is that Neon types do not have immediates like these. I've had it return false on vectors, so the T32 results of testvecs in gep.ll have not changed.

t.p.northover accepted this revision.Apr 18 2019, 2:50 AM

LGTM. Thanks for updating it.

llvm/lib/Target/ARM/ARMISelLowering.cpp
13305–13307 ↗(On Diff #195100)

Oh yes, sorry. I got too focused on the code and ignored the context.

This revision is now accepted and ready to land.Apr 18 2019, 2:50 AM
This revision was automatically updated to reflect the committed changes.