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

[Dominik Adamski via Phabricator via cfe-commits]

domada added inline comments.


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:2976
 
+  const int DefaultAlignment = 16;
+
----------------
jdoerfert wrote:
> domada wrote:
> > 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.
> Let's step back:
> 1) User tells us the alignment via `aligned(X:64)`. This should populate the map in Clang, I would assume.
> 2) User tells us the alignment "implicitly" via `align(Y)`. Again, clang should populate the map with the default/preferred alignment of the type. You can use the function you linked, or use the data layout API to get the preferred alignment.
> 3) User tells us nothing. We should pick the default or, don't annotate anything. If we go with defaults, you need to look up the default for the address space but I don't know why we would annotate anything not provided by the user.
> 
> 
Ad 1) Agree
Ad 2) OK, I will add assertion to ensure that the map is correctly populated with the default/preferred alignment if the user specifies only: `align(Y)`.

I was thinking that OMPIRBuilder should pick the default value if the user specifies `align(Y)`. Thanks for clarification. 

Ad 3)  I assume that if the user tells us nothing we should not annotate anything. I will expect empty map and no generated annotations. Currently we do not generate any annotations if there is no align clause.


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

https://reviews.llvm.org/D133578