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

Simon Cook via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 8 06:48:27 PDT 2019


simoncook added a comment.

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



================
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)
----------------
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.


================
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:
----------------
Given other tests in the files added by this have comments above them, should these also have comments?


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