<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Mon, Jan 8, 2018 at 7:07 AM Davide Italiano via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: davide<br>
Date: Sun Jan 7 14:06:24 2018<br>
New Revision: 321974<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=321974&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=321974&view=rev</a><br>
Log:<br>
[SLPVectorizer] Reintroduce std::stable_sort(properlyDominates()).<br></blockquote><div><br></div><div>I would be happy if you added FIXME since it was bad.</div><div>It should be rewritten with more stable algorithm.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
The approach was never discussed, I wasn't able to reproduce this<br>
non-determinism, and the original author went AWOL.<br></blockquote><div><br></div><div>Have you really tried to reproduce the issue?</div><div>I made a simple testscase that works differently between libstdc++ and libc++, and I suppose no one but I tried it.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
After a discussion on the ML, Philip suggested to revert this.<br>
<br>
Removed:<br>
llvm/trunk/test/Transforms/SLPVectorizer/X86/visit-dominated.ll<br></blockquote><div><br></div><div>I am sorry if this didn't satisfy to revert.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Modified:<br>
llvm/trunk/lib/Transforms/Vectorize/SLPVectorizer.cpp<br>
<br>
Modified: llvm/trunk/lib/Transforms/Vectorize/SLPVectorizer.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Vectorize/SLPVectorizer.cpp?rev=321974&r1=321973&r2=321974&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Vectorize/SLPVectorizer.cpp?rev=321974&r1=321973&r2=321974&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/Vectorize/SLPVectorizer.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/Vectorize/SLPVectorizer.cpp Sun Jan 7 14:06:24 2018<br>
@@ -597,7 +597,7 @@ public:<br>
unsigned getTreeSize() const { return VectorizableTree.size(); }<br>
<br>
/// \brief Perform LICM and CSE on the newly generated gather sequences.<br>
- void optimizeGatherSequence(Function &F);<br>
+ void optimizeGatherSequence();<br>
<br>
/// \returns true if it is beneficial to reverse the vector order.<br>
bool shouldReorder() const {<br>
@@ -3310,7 +3310,7 @@ BoUpSLP::vectorizeTree(ExtraValueToDebug<br>
return VectorizableTree[0].VectorizedValue;<br>
}<br>
<br>
-void BoUpSLP::optimizeGatherSequence(Function &F) {<br>
+void BoUpSLP::optimizeGatherSequence() {<br>
DEBUG(dbgs() << "SLP: Optimizing " << GatherSeq.size()<br>
<< " gather sequences instructions.\n");<br>
// LICM InsertElementInst sequences.<br>
@@ -3344,16 +3344,30 @@ void BoUpSLP::optimizeGatherSequence(Fun<br>
Insert->moveBefore(PreHeader->getTerminator());<br>
}<br>
<br>
+ // Make a list of all reachable blocks in our CSE queue.<br>
+ SmallVector<const DomTreeNode *, 8> CSEWorkList;<br>
+ CSEWorkList.reserve(CSEBlocks.size());<br>
+ for (BasicBlock *BB : CSEBlocks)<br>
+ if (DomTreeNode *N = DT->getNode(BB)) {<br>
+ assert(DT->isReachableFromEntry(N));<br>
+ CSEWorkList.push_back(N);<br>
+ }<br>
+<br>
+ // Sort blocks by domination. This ensures we visit a block after all blocks<br>
+ // dominating it are visited.<br>
+ std::stable_sort(CSEWorkList.begin(), CSEWorkList.end(),<br>
+ [this](const DomTreeNode *A, const DomTreeNode *B) {<br>
+ return DT->properlyDominates(A, B);<br>
+ });<br>
+<br>
// Perform O(N^2) search over the gather sequences and merge identical<br>
// instructions. TODO: We can further optimize this scan if we split the<br>
// instructions into different buckets based on the insert lane.<br>
SmallVector<Instruction *, 16> Visited;<br>
- ReversePostOrderTraversal<Function *> RPOT(&F);<br>
- for (auto BB : RPOT) {<br>
- // Traverse CSEBlocks by RPOT order.<br>
- if (!CSEBlocks.count(BB))<br>
- continue;<br>
-<br>
+ for (auto I = CSEWorkList.begin(), E = CSEWorkList.end(); I != E; ++I) {<br>
+ assert((I == CSEWorkList.begin() || !DT->dominates(*I, *std::prev(I))) &&<br>
+ "Worklist not sorted properly!");<br>
+ BasicBlock *BB = (*I)->getBlock();<br>
// For all instructions in blocks containing gather sequences:<br>
for (BasicBlock::iterator it = BB->begin(), e = BB->end(); it != e;) {<br>
Instruction *In = &*it++;<br>
@@ -4222,7 +4236,7 @@ bool SLPVectorizerPass::runImpl(Function<br>
}<br>
<br>
if (Changed) {<br>
- R.optimizeGatherSequence(F);<br>
+ R.optimizeGatherSequence();<br>
DEBUG(dbgs() << "SLP: vectorized \"" << F.getName() << "\"\n");<br>
DEBUG(verifyFunction(F));<br>
}<br>
<br>
Removed: llvm/trunk/test/Transforms/SLPVectorizer/X86/visit-dominated.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SLPVectorizer/X86/visit-dominated.ll?rev=321973&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SLPVectorizer/X86/visit-dominated.ll?rev=321973&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/SLPVectorizer/X86/visit-dominated.ll (original)<br>
+++ llvm/trunk/test/Transforms/SLPVectorizer/X86/visit-dominated.ll (removed)<br>
@@ -1,153 +0,0 @@<br>
-; RUN: opt -slp-vectorizer < %s -S | FileCheck %s<br>
-; Ensure each dominator block comes first in advance of its users.<br>
-; VEC_VALUE_QUALTYPE should dominate others.<br>
-; QUAL1_*(s) may be inducted by VEC_VALUE_QUALTYPE, since their pred is "entry".<br>
-<br>
-target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"<br>
-target triple = "x86_64-unknown-linux-gnu"<br>
-<br>
-%AtomicInfo = type { %"class.clang::CodeGen::LValue" }<br>
-%"class.clang::QualType" = type { %"class.llvm::PointerIntPair.25" }<br>
-%"class.llvm::PointerIntPair.25" = type { i64 }<br>
-%"class.clang::CodeGen::LValue" = type { i32, i64*, %union.anon.1473, %"class.clang::QualType", %"class.clang::Qualifiers", i64, i8, [3 x i8], i64*, %"struct.clang::CodeGen::TBAAAccessInfo" }<br>
-%union.anon.1473 = type { %"class.llvm::Value"* }<br>
-%"class.llvm::Value" = type { i64*, i64*, i8, i8, i16, i32 }<br>
-%"class.clang::Qualifiers" = type { i32 }<br>
-%"struct.clang::CodeGen::TBAAAccessInfo" = type { %"class.clang::QualType", %"class.llvm::MDNode"*, i64 }<br>
-%"class.llvm::MDNode" = type { i64*, i32, i32, i64* }<br>
-%ExtQualsTypeCommonBase = type { %"class.clang::Type"*, %"class.clang::QualType" }<br>
-%"class.clang::Type" = type { %ExtQualsTypeCommonBase, %union.anon.26 }<br>
-%union.anon.26 = type { %"class.clang::Type::AttributedTypeBitfields", [4 x i8] }<br>
-%"class.clang::Type::AttributedTypeBitfields" = type { i32 }<br>
-%ExtQuals = type <{ %ExtQualsTypeCommonBase, %"class.llvm::FoldingSetBase::Node", %"class.clang::Qualifiers", [4 x i8] }><br>
-%"class.llvm::FoldingSetBase::Node" = type { i8* }<br>
-<br>
-define hidden fastcc void @_ZL21EmitAtomicUpdateValueRN5clang7CodeGen15CodeGenFunctionERN12_GLOBAL__N_110AtomicInfoENS0_6RValueENS0_7AddressE(%AtomicInfo* nocapture readonly dereferenceable(192) %Atomics) unnamed_addr {<br>
-entry:<br>
- %agg = alloca %"class.clang::CodeGen::LValue", align 8<br>
- %AtomicLValP00 = getelementptr inbounds %AtomicInfo, %AtomicInfo* %Atomics, i64 0, i32 0, i32 0<br>
-<br>
- %AtomicLValP02 = getelementptr inbounds %AtomicInfo, %AtomicInfo* %Atomics, i64 0, i32 0, i32 2, i32 0<br>
-; CHECK: [[VALUE0:%.+]] = getelementptr inbounds %AtomicInfo, %AtomicInfo* %Atomics, i64 0, i32 0, i32 2, i32 0<br>
- %AtomicLValP03 = getelementptr inbounds %AtomicInfo, %AtomicInfo* %Atomics, i64 0, i32 0, i32 3, i32 0, i32 0<br>
-<br>
- %AtomicLVal = load i32, i32* %AtomicLValP00, align 8<br>
- %tmp = bitcast %"class.llvm::Value"** %AtomicLValP02 to i64*<br>
-; CHECK: [[TMP:%.+]] = bitcast %"class.llvm::Value"** [[VALUE0]] to i64*<br>
-<br>
- %AtomicLVal.LValue = load i64, i64* %tmp, align 8<br>
- %AtomicLVal.QualType = load i64, i64* %AtomicLValP03, align 8<br>
-; CHECK: [[VECP:%.+]] = bitcast i64* [[TMP]] to <2 x i64>*<br>
-; CHECK: [[VEC_VALUE_QUALTYPE:%.+]] = load <2 x i64>, <2 x i64>* [[VECP]], align 8<br>
-<br>
- switch i32 %AtomicLVal, label %if.else23 [<br>
- i32 2, label %if.then<br>
- i32 1, label %if.then11<br>
- ]<br>
-<br>
-; CHECK-LABEL: if.then11:<br>
-if.then11: ; preds = %entry<br>
-; CHECK: [[QUAL1_11:%.+]] = extractelement <2 x i64> [[VEC_VALUE_QUALTYPE]], i32 1<br>
- %and.57 = and i64 %AtomicLVal.QualType, -16<br>
-; CHECK: = and i64 [[QUAL1_11]], -16<br>
-<br>
- %tmp5 = inttoptr i64 %and.57 to %ExtQualsTypeCommonBase*<br>
- %Value.58 = getelementptr inbounds %ExtQualsTypeCommonBase, %ExtQualsTypeCommonBase* %tmp5, i64 0, i32 1, i32 0, i32 0<br>
- %tmp6 = load i64, i64* %Value.58, align 8<br>
- %tmp7 = and i64 %tmp6, 8<br>
- %tobool.59 = icmp eq i64 %tmp7, 0<br>
- br i1 %tobool.59, label %MakeVectorElt.exit, label %if.then.63<br>
-<br>
-; CHECK-LABEL: if.then:<br>
-if.then: ; preds = %entry<br>
-; CHECK: [[QUAL1:%.+]] = extractelement <2 x i64> [[VEC_VALUE_QUALTYPE]], i32 1<br>
- %and.96 = and i64 %AtomicLVal.QualType, -16<br>
-; CHECK: = and i64 [[QUAL1]], -16<br>
-<br>
- %tmp1 = inttoptr i64 %and.96 to %ExtQualsTypeCommonBase*<br>
- %Value.97 = getelementptr inbounds %ExtQualsTypeCommonBase, %ExtQualsTypeCommonBase* %tmp1, i64 0, i32 1, i32 0, i32 0<br>
- %tmp2 = load i64, i64* %Value.97, align 8<br>
- %tmp3 = and i64 %tmp2, 8<br>
- %tobool.98 = icmp eq i64 %tmp3, 0<br>
- br i1 %tobool.98, label %MakeBitfield.exit, label %if.then.102<br>
-<br>
-if.then.102: ; preds = %if.then<br>
- %and.99 = and i64 %tmp2, -16<br>
- %tmp4 = inttoptr i64 %and.99 to %ExtQuals*<br>
- %retval.100 = getelementptr inbounds %ExtQuals, %ExtQuals* %tmp4, i64 0, i32 2, i32 0<br>
- %retval.101 = load i32, i32* %retval.100, align 8<br>
- br label %MakeBitfield.exit<br>
-<br>
-; CHECK_LABEL: MakeBitfield.exit:<br>
-MakeBitfield.exit: ; preds = %if.then.102, %if.then<br>
- %retval.103 = phi i32 [ %retval.101, %if.then.102 ], [ 0, %if.then ]<br>
-<br>
- %conv.104 = or i64 %tmp2, %AtomicLVal.QualType<br>
-; CHECK: = or i64 %tmp2, [[QUAL1]]<br>
-<br>
- %conv.105 = trunc i64 %conv.104 to i32<br>
- %or.106 = and i32 %conv.105, 7<br>
- %or.107 = or i32 %retval.103, %or.106<br>
- br label %if.end35<br>
-<br>
-if.then.63: ; preds = %if.then11<br>
- %and.60 = and i64 %tmp6, -16<br>
- %tmp8 = inttoptr i64 %and.60 to %ExtQuals*<br>
- %retval.61 = getelementptr inbounds %ExtQuals, %ExtQuals* %tmp8, i64 0, i32 2, i32 0<br>
- %retval.62 = load i32, i32* %retval.61<br>
- br label %MakeVectorElt.exit<br>
-<br>
-; CHECK-LABEL:MakeVectorElt.exit:<br>
-MakeVectorElt.exit: ; preds = %if.then.63, %if.then11<br>
- %retval.64 = phi i32 [ %retval.62, %if.then.63 ], [ 0, %if.then11 ]<br>
-<br>
- %conv.65 = or i64 %tmp6, %AtomicLVal.QualType<br>
-; CHECK: = or i64 %tmp6, [[QUAL1_11]]<br>
-<br>
- %conv.66 = trunc i64 %conv.65 to i32<br>
- %or.67 = and i32 %conv.66, 7<br>
- %or.68 = or i32 %retval.64, %or.67<br>
- br label %if.end35<br>
-<br>
-; CHECK-LABEL: if.else23:<br>
-if.else23: ; preds = %entry<br>
-; CHECK: [[QUAL1_23:%.+]] = extractelement <2 x i64> [[VEC_VALUE_QUALTYPE]], i32 1<br>
- %and.0 = and i64 %AtomicLVal.QualType, -16<br>
-; CHECK: = and i64 [[QUAL1_23]], -16<br>
-<br>
- %tmp9 = inttoptr i64 %and.0 to %ExtQualsTypeCommonBase*<br>
- %Value.9 = getelementptr inbounds %ExtQualsTypeCommonBase, %ExtQualsTypeCommonBase* %tmp9, i64 0, i32 1, i32 0, i32 0<br>
- %tmp10 = load i64, i64* %Value.9, align 8<br>
- %tmp11 = and i64 %tmp10, 8<br>
- %tobool.0 = icmp eq i64 %tmp11, 0<br>
- br i1 %tobool.0, label %MakeExtVectorElt.exit, label %MakeExtVectorElt.exit<br>
-<br>
-; CHECK-LABEL:MakeExtVectorElt.exit:<br>
-MakeExtVectorElt.exit: ; preds = %MakeExtVectorElt.exit, %if.else23<br>
-<br>
- %conv.67 = or i64 %tmp10, %AtomicLVal.QualType<br>
-; CHECK: = or i64 %tmp10, [[QUAL1_23]]<br>
-<br>
- %or.0 = trunc i64 %conv.67 to i32<br>
- br label %if.end35<br>
-<br>
-; CHECK-LABEL: if.end35:<br>
-if.end35: ; preds = %MakeExtVectorElt.exit, %MakeVectorElt.exit, %MakeBitfield.exit<br>
- %DesiredLVal = phi i32 [ %or.107, %MakeBitfield.exit ], [ %or.68, %MakeVectorElt.exit ], [ %or.0, %MakeExtVectorElt.exit ]<br>
-<br>
- %DesiredLValP.2 = getelementptr inbounds %"class.clang::CodeGen::LValue", %"class.clang::CodeGen::LValue"* %agg, i64 0, i32 2, i32 0<br>
-; CHECK: [[VALP2:%.+]] = getelementptr inbounds %"class.clang::CodeGen::LValue", %"class.clang::CodeGen::LValue"* %agg, i64 0, i32 2, i32 0<br>
- %DesiredLValP.3 = getelementptr inbounds %"class.clang::CodeGen::LValue", %"class.clang::CodeGen::LValue"* %agg, i64 0, i32 3, i32 0, i32 0<br>
-<br>
- %tmp14 = bitcast %"class.llvm::Value"** %DesiredLValP.2 to i64*<br>
-; CHECK: [[TMP14:%.+]] = bitcast %"class.llvm::Value"** [[VALP2]] to i64*<br>
-<br>
- store i64 %AtomicLVal.LValue, i64* %tmp14, align 8<br>
- store i64 %AtomicLVal.QualType, i64* %DesiredLValP.3, align 8<br>
-; CHECK: [[LVALUE:%.+]] = bitcast i64* [[TMP14]] to <2 x i64>*<br>
-; CHECK: store <2 x i64> [[VEC_VALUE_QUALTYPE]], <2 x i64>* [[LVALUE]], align 8<br>
-<br>
- %DesiredLValP = getelementptr inbounds %"class.clang::CodeGen::LValue", %"class.clang::CodeGen::LValue"* %agg, i64 0, i32 4, i32 0<br>
- store i32 %DesiredLVal, i32* %DesiredLValP, align 8<br>
- ret void<br>
-}<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div></div>