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

Eli Friedman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 23 12:27:44 PDT 2022


efriedma added a comment.

> Don't know how to handle OpenMP/OMPIRBuilder.cpp as there is no TargetLibraryInfo available

Maybe split the computation out of TargetLibraryInfo, so we can use it without pulling in the entire analysis pass?  The computation originated there because it's mostly needed by TargetLibraryInfo itself, but the ShouldExtI32Param/ShouldExtI32Return/ShouldSignExtI32Param bits aren't really tied to the rest of the computations done in TargetLibraryInfo.

Or we could just make the users of OMPIRBuilder construct a TargetLibraryInfo and pass it in, I guess.

> At second thought, I think that would not work. The clang front end would add the attributes when building them from the .cpp files so the instrumented declarations really must hold them as well, right?

Yes, we're calling functions written in C++, so we need to follow the C/C++ calling convention.

------

getOrInsertFunction() is, in general, not guaranteed to return a Function.  If opaque pointer types are enabled, it can't return a bitcast like it could before, but it could return function with the wrong number of arguments, or a global variable.  Ideally, this shouldn't happen, but I wouldn't trust users not to do something weird.

It's probably more clear to pass in the attribute list like GCOVProfiling does.  (In theory, we should then set attributes on the call, but I'm okay leaving that part out, at least for now.)


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

https://reviews.llvm.org/D133949



More information about the llvm-commits mailing list