[PATCH] D66302: [SVE][Inline-Asm] Support for SVE asm operands

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen added inline comments.


================
Comment at: lib/Target/AArch64/AArch64AsmPrinter.cpp:618
 
+    bool hasAltName;
+    const TargetRegisterClass *RegClass;
----------------
The use of `hasAltName` is confusing me here. When I look at the declaration and definition of `printAsmRegInClass`, the parameter is called `bool isVector`, and a SVE vector is still a vector, so passing `false` is odd. I think it makes more sense to change `printAsmRegInClass` to accept an `unsigned AltName`, and just pass `AArch64::vreg` for Neon Vectors directly (or `AArch64::NoRegAltName` otherwise).


================
Comment at: test/CodeGen/AArch64/aarch64-sve-asm-negative.ll:2
+; RUN: not llc -mtriple aarch64-none-linux-gnu -mattr=+neon -o %t.s -filetype=asm %s 2>&1 | FileCheck %s
+
+; Function Attrs: nounwind readnone
----------------
Can you add a comment explaining what this is testing (and why the inline asm below is not valid)?


================
Comment at: test/CodeGen/AArch64/aarch64-sve-asm.ll:46
+
+!0 = !{i32 188, i32 210}
----------------
nit: what is this line doing here?


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

https://reviews.llvm.org/D66302