[llvm] r296398 - Revert r296366 "[InlineFunction] add nonnull assumptions based on argument attributes"
Hans Wennborg via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 27 14:33:02 PST 2017
Author: hans
Date: Mon Feb 27 16:33:02 2017
New Revision: 296398
URL: http://llvm.org/viewvc/llvm-project?rev=296398&view=rev
Log:
Revert r296366 "[InlineFunction] add nonnull assumptions based on argument attributes"
It causes miscompiles e.g. during self-host of Clang (PR32082).
Modified:
llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp
llvm/trunk/test/Transforms/Inline/arg-attr-propagation.ll
Modified: llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp?rev=296398&r1=296397&r2=296398&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/InlineFunction.cpp Mon Feb 27 16:33:02 2017
@@ -1093,52 +1093,38 @@ static void AddAliasScopeMetadata(CallSi
}
}
-/// Add @llvm.assume-based assumptions to preserve information supplied by
-/// argument attributes because the attributes will disappear after inlining.
-static void addAssumptions(CallSite CS, InlineFunctionInfo &IFI) {
- if (!IFI.GetAssumptionCache)
+/// If the inlined function has non-byval align arguments, then
+/// add @llvm.assume-based alignment assumptions to preserve this information.
+static void AddAlignmentAssumptions(CallSite CS, InlineFunctionInfo &IFI) {
+ if (!PreserveAlignmentAssumptions || !IFI.GetAssumptionCache)
return;
AssumptionCache *AC = &(*IFI.GetAssumptionCache)(*CS.getCaller());
auto &DL = CS.getCaller()->getParent()->getDataLayout();
- // To avoid inserting redundant assumptions, check that an assumption provides
- // new information in the caller. This might require a dominator tree.
+ // To avoid inserting redundant assumptions, we should check for assumptions
+ // already in the caller. To do this, we might need a DT of the caller.
DominatorTree DT;
bool DTCalculated = false;
- auto calcDomTreeIfNeeded = [&]() {
- if (!DTCalculated) {
- DT.recalculate(*CS.getCaller());
- DTCalculated = true;
- }
- };
Function *CalledFunc = CS.getCalledFunction();
- IRBuilder<> Builder(CS.getInstruction());
for (Argument &Arg : CalledFunc->args()) {
- Value *ArgVal = CS.getArgument(Arg.getArgNo());
-
unsigned Align = Arg.getType()->isPointerTy() ? Arg.getParamAlignment() : 0;
- if (PreserveAlignmentAssumptions && Align &&
- !Arg.hasByValOrInAllocaAttr() && !Arg.hasNUses(0)) {
+ if (Align && !Arg.hasByValOrInAllocaAttr() && !Arg.hasNUses(0)) {
+ if (!DTCalculated) {
+ DT.recalculate(*CS.getCaller());
+ DTCalculated = true;
+ }
+
// If we can already prove the asserted alignment in the context of the
// caller, then don't bother inserting the assumption.
- calcDomTreeIfNeeded();
- if (getKnownAlignment(ArgVal, DL, CS.getInstruction(), AC, &DT) < Align) {
- CallInst *Asmp = Builder.CreateAlignmentAssumption(DL, ArgVal, Align);
- AC->registerAssumption(Asmp);
- }
- }
+ Value *ArgVal = CS.getArgument(Arg.getArgNo());
+ if (getKnownAlignment(ArgVal, DL, CS.getInstruction(), AC, &DT) >= Align)
+ continue;
- if (Arg.hasNonNullAttr()) {
- // If we can already prove nonnull in the context of the caller, then
- // don't bother inserting the assumption.
- calcDomTreeIfNeeded();
- if (!isKnownNonNullAt(ArgVal, CS.getInstruction(), &DT)) {
- Value *NotNull = Builder.CreateIsNotNull(ArgVal);
- CallInst *Asmp = Builder.CreateAssumption(NotNull);
- AC->registerAssumption(Asmp);
- }
+ CallInst *NewAsmp = IRBuilder<>(CS.getInstruction())
+ .CreateAlignmentAssumption(DL, ArgVal, Align);
+ AC->registerAssumption(NewAsmp);
}
}
}
@@ -1635,10 +1621,10 @@ bool llvm::InlineFunction(CallSite CS, I
VMap[&*I] = ActualArg;
}
- // Add assumptions if necessary. We do this before the inlined instructions
- // are actually cloned into the caller so that we can easily check what will
- // be known at the start of the inlined code.
- addAssumptions(CS, IFI);
+ // Add alignment assumptions if necessary. We do this before the inlined
+ // instructions are actually cloned into the caller so that we can easily
+ // check what will be known at the start of the inlined code.
+ AddAlignmentAssumptions(CS, IFI);
// We want the inliner to prune the code as it copies. We would LOVE to
// have no dead or constant instructions leftover after inlining occurs
Modified: llvm/trunk/test/Transforms/Inline/arg-attr-propagation.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Inline/arg-attr-propagation.ll?rev=296398&r1=296397&r2=296398&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/Inline/arg-attr-propagation.ll (original)
+++ llvm/trunk/test/Transforms/Inline/arg-attr-propagation.ll Mon Feb 27 16:33:02 2017
@@ -12,13 +12,11 @@ define i32 @callee(i32* dereferenceable(
ret i32 %t2
}
-; Add a nonnull assumption.
+; FIXME: All dereferenceability information is lost.
; The caller argument could be known nonnull and dereferenceable(32).
define i32 @caller1(i32* %t1) {
; CHECK-LABEL: @caller1(i32* %t1)
-; CHECK-NEXT: [[TMP1:%.*]] = icmp ne i32* %t1, null
-; CHECK-NEXT: call void @llvm.assume(i1 [[TMP1]])
; CHECK-NEXT: [[T2_I:%.*]] = load i32, i32* %t1
; CHECK-NEXT: ret i32 [[T2_I]]
;
@@ -26,7 +24,6 @@ define i32 @caller1(i32* %t1) {
ret i32 %t2
}
-; Don't add a nonnull assumption if it's redundant.
; The caller argument is nonnull, but that can be explicit.
; The dereferenceable amount could be increased.
@@ -39,7 +36,6 @@ define i32 @caller2(i32* dereferenceable
ret i32 %t2
}
-; Don't add a nonnull assumption if it's redundant.
; The caller argument is nonnull, but that can be explicit.
; Make sure that we don't propagate a smaller dereferenceable amount.
More information about the llvm-commits
mailing list