This is an archive of the discontinued LLVM Phabricator instance.

[SystemZ] Use valid base/index registers for memory constraints
ClosedPublic

Authored by zhanjunl on Aug 10 2016, 1:12 PM.

Details

Summary

Inline asm memory constraints can have the base or index register be assigned to %r0 right now.
Make sure that we assign only ADDR64 registers to the base and index.

Diff Detail

Event Timeline

zhanjunl retitled this revision from to [SystemZ] Use valid base/index registers for memory constraints.
zhanjunl updated this object.
zhanjunl added a reviewer: uweigand.
zhanjunl added a subscriber: llvm-commits.
uweigand edited edge metadata.Aug 17 2016, 9:38 AM

Hmm, I guess that makes sense. I'm wondering why most other targets don't have to do anything similar ...

I am a bit concerned about the ISD::Register tests. I assume this is to catch the case where no base and/or no index register are actually used; in those cases selectBDXAddr sets Base and/or Index to CurDAG->getRegister(0, VT). In those cases, leaving this as-is is fine. (Mentioning this in a comment would be good, though.) However, can there be other instances of ISD::Register? Can this e.g. be hard-coded by a user somehow? What if there is a hard-coded register, which is *not* suitable for use in addressing?

Hmm, I guess that makes sense. I'm wondering why most other targets don't have to do anything similar ...

I am a bit concerned about the ISD::Register tests. I assume this is to catch the case where no base and/or no index register are actually used; in those cases selectBDXAddr sets Base and/or Index to CurDAG->getRegister(0, VT). In those cases, leaving this as-is is fine. (Mentioning this in a comment would be good, though.) However, can there be other instances of ISD::Register? Can this e.g. be hard-coded by a user somehow? What if there is a hard-coded register, which is *not* suitable for use in addressing?

I'm not sure of the other targets, but PowerPC has to do something similar because they have the same restrictions (r0 cannot be used in addresses).

My thinking is that if there's a hard coded register, then we should let it go through, flaws and all. So for the example of a user hard coding the register, we should be doing exactly what the user wants. If the user specifies r0 as the base/index register, then r0 should be what we use for that register, even if it's wrong. LLVM will throw an error when it detects that anyway.

uweigand accepted this revision.Aug 17 2016, 10:21 AM
uweigand edited edge metadata.

My thinking is that if there's a hard coded register, then we should let it go through, flaws and all. So for the example of a user hard coding the register, we should be doing exactly what the user wants. If the user specifies r0 as the base/index register, then r0 should be what we use for that register, even if it's wrong. LLVM will throw an error when it detects that anyway.

OK, fair enough. Patch LGTM, thanks!

This revision is now accepted and ready to land.Aug 17 2016, 10:21 AM
This revision was automatically updated to reflect the committed changes.