[llvm] r277977 - [JumpThreading] Fix handling of aliasing metadata.
Eli Friedman via llvm-commits
llvm-commits at lists.llvm.org
Sun Aug 7 21:10:23 PDT 2016
Author: efriedma
Date: Sun Aug 7 23:10:22 2016
New Revision: 277977
URL: http://llvm.org/viewvc/llvm-project?rev=277977&view=rev
Log:
[JumpThreading] Fix handling of aliasing metadata.
Summary:
The correctness fix here is that when we CSE a load with another load,
we need to combine the metadata on the two loads. This matches the
behavior of other passes, like instcombine and GVN.
There's also a minor optimization improvement here: for load PRE, the
aliasing metadata on the inserted load should be the same as the
metadata on the original load. Not sure why the old code was throwing
it away.
Issue found by inspection.
Differential Revision: http://reviews.llvm.org/D21460
Modified:
llvm/trunk/include/llvm/Analysis/Loads.h
llvm/trunk/include/llvm/Transforms/Utils/Local.h
llvm/trunk/lib/Analysis/Loads.cpp
llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp
llvm/trunk/lib/Transforms/Utils/Local.cpp
llvm/trunk/test/Transforms/JumpThreading/thread-loads.ll
Modified: llvm/trunk/include/llvm/Analysis/Loads.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/Loads.h?rev=277977&r1=277976&r2=277977&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Analysis/Loads.h (original)
+++ llvm/trunk/include/llvm/Analysis/Loads.h Sun Aug 7 23:10:22 2016
@@ -71,7 +71,7 @@ extern cl::opt<unsigned> DefMaxInstsToSc
/// the only relevant load gets deleted.)
///
/// \param Load The load we want to replace.
-/// \param ScanBB The basic block to scan. FIXME: This is redundant.
+/// \param ScanBB The basic block to scan.
/// \param [in,out] ScanFrom The location to start scanning from. When this
/// function returns, it points at the last instruction scanned.
/// \param MaxInstsToScan The maximum number of instructions to scan. If this
@@ -89,7 +89,6 @@ Value *FindAvailableLoadedValue(LoadInst
BasicBlock::iterator &ScanFrom,
unsigned MaxInstsToScan = DefMaxInstsToScan,
AliasAnalysis *AA = nullptr,
- AAMDNodes *AATags = nullptr,
bool *IsLoadCSE = nullptr);
}
Modified: llvm/trunk/include/llvm/Transforms/Utils/Local.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Transforms/Utils/Local.h?rev=277977&r1=277976&r2=277977&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Transforms/Utils/Local.h (original)
+++ llvm/trunk/include/llvm/Transforms/Utils/Local.h Sun Aug 7 23:10:22 2016
@@ -316,6 +316,12 @@ bool removeUnreachableBlocks(Function &F
/// Metadata not listed as known via KnownIDs is removed
void combineMetadata(Instruction *K, const Instruction *J, ArrayRef<unsigned> KnownIDs);
+/// Combine the metadata of two instructions so that K can replace J. This
+/// specifically handles the case of CSE-like transformations.
+///
+/// Unknown metadata is removed.
+void combineMetadataForCSE(Instruction *K, const Instruction *J);
+
/// Replace each use of 'From' with 'To' if that use is dominated by
/// the given edge. Returns the number of replacements made.
unsigned replaceDominatedUsesWith(Value *From, Value *To, DominatorTree &DT,
Modified: llvm/trunk/lib/Analysis/Loads.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/Loads.cpp?rev=277977&r1=277976&r2=277977&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/Loads.cpp (original)
+++ llvm/trunk/lib/Analysis/Loads.cpp Sun Aug 7 23:10:22 2016
@@ -302,11 +302,11 @@ llvm::DefMaxInstsToScan("available-load-
"to scan backward from a given instruction, when searching for "
"available loaded value"));
-Value *llvm::FindAvailableLoadedValue(LoadInst *Load, BasicBlock *ScanBB,
+Value *llvm::FindAvailableLoadedValue(LoadInst *Load,
+ BasicBlock *ScanBB,
BasicBlock::iterator &ScanFrom,
unsigned MaxInstsToScan,
- AliasAnalysis *AA, AAMDNodes *AATags,
- bool *IsLoadCSE) {
+ AliasAnalysis *AA, bool *IsLoadCSE) {
if (MaxInstsToScan == 0)
MaxInstsToScan = ~0U;
@@ -356,8 +356,6 @@ Value *llvm::FindAvailableLoadedValue(Lo
if (LI->isAtomic() < Load->isAtomic())
return nullptr;
- if (AATags)
- LI->getAAMetadata(*AATags);
if (IsLoadCSE)
*IsLoadCSE = true;
return LI;
@@ -377,8 +375,8 @@ Value *llvm::FindAvailableLoadedValue(Lo
if (SI->isAtomic() < Load->isAtomic())
return nullptr;
- if (AATags)
- SI->getAAMetadata(*AATags);
+ if (IsLoadCSE)
+ *IsLoadCSE = false;
return SI->getOperand(0);
}
Modified: llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp?rev=277977&r1=277976&r2=277977&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp (original)
+++ llvm/trunk/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp Sun Aug 7 23:10:22 2016
@@ -819,11 +819,10 @@ Instruction *InstCombiner::visitLoadInst
// where there are several consecutive memory accesses to the same location,
// separated by a few arithmetic operations.
BasicBlock::iterator BBI(LI);
- AAMDNodes AATags;
bool IsLoadCSE = false;
if (Value *AvailableVal =
FindAvailableLoadedValue(&LI, LI.getParent(), BBI,
- DefMaxInstsToScan, AA, &AATags, &IsLoadCSE)) {
+ DefMaxInstsToScan, AA, &IsLoadCSE)) {
if (IsLoadCSE) {
LoadInst *NLI = cast<LoadInst>(AvailableVal);
unsigned KnownIDs[] = {
Modified: llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp?rev=277977&r1=277976&r2=277977&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/JumpThreading.cpp Sun Aug 7 23:10:22 2016
@@ -951,12 +951,17 @@ bool JumpThreadingPass::SimplifyPartiall
// Scan a few instructions up from the load, to see if it is obviously live at
// the entry to its block.
BasicBlock::iterator BBIt(LI);
-
+ bool IsLoadCSE;
if (Value *AvailableVal =
- FindAvailableLoadedValue(LI, LoadBB, BBIt, DefMaxInstsToScan)) {
+ FindAvailableLoadedValue(LI, LoadBB, BBIt, DefMaxInstsToScan, nullptr, &IsLoadCSE)) {
// If the value of the load is locally available within the block, just use
// it. This frequently occurs for reg2mem'd allocas.
+ if (IsLoadCSE) {
+ LoadInst *NLI = cast<LoadInst>(AvailableVal);
+ combineMetadataForCSE(NLI, LI);
+ };
+
// If the returned value is the load itself, replace with an undef. This can
// only happen in dead loops.
if (AvailableVal == LI) AvailableVal = UndefValue::get(LI->getType());
@@ -983,6 +988,7 @@ bool JumpThreadingPass::SimplifyPartiall
typedef SmallVector<std::pair<BasicBlock*, Value*>, 8> AvailablePredsTy;
AvailablePredsTy AvailablePreds;
BasicBlock *OneUnavailablePred = nullptr;
+ SmallVector<LoadInst*, 8> CSELoads;
// If we got here, the loaded value is transparent through to the start of the
// block. Check to see if it is available in any of the predecessor blocks.
@@ -993,17 +999,17 @@ bool JumpThreadingPass::SimplifyPartiall
// Scan the predecessor to see if the value is available in the pred.
BBIt = PredBB->end();
- AAMDNodes ThisAATags;
Value *PredAvailable = FindAvailableLoadedValue(LI, PredBB, BBIt,
DefMaxInstsToScan,
- nullptr, &ThisAATags);
+ nullptr,
+ &IsLoadCSE);
if (!PredAvailable) {
OneUnavailablePred = PredBB;
continue;
}
- // If AA tags disagree or are not present, forget about them.
- if (AATags != ThisAATags) AATags = AAMDNodes();
+ if (IsLoadCSE)
+ CSELoads.push_back(cast<LoadInst>(PredAvailable));
// If so, this load is partially redundant. Remember this info so that we
// can create a PHI node.
@@ -1101,6 +1107,10 @@ bool JumpThreadingPass::SimplifyPartiall
PN->addIncoming(PredV, I->first);
}
+ for (LoadInst *PredLI : CSELoads) {
+ combineMetadataForCSE(PredLI, LI);
+ }
+
LI->replaceAllUsesWith(PN);
LI->eraseFromParent();
Modified: llvm/trunk/lib/Transforms/Utils/Local.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/Local.cpp?rev=277977&r1=277976&r2=277977&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/Local.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/Local.cpp Sun Aug 7 23:10:22 2016
@@ -1646,6 +1646,17 @@ void llvm::combineMetadata(Instruction *
K->setMetadata(LLVMContext::MD_invariant_group, JMD);
}
+void llvm::combineMetadataForCSE(Instruction *K, const Instruction *J) {
+ unsigned KnownIDs[] = {
+ LLVMContext::MD_tbaa, LLVMContext::MD_alias_scope,
+ LLVMContext::MD_noalias, LLVMContext::MD_range,
+ LLVMContext::MD_invariant_load, LLVMContext::MD_nonnull,
+ LLVMContext::MD_invariant_group, LLVMContext::MD_align,
+ LLVMContext::MD_dereferenceable,
+ LLVMContext::MD_dereferenceable_or_null};
+ combineMetadata(K, J, KnownIDs);
+}
+
unsigned llvm::replaceDominatedUsesWith(Value *From, Value *To,
DominatorTree &DT,
const BasicBlockEdge &Root) {
Modified: llvm/trunk/test/Transforms/JumpThreading/thread-loads.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/JumpThreading/thread-loads.ll?rev=277977&r1=277976&r2=277977&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/JumpThreading/thread-loads.ll (original)
+++ llvm/trunk/test/Transforms/JumpThreading/thread-loads.ll Sun Aug 7 23:10:22 2016
@@ -246,7 +246,70 @@ bb3:
ret i32 %res.0
}
+; Make sure we merge the aliasing metadata. (If we don't, we have a load
+; with the wrong metadata, so the branch gets incorrectly eliminated.)
+define void @test8(i32*, i32*, i32*) {
+; CHECK-LABEL: @test8(
+; CHECK: %a = load i32, i32* %0, !range !4
+; CHECK-NEXT: store i32 %a
+; CHECK: br i1 %c
+ %a = load i32, i32* %0, !tbaa !0, !range !4, !alias.scope !9, !noalias !10
+ %b = load i32, i32* %0, !range !5
+ store i32 %a, i32* %1
+ %c = icmp eq i32 %b, 8
+ br i1 %c, label %ret1, label %ret2
+
+ret1:
+ ret void
+
+ret2:
+ %xxx = tail call i32 (...) @f1() nounwind
+ ret void
+}
+
+; Make sure we merge/PRE aliasing metadata correctly. That means that
+; we need to remove metadata from the existing load, and add appropriate
+; metadata to the newly inserted load.
+define void @test9(i32*, i32*, i32*, i1 %c) {
+; CHECK-LABEL: @test9(
+ br i1 %c, label %d1, label %d2
+
+; CHECK: d1:
+; CHECK-NEXT: %a = load i32, i32* %0{{$}}
+d1:
+ %a = load i32, i32* %0, !range !4, !alias.scope !9, !noalias !10
+ br label %d3
+
+; CHECK: d2:
+; CHECK-NEXT: %xxxx = tail call i32 (...) @f1()
+; CHECK-NEXT: %b.pr = load i32, i32* %0, !tbaa !0{{$}}
+d2:
+ %xxxx = tail call i32 (...) @f1() nounwind
+ br label %d3
+
+d3:
+ %p = phi i32 [ 1, %d2 ], [ %a, %d1 ]
+ %b = load i32, i32* %0, !tbaa !0
+ store i32 %p, i32* %1
+ %c2 = icmp eq i32 %b, 8
+ br i1 %c2, label %ret1, label %ret2
+
+ret1:
+ ret void
+
+ret2:
+ %xxx = tail call i32 (...) @f1() nounwind
+ ret void
+}
+
!0 = !{!3, !3, i64 0}
!1 = !{!"omnipotent char", !2}
!2 = !{!"Simple C/C++ TBAA", null}
!3 = !{!"int", !1}
+!4 = !{ i32 0, i32 1 }
+!5 = !{ i32 8, i32 10 }
+!6 = !{!6}
+!7 = !{!7, !6}
+!8 = !{!8, !6}
+!9 = !{!7}
+!10 = !{!8}
More information about the llvm-commits
mailing list