rnk added inline comments.
================ Comment at: llvm/lib/CodeGen/JMCInstrumenter.cpp:145 + LLVMContext &Ctx = M.getContext(); + bool UseX86FastCall = Triple(M.getTargetTriple()).getArch() == Triple::x86; + ---------------- ychen wrote: > ychen wrote: > > hans wrote: > > > I still worry a bit about the target-specific code here. Normally, IR > > > passes don't have any target-specific knowledge, but ask classes such as > > > TargetTransformInfo for target-specific details, or possibly take them as > > > input to the pass. For example, see > > > llvm/lib/Transforms/CFGuard/CFGuard.cpp > > > > > > I'm also not sure that lib/CodeGen/ is the right place for this pass, > > > since most files there seem to be machine-IR passes. Maybe the natural > > > place for this would be lib/Transforms/Instrumentation/? Is there some > > > good pass we can compare this with? > > > I still worry a bit about the target-specific code here. Normally, IR > > > passes don't have any target-specific knowledge, but ask classes such as > > > TargetTransformInfo for target-specific details, or possibly take them as > > > input to the pass. For example, see > > > llvm/lib/Transforms/CFGuard/CFGuard.cpp > > Understood. `TargetTransformInfo` is mostly for the "IR optimization > > passes". The JMC pass is "IR codegen passes", it is more similar to > > `CodeGenPrepare` pass than any "IR optimization passes". I think we could > > move the target-specific stuff into the `TargetPassConfig` & its derived > > classes, not in this patch, but the following ELF port one. WDYT? > > > I still worry a bit about the target-specific code here. Normally, IR > > > passes don't have any target-specific knowledge, but ask classes such as > > > TargetTransformInfo for target-specific details, or possibly take them as > > > input to the pass. For example, see > > > llvm/lib/Transforms/CFGuard/CFGuard.cpp > > Understood. `TargetTransformInfo` is mostly for the "IR optimization > > passes". The JMC pass is "IR codegen passes", it is more similar to > > `CodeGenPrepare` pass than any "IR optimization passes". I think we could > > move the target-specific stuff into the `TargetPassConfig` & its derived > > classes, not in this patch, but the following ELF port one. WDYT? > > Scratch that. I think this is more OS/platform-specific than target-specific. > For X86, MSVC COFF and ELF are likely to have different symbol mangling and > section naming preferences. And this information is pretty specific to JMC, > like section name '.msvcjmc'. I think only X86 COFF has this `weird` mangling > happen in LLVM codegen instead of the frontend. For non-x86 COFF and ELF, the > handling is pretty much the same. So it may not be worth the effort of > putting small pieces of OS/platform-specific information elsewhere? I think the existing PGO passes do a variety of target-specific things, and they live in lib/Transforms/Instrumentation. For example, they pick different section names for ELF and COFF. This seems like the function entry/exit instrumentation, and I wonder if it should be added as part of the CodeGenPassBuilder.h list of passes. 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
