[PATCH] D133578: [OpenMP] Add generation of SIMD align assumptions to OMPIRBuilder

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

Thanks for this patch! I have two drive by comments that should probably be addressed first.



================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:629
+                 llvm::ArrayRef<llvm::Value *> AlignedVars,
+                 llvm::Value *Alignment, Value *IfCond, ConstantInt *Simdlen,
                  ConstantInt *Safelen);
----------------
Aren't there different alignments possible, so X is aligned 32 and Y is aligned 64? If so, should we tie the Value and Alignment together in the API?


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:2976
 
+  const int DefaultAlignment = 16;
+
----------------
This doesn't work. Use the data layout for any default values please.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133578