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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits