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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
