[PATCH] D133949: Make sure the right parameter extension attributes are added in various instrumentation passes.

[Jonas Paulsson via Phabricator via llvm-commits]

jonpa added inline comments.


================
Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:344
+  // Add AS to FnAS while taking special care with integer extensions.
+  auto addAttrSet = [&](AttributeSet &FnAS, const AttributeSet &AS,
+                        bool Param = true) -> void {
----------------
efriedma wrote:
> Can we mess with the way this works so that the "attributes" from OMPKinds.def aren't directly LLVM IR attributes?  I think the current version has the right semantics, but using zext/sext as "placeholder" attributes is really confusing.  (Maybe instead of writing AttributeSet constructors in OMPKinds.def, make the names from OMP_ATTRS_SET into an enum, and put the actual code to translate from those enums to LLVM IR attributes here.)
It says at the top of OMPKinds.def "This file is under transition to OMP.td with TableGen code generation.", so perhaps we could follow the current use of AttributeSet constructors in this patch for now..?

Personally, I haven't thought about this as a problem, even though the whole #include scheme isn't exactly super readable to me. Would probably be easier to work with using TableGen... 




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

https://reviews.llvm.org/D133949