rnk added inline comments.
Herald added a project: All.
================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5372
+ // This controls whether or not we perform JustMyCode instrumentation.
+ if (TC.getTriple().isOSBinFormatELF() && Args.hasArg(options::OPT_fjmc)) {
+ if (DebugInfoKind >= codegenoptions::DebugInfoConstructor)
----------------
Generally all flags have an fno- variant. Normally, this would be
`hasFlag(OPT_fjmc, OPT_fno_jmc, false)`, which handles the behavior of making
the last flag win. Any reason not to set that up?
================
Comment at: llvm/lib/CodeGen/JMCInstrumenter.cpp:177
+ DefaultCheckFunc->setName(CheckFunctionName);
+ DefaultCheckFunc->setLinkage(GlobalValue::WeakODRLinkage);
+ CheckFunction = DefaultCheckFunc;
----------------
This is a bit pedantic, but the idea is that this weak function will be
overridden with a strong function, right? Technically, for that use case,
`weak` linkage is the correct linkage. Because the JMC pass runs late after
inlining, it is unlikely that this will ever cause issues in practice, but I
think it expresses the intention better to use the linkage that matches the use
case.
ODR linkage is supposed to indicate that all definitions must be functionally
equivalent. It just turns out that the only real power that grants the
optimizer is the ability to inline.
================
Comment at: llvm/test/Instrumentation/JustMyCode/jmc-instrument-elf.ll:1
-; REQUIRES: system-windows
-; RUN: opt -jmc-instrument -mtriple=x86_64-pc-windows-msvc -S < %s |
FileCheck %s
-; RUN: opt -jmc-instrument -mtriple=aarch64-pc-windows-msvc -S < %s |
FileCheck %s
-; RUN: opt -jmc-instrument -mtriple=arm-pc-windows-msvc -S < %s |
FileCheck %s
+; REQUIRES: system-linux
+; RUN: opt -jmc-instrument -mtriple=x86_64-unknown-linux-gnu -S < %s |
FileCheck %s
----------------
Please remove the REQUIRES line, this test should pass on Windows, and the
other test should pass on Linux as well. Neither of these tests depend on
anything from the system.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D119910/new/
https://reviews.llvm.org/D119910
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
