[PATCH] D118428: [clang-cl] Support the /JMC flag

[Hans Wennborg via Phabricator via cfe-commits]

hans added inline comments.


================
Comment at: llvm/lib/CodeGen/JMCInstrumenter.cpp:180
+             "JMC instrument more than once?");
+      FunctionCallee Fn =
+          M.getOrInsertFunction(CheckFunctionName, getCheckFunctionType(Ctx));
----------------
might as well get the Function* already here?

```
CheckFunction = cast<Function>(M.getOrInsertFunction(...).getCallee());
```


================
Comment at: llvm/lib/CodeGen/TargetPassConfig.cpp:1096
+  if (TM->Options.JMCInstrument)
+    addPass(createJMCInstrumenterPass());
   addCodeGenPrepare();
----------------
could this be moved into addIRPasses()?


================
Comment at: llvm/test/CodeGen/X86/jmc-instrument.ll:2
+; Check that the flag symbol is not full-qualified.
+; RUN: llc < %s -enable-jmc-instrument | FileCheck %s
+
----------------
hans wrote:
> Since it's an IR pass, I think we don't need to have an llc test at all.
I still think it's unusual to have an llc test for an IR pass. Are there any other examples where we do something similar?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118428