[PATCH] D146466: [clang] diagnose function fallthrough
Nick Desaulniers via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Mar 23 11:46:07 PDT 2023
nickdesaulniers added a comment.
In D146466#4217363 <https://reviews.llvm.org/D146466#4217363>, @efriedma wrote:
> There are limits to how much we can do by just changing the code clang generates... but the two particular cases you mention probably could be "fixed" by messing with the IR generated by clang. Sure, that probably makes sense to pursue. (If you're going to pick an arbitrary value, zero is probably a better choice than "freeze poison".)
I don't think that could be a clang codegen of IR change. What I had in mind is closer to:
diff --git a/llvm/lib/Transforms/Utils/LoopUnroll.cpp b/llvm/lib/Transforms/Utils/LoopUnroll.cpp
index 916c02692823..aa1f288b51ee 100644
--- a/llvm/lib/Transforms/Utils/LoopUnroll.cpp
+++ b/llvm/lib/Transforms/Utils/LoopUnroll.cpp
@@ -236,9 +236,17 @@ void llvm::simplifyLoopAfterUnroll(Loop *L, bool SimplifyIVs, LoopInfo *LI,
SmallVector<WeakTrackingVH, 16> DeadInsts;
for (BasicBlock *BB : L->getBlocks()) {
for (Instruction &Inst : llvm::make_early_inc_range(*BB)) {
- if (Value *V = simplifyInstruction(&Inst, {DL, nullptr, DT, AC}))
- if (LI->replacementPreservesLCSSAForm(&Inst, V))
+ if (Value *V = simplifyInstruction(&Inst, {DL, nullptr, DT, AC})) {
+ // TODO: check if sanitizer is enabled
+ if (isa<PoisonValue>(V) && isa<LoadInst>(Inst)) {
+ errs() << "XXX: found poison (replacing a load)!\n";
+ FreezeInst *FI = new FreezeInst(V);
+ FI->insertInto(BB, Inst.getIterator());
+ if (LI->replacementPreservesLCSSAForm(FI, V))
+ Inst.replaceAllUsesWith(FI);
+ } else if (LI->replacementPreservesLCSSAForm(&Inst, V))
Inst.replaceAllUsesWith(V);
+ }
if (isInstructionTriviallyDead(&Inst))
DeadInsts.emplace_back(&Inst);
}
Which seems to do exactly what I want (minus only doing so when `-fsanitize=array-bounds` is enabled; not sure we leave such breadcrumbs behind in IR; still looking to see if we can tell in LLVM when specific sanitizers are enabled)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146466/new/
https://reviews.llvm.org/D146466
More information about the cfe-commits
mailing list