This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Allow ABI Names in Inline Assembly Constraints
ClosedPublic

Authored by lenary on Aug 8 2019, 6:24 AM.

Details

Summary

Clang will replace references to registers using ABI names in inline
assembly constraints with references to architecture names, but other
frontends do not. LLVM uses the regular assembly parser to parse inline asm,
so inline assembly strings can contain references to registers using their
ABI names.

This patch adds support for parsing constraints using either the ABI name or
the architectural register name. This means we do not need to implement the
ABI name replacement code in every single frontend, especially those like
Rust which are a very thin shim on top of LLVM IR's inline asm, and that
constraints can more closely match the assembly strings they refer to.

Diff Detail

Repository
rL LLVM

Event Timeline

lenary created this revision.Aug 8 2019, 6:24 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 8 2019, 6:24 AM
lenary edited the summary of this revision. (Show Details)Aug 8 2019, 6:25 AM

Other than my small suggestion about the use of NoRegister, this looks good to me.

llvm/lib/Target/RISCV/RISCVISelLowering.cpp
2547 ↗(On Diff #214133)

Given the above block uses RISCV::NoRegister as the default/no match case, it probably makes sense to update this one to use the same.

llvm/test/CodeGen/RISCV/inline-asm-abi-names.ll
115 ↗(On Diff #214133)

Given other tests in the files added by this have comments above them, should these also have comments?

lenary marked 2 inline comments as done.Aug 8 2019, 7:00 AM
lenary added inline comments.
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
2547 ↗(On Diff #214133)

Sure, will do.

llvm/test/CodeGen/RISCV/inline-asm-abi-names.ll
115 ↗(On Diff #214133)

The notes are to explain why the assembly is more complex than just "load the argument into the requested register, expand the inline assembly, and then return". This is so that future reviewers can more easily understand the assembly, should future functionality change exactly what is emitted before and after the inline assembly.

In the case of x0, this is because you can only pass one value using that register.

In the case of other functions with notes, it is because that register needs to be saved (either because it is callee-saved, or because it's gp or tp which we treat as effectively callee-saved).

lenary updated this revision to Diff 214146.Aug 8 2019, 7:05 AM
lenary edited the summary of this revision. (Show Details)

Address review feedback

  • Use RISCV::NoRegister in FReg StringSwitch
lenary marked an inline comment as done.Aug 8 2019, 7:12 AM
simoncook accepted this revision.Aug 8 2019, 7:30 AM

In that case LGTM

This revision is now accepted and ready to land.Aug 8 2019, 7:30 AM
This revision was automatically updated to reflect the committed changes.