[PATCH] D76238: [SveEmitter] Implement builtins for contiguous loads/stores

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.


================
Comment at: clang/include/clang/Basic/AArch64SVETypeFlags.h:72
+  bool isStructStore() const { return Flags & IsStructStore; }
+  bool isZxtReturn() const { return Flags & IsZxtReturn; }
+
----------------
SjoerdMeijer wrote:
> nit: this one is non obvious (at least to me), so perhaps worth a comment what this is. I can guess that Zext means zero extending, but you know, the context...
"Zxt" is not a standard abbreviation in LLVM: please use "Zext", or "ZeroExtend".


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7471
+
+Value *CodeGenFunction::EmitSVEMaskedStore(llvm::Type *ReturnTy,
+                                           SmallVectorImpl<Value *> &Ops,
----------------
ReturnTy is unused?


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7475
+  llvm::PointerType *PTy = cast<llvm::PointerType>(Ops[1]->getType());
+  llvm::Type *MemEltTy = PTy->getPointerElementType();
+
----------------
Please avoid using getPointerElementType where possible... it will interfere with opaque pointer types work.  (You should be able to get the element type from the AST.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76238