[PATCH] D66524: [SVE][Inline-Asm] Add constraints for SVE predicate registers

Sander de Smalen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 21 05:55:37 PDT 2019


sdesmalen added inline comments.


================
Comment at: lib/IR/InlineAsm.cpp:188
+      unsigned char C = static_cast<unsigned char>(*I);
+      assert(isdigit(C) && "Not a single digit!");
+      int N = C - '0';
----------------
`"Expected a digit"` seems more appropriate, since this code is only testing a single character.


================
Comment at: lib/IR/InlineAsm.cpp:192
+      ++I;
+      pCodes->push_back(std::string(I, I+N));
+      I += N;
----------------
StringRef can be used here instead of std::string.


================
Comment at: lib/Target/AArch64/AArch64ISelLowering.cpp:5803
+  case 'U':
+    if (constraint[1] == 'p' && (constraint[2] == 'l' || constraint[2] == 'a'))
+      weight = CW_Register;
----------------
This is missing a check that `Constraint.size() == 3`. Or perhaps it is even better to split this code out into a separate function, because this patch writes out the condition three times. Maybe something like  `PredicateConstraint isPredicateConstraint(StringRef S)`, where `PredicateConstraint` is an enum.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66524





More information about the llvm-commits mailing list