[PATCH] D79249: [NOT FOR REVIEW] Experimental support for zero-or-trap behavior foruninitialized variables.
jdoerfert added a comment. FWIW, I tried to solve something similar the other day. My solution sketch looked like this: https://godbolt.org/z/bRQPjd The idea would be that we teach DSE (and others) to remove the `llvm.undef.init` intrinsic if the location is overwritten. In the example above only SROA kicks in properly. I guess this solution is more powerful though. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79249/new/ https://reviews.llvm.org/D79249 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D79249: [NOT FOR REVIEW] Experimental support for zero-or-trap behavior foruninitialized variables.
jfb added a comment.
Overall I like this approach.
I think it needs three more things to make it:
- Better size optimizations. I measured the code size impact on a codebase
which deploys variable auto-init already, and it's a 0.5% code size hit.
Considering that auto-init itself was 1%, it means the mitigation now costs 50%
more. I haven't dug into why the increase is such, and I assume that there's
low-lying fruits.
- I haven't measure performance impact. It might be zero.
- I think we'd need opt remarks support, because we'd want to audit all the
places where a trap is left behind.
Comment at: clang/lib/CodeGen/CGDecl.cpp:1166
+ (), llvm::Intrinsic::trapping, {V->getType()});
+ V = Builder.CreateCall(TrapFn, {V});
+}
Here you need something like:
```
auto *PointerTy = Ty->getPointerTo(Loc.getAddressSpace());
if (V->getType() != PointerTy)
Loc = Builder.CreateBitCast(Loc, PointerTy);
```
Because you can be storing a constant which is layout-compatible with the
location, but which doesn't have the same type (say, when we have an explicit
padding array of bytes). Line 1184 does that already for other code paths.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79249/new/
https://reviews.llvm.org/D79249
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D79249: [NOT FOR REVIEW] Experimental support for zero-or-trap behavior foruninitialized variables.
rsmith updated this revision to Diff 261511.
rsmith added a comment.
- The second operand of 'store' is the pointer, not the first.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79249/new/
https://reviews.llvm.org/D79249
Files:
clang/lib/CodeGen/CGDecl.cpp
llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
llvm/include/llvm/IR/Intrinsics.td
llvm/lib/CodeGen/CodeGenPrepare.cpp
llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
llvm/lib/Transforms/InstCombine/InstCombineInternal.h
llvm/lib/Transforms/Scalar/LowerConstantIntrinsics.cpp
Index: llvm/lib/Transforms/Scalar/LowerConstantIntrinsics.cpp
===
--- llvm/lib/Transforms/Scalar/LowerConstantIntrinsics.cpp
+++ llvm/lib/Transforms/Scalar/LowerConstantIntrinsics.cpp
@@ -6,8 +6,9 @@
//
//===--===//
//
-// This pass lowers all remaining 'objectsize' 'is.constant' intrinsic calls
-// and provides constant propagation and basic CFG cleanup on the result.
+// This pass lowers all remaining 'objectsize', 'is.constant', and 'trapping'
+// intrinsic calls and provides constant propagation and basic CFG cleanup on
+// the result.
//
//===--===//
@@ -39,6 +40,8 @@
"Number of 'is.constant' intrinsic calls handled");
STATISTIC(ObjectSizeIntrinsicsHandled,
"Number of 'objectsize' intrinsic calls handled");
+STATISTIC(TrappingIntrinsicsHandled,
+ "Number of 'trapping' intrinsic calls handled");
static Value *lowerIsConstantIntrinsic(IntrinsicInst *II) {
Value *Op = II->getOperand(0);
@@ -99,6 +102,7 @@
break;
case Intrinsic::is_constant:
case Intrinsic::objectsize:
+ case Intrinsic::trapping:
Worklist.push_back(WeakTrackingVH());
break;
}
@@ -125,6 +129,10 @@
NewValue = lowerObjectSizeCall(II, DL, TLI, true);
ObjectSizeIntrinsicsHandled++;
break;
+case Intrinsic::trapping:
+ NewValue = II->getOperand(0);
+ TrappingIntrinsicsHandled++;
+ break;
}
HasDeadBlocks |= replaceConditionalBranchesOnConstant(II, NewValue);
}
Index: llvm/lib/Transforms/InstCombine/InstCombineInternal.h
===
--- llvm/lib/Transforms/InstCombine/InstCombineInternal.h
+++ llvm/lib/Transforms/InstCombine/InstCombineInternal.h
@@ -635,6 +635,8 @@
Instruction *foldIntrinsicWithOverflowCommon(IntrinsicInst *II);
+ bool simplifyTrappingUse(Use );
+
public:
/// Inserts an instruction \p New before instruction \p Old
///
Index: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
===
--- llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -1846,6 +1846,80 @@
return nullptr;
}
+static bool canObservePoison(Use , Instruction *I) {
+ if (U.getOperandNo() == 0 &&
+ (isa(I) || isa(I) || isa(I)))
+return true;
+ if (auto *CB = dyn_cast(I))
+if (CB->getCalledOperandUse().getOperandNo() == U.getOperandNo())
+ return true;
+ if (auto *LI = dyn_cast(I))
+if (LI->getPointerOperandIndex() == U.getOperandNo())
+ return true;
+ if (auto *SI = dyn_cast(I))
+if (SI->getPointerOperandIndex() == U.getOperandNo())
+ return true;
+ if (U.getOperandNo() == 1 && I->isIntDivRem())
+return true;
+ // FIXME: More cases?
+ return false;
+}
+
+/// Simplify a use of @llvm.trapping.( %x)
+///
+/// Return true if we've performed a simplification and the use should be
+/// replaced by %x.
+bool InstCombiner::simplifyTrappingUse(Use ) {
+ auto *I = dyn_cast(U.getUser());
+
+ // We only care about trapping indicators on instructions.
+ if (!I)
+return true;
+
+ // FIXME: If all operands are trapping, we can convert to trapping(phi(...)).
+ if (isa(I))
+return false;
+
+ // The result of select is trapping if its condition is trapping.
+ if (isa(I) && U.getOperandNo() != 0)
+return false;
+
+ // trapping(trapping(x)) -> trapping(x).
+ if (auto *II = dyn_cast(I))
+if (II->getIntrinsicID() == Intrinsic::trapping)
+ return true;
+
+ // op(trapping(x)) -> @llvm.trap() where possible.
+ if (canObservePoison(U, I)) {
+CallInst *Trap = CallInst::Create(
+Intrinsic::getDeclaration(I->getModule(), Intrinsic::trap));
+InsertNewInstBefore(Trap, *I);
+return true;
+ }
+
+ // We can't propagate trapping to the result of a void-typed instruction and
+ // a few other cases.
+ if (I->getType()->isVoidTy() || I->getType()->isTokenTy() ||
+ I->isTerminator())
+return false;
+
+ // Don't
[PATCH] D79249: [NOT FOR REVIEW] Experimental support for zero-or-trap behavior foruninitialized variables.
rsmith created this revision.
Herald added subscribers: llvm-commits, cfe-commits, jdoerfert, hiraditya.
Herald added projects: clang, LLVM.
rsmith updated this revision to Diff 261511.
rsmith added a comment.
- The second operand of 'store' is the pointer, not the first.
Add @llvm.trapping.() intrinsic.
This is intended to produce either the given value or, if we can trace a
use of the value to an instruction with side-effects, a trap.
Notionally, it behaves as a nondeterministic choice between the given
value and poison, where the choice is made angelically such that the
given value is always chosen when the program does not reach a trap.
Simplify trapping(x) and convert it into traps where possible.
Update Clang CodeGen to produce trapping(x) in trivial auto var init
mode.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D79249
Files:
clang/lib/CodeGen/CGDecl.cpp
llvm/include/llvm/Analysis/TargetTransformInfoImpl.h
llvm/include/llvm/IR/Intrinsics.td
llvm/lib/CodeGen/CodeGenPrepare.cpp
llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
llvm/lib/Transforms/InstCombine/InstCombineInternal.h
llvm/lib/Transforms/Scalar/LowerConstantIntrinsics.cpp
Index: llvm/lib/Transforms/Scalar/LowerConstantIntrinsics.cpp
===
--- llvm/lib/Transforms/Scalar/LowerConstantIntrinsics.cpp
+++ llvm/lib/Transforms/Scalar/LowerConstantIntrinsics.cpp
@@ -6,8 +6,9 @@
//
//===--===//
//
-// This pass lowers all remaining 'objectsize' 'is.constant' intrinsic calls
-// and provides constant propagation and basic CFG cleanup on the result.
+// This pass lowers all remaining 'objectsize', 'is.constant', and 'trapping'
+// intrinsic calls and provides constant propagation and basic CFG cleanup on
+// the result.
//
//===--===//
@@ -39,6 +40,8 @@
"Number of 'is.constant' intrinsic calls handled");
STATISTIC(ObjectSizeIntrinsicsHandled,
"Number of 'objectsize' intrinsic calls handled");
+STATISTIC(TrappingIntrinsicsHandled,
+ "Number of 'trapping' intrinsic calls handled");
static Value *lowerIsConstantIntrinsic(IntrinsicInst *II) {
Value *Op = II->getOperand(0);
@@ -99,6 +102,7 @@
break;
case Intrinsic::is_constant:
case Intrinsic::objectsize:
+ case Intrinsic::trapping:
Worklist.push_back(WeakTrackingVH());
break;
}
@@ -125,6 +129,10 @@
NewValue = lowerObjectSizeCall(II, DL, TLI, true);
ObjectSizeIntrinsicsHandled++;
break;
+case Intrinsic::trapping:
+ NewValue = II->getOperand(0);
+ TrappingIntrinsicsHandled++;
+ break;
}
HasDeadBlocks |= replaceConditionalBranchesOnConstant(II, NewValue);
}
Index: llvm/lib/Transforms/InstCombine/InstCombineInternal.h
===
--- llvm/lib/Transforms/InstCombine/InstCombineInternal.h
+++ llvm/lib/Transforms/InstCombine/InstCombineInternal.h
@@ -635,6 +635,8 @@
Instruction *foldIntrinsicWithOverflowCommon(IntrinsicInst *II);
+ bool simplifyTrappingUse(Use );
+
public:
/// Inserts an instruction \p New before instruction \p Old
///
Index: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
===
--- llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
+++ llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
@@ -1846,6 +1846,80 @@
return nullptr;
}
+static bool canObservePoison(Use , Instruction *I) {
+ if (U.getOperandNo() == 0 &&
+ (isa(I) || isa(I) || isa(I)))
+return true;
+ if (auto *CB = dyn_cast(I))
+if (CB->getCalledOperandUse().getOperandNo() == U.getOperandNo())
+ return true;
+ if (auto *LI = dyn_cast(I))
+if (LI->getPointerOperandIndex() == U.getOperandNo())
+ return true;
+ if (auto *SI = dyn_cast(I))
+if (SI->getPointerOperandIndex() == U.getOperandNo())
+ return true;
+ if (U.getOperandNo() == 1 && I->isIntDivRem())
+return true;
+ // FIXME: More cases?
+ return false;
+}
+
+/// Simplify a use of @llvm.trapping.( %x)
+///
+/// Return true if we've performed a simplification and the use should be
+/// replaced by %x.
+bool InstCombiner::simplifyTrappingUse(Use ) {
+ auto *I = dyn_cast(U.getUser());
+
+ // We only care about trapping indicators on instructions.
+ if (!I)
+return true;
+
+ // FIXME: If all operands are trapping, we can convert to trapping(phi(...)).
+ if (isa(I))
+return false;
+
+ // The result of select is trapping if its condition is trapping.
+ if (isa(I) && U.getOperandNo() != 0)
+return false;
+
+ // trapping(trapping(x)) ->
