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

Tue Sep 13 03:41:01 PDT 2022

domada marked an inline comment as done.
domada added inline comments.

================
Comment at: llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h:629
+                 llvm::ArrayRef<llvm::Value *> AlignedVars,
+                 llvm::Value *Alignment, Value *IfCond, ConstantInt *Simdlen,
                  ConstantInt *Safelen);
----------------
jdoerfert wrote:
> 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?
You are right. It is possible to specify multiple align clauses for single omp clause (for example:  #pragma omp simd aligned(X:32) aligned(Y:64)

================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:2976

+  const int DefaultAlignment = 16;
+
----------------
jdoerfert wrote:
> This doesn't work. Use the data layout for any default values please.
I have used pointer ABI alignment as the default value. Is it ok?
Clang has separate method to calculate OpenMP default alignment defined in TargetInfo class ( [[ https://clang.llvm.org/doxygen/classclang_1_1TargetInfo.html#af0c75e3288adc13601732329c44db147 |  link ]])
Unfortunately, there is no simple replacement of this function in OMPIRBuilder or in Flang driver. There are some requests to expose TargetInfo data in Clang-independent manner: [[ https://discourse.llvm.org/t/rfc-targetinfo-library/64342 | RFC1 ]], [[https://discourse.llvm.org/t/complex-to-libm-conversion-abi-issues/65131 |RFC2]] , but this issue is not solved.

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

https://reviews.llvm.org/D133578