[PATCH] D50016: IR: Add entry/exit instrumentation symbols to the libcall list.

Peter Collingbourne via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 14 14:30:51 PST 2019


pcc marked an inline comment as done.
pcc added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp:23
                        Instruction *InsertionPt, DebugLoc DL) {
+  // If you add code to recognize a new instrumentation function here, the
+  // function also needs to be added to include/llvm/IR/RuntimeLibcalls.def
----------------
pcc wrote:
> tejohnson wrote:
> > pcc wrote:
> > > tejohnson wrote:
> > > > Can you include RuntimeLibcalls.def and assert that the name is in that list?
> > > I could, but I don't think we need to do that unless there's evidence that people are ignoring the comment.
> > I'd prefer an assert, because I think it would be easy to miss the comment if the second if block is extended with another func name, or a subsequent new if block is added below it.
> How about just putting a comment in all the places where people are likely to add new code to this function (before ifs, at the end)?
I suppose another idea is to do something like
```
#define HANDLE_LIBCALL(name, value) const char LibCallName_##name[] = value;
#include "llvm/IR/RuntimeLibcalls.def"
```
in a header somewhere, and then change this code to use `LibCallName_FOO`.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D50016





More information about the llvm-commits mailing list