[PATCH] D65947: [RISCV] Allow ABI Names in Inline Assembly Constraints

Sam Elliott via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 8 07:01:58 PDT 2019


lenary marked 2 inline comments as done.
lenary added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:2547
+            .Cases("{f31}", "{ft11}", {RISCV::F31_32, RISCV::F31_64})
             .Default({-1U, -1U});
     if (FReg.first != -1U)
----------------
simoncook wrote:
> 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.
Sure, will do.


================
Comment at: llvm/test/CodeGen/RISCV/inline-asm-abi-names.ll:115
+
+define i32 @explicit_register_x2(i32 %a) nounwind {
+; RV32I-LABEL: explicit_register_x2:
----------------
simoncook wrote:
> Given other tests in the files added by this have comments above them, should these also have comments?
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).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65947/new/

https://reviews.llvm.org/D65947





More information about the llvm-commits mailing list