jdoerfert added inline comments.
================ Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:2976 + const int DefaultAlignment = 16; + ---------------- domada wrote: > 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. > I was thinking that OMPIRBuilder should pick the default value if the user > specifies align(Y). Thanks for clarification. That works with me, but you need the actual type. So the Value is not sufficient, you need `&X and double` to ask DataLayout for the preferred type. For now you can use the clang functionality or pass the type and use DL. > I assume that if the user tells us nothing we should not annotate anything. Agreed. 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
