Details
Diff Detail
Event Timeline
lib/IR/InlineAsm.cpp | ||
---|---|---|
188 | "Expected a digit" seems more appropriate, since this code is only testing a single character. | |
192 | StringRef can be used here instead of std::string. | |
lib/Target/AArch64/AArch64ISelLowering.cpp | ||
5803 | 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. |
- Added isPredicateConstraint function to AArch64ISelLowering.cpp, which returns Upl, Upa or Invalid. This is used to replace some repeated checks of the predicate type
- Minor changes to InlineAsm.cpp
Just some drive-by suggestions :)
lib/Target/AArch64/AArch64ISelLowering.cpp | ||
---|---|---|
5744 | Nit: I think get- or parsePredicateConstraint reads better, since this doesn't return a simple yes/no answer. | |
test/CodeGen/AArch64/aarch64-sve-asm.ll | ||
50 | Nit: I would be a bit pedantic here and also check that they are used for the inline asm. |
- Renamed the isPredicateConstraint function to parsePredicateConstraint
- Added more thorough checks to the tests in aarch64-sve-asm.ll
docs/LangRef.rst | ||
---|---|---|
3818 | The length of the constraint depends on the first letter, and for AArch64 'U' was chosen to indicate a 3-letter constraint. These are not in the SVE architecture manual as they are only for compatibility with GCC inline asm (see https://gcc.gnu.org/onlinedocs/gcc/Machine-Constraints.html#Machine-Constraints) | |
lib/Target/AArch64/AArch64ISelLowering.cpp | ||
5744 | Thanks for the suggestion @rovka - I have changed this to parsePredicateConstraint |
What do these names mean? "<something> predicate lower|all?" I see they are the names used in gcc, so I guess it makes sense to use them here. Are these names used in the SVE architecture manual somewhere? I cannot find them.