Hi Wei,<br><br>The important difference between loopunroll and the loop vectoriser unrolling is alias checks and interleaving. <br><br>The normal unroller will concatenate iterations, which without good alias analysis results in a schedule that cannot be reordered. The loop vectoriser will use loopaccessanalysis to plant runtime pointer checks, allowing a much better schedule. <br><br>So I don't think your patch as-is makes sense, unless loopunrollruntime has grown runtime ptr check support recently.<br><br>Cheers,<br><br>James<br><div class="gmail_quote">On Mon, 13 Apr 2015 at 22:43, Wei Mi <<a href="mailto:wmi@google.com">wmi@google.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi hfinkel,<br>
<br>
The patch is to fix the problem described in <a href="https://llvm.org/bugs/show_bug.cgi?id=23217" target="_blank">https://llvm.org/bugs/show_bug.cgi?id=23217</a><br>
<br>
The problem is: When loop vectorization decide not to vectorize a loop (VF==1), it may still unroll the loop. However, the unroll factor may be smaller than the factor computed in loop unroll pass, so loop unroll pass may unroll the already unrolled loop once more and unroll the remainder loop at the same time.<br>
<br>
I don't see the benefit of unrolling when VF==1. The patch is to disable the unrolling when VF==1 in loop vectorization pass, and let loop unroll pass to do the unrolling for such loop.<br>
<br>
Performance neutral for spec2000. Google internal benchmarks: detection improved by 5% on sandybridge and 9% on westmere, saw improved by 1.5% on both platforms.<br>
<br>
REPOSITORY<br>
rL LLVM<br>
<br>
<a href="http://reviews.llvm.org/D9007" target="_blank">http://reviews.llvm.org/D9007</a><br>
<br>
Files:<br>
lib/Transforms/Vectorize/LoopVectorize.cpp<br>
test/Transforms/LoopVectorize/unroll.ll<br>
<br>
Index: lib/Transforms/Vectorize/LoopVectorize.cpp<br>
===================================================================<br>
--- lib/Transforms/Vectorize/LoopVectorize.cpp<br>
+++ lib/Transforms/Vectorize/LoopVectorize.cpp<br>
@@ -1468,34 +1468,24 @@<br>
CM.selectVectorizationFactor(OptForSize);<br>
<br>
// Select the unroll factor.<br>
- const unsigned UF =<br>
- CM.selectUnrollFactor(OptForSize, VF.Width, VF.Cost);<br>
+ unsigned UF = CM.selectUnrollFactor(OptForSize, VF.Width, VF.Cost);<br>
+<br>
+ // If a loop is not beneficial to be vectorized, don't unroll it here.<br>
+ // Let the LoopUnroll pass to unroll it.<br>
+ if (VF.Width == 1)<br>
+ UF = 1;<br>
<br>
DEBUG(dbgs() << "LV: Found a vectorizable loop (" << VF.Width << ") in "<br>
<< DebugLocStr << '\n');<br>
DEBUG(dbgs() << "LV: Unroll Factor is " << UF << '\n');<br>
<br>
if (VF.Width == 1) {<br>
DEBUG(dbgs() << "LV: Vectorization is possible but not beneficial\n");<br>
<br>
- if (UF == 1) {<br>
- emitOptimizationRemarkAnalysis(<br>
- F->getContext(), DEBUG_TYPE, *F, L->getStartLoc(),<br>
- "not beneficial to vectorize and user disabled interleaving");<br>
- return false;<br>
- }<br>
- DEBUG(dbgs() << "LV: Trying to at least unroll the loops.\n");<br>
-<br>
- // Report the unrolling decision.<br>
- emitOptimizationRemark(F->getContext(), DEBUG_TYPE, *F, L->getStartLoc(),<br>
- Twine("unrolled with interleaving factor " +<br>
- Twine(UF) +<br>
- " (vectorization not beneficial)"));<br>
-<br>
- // We decided not to vectorize, but we may want to unroll.<br>
-<br>
- InnerLoopUnroller Unroller(L, SE, LI, DT, TLI, TTI, UF);<br>
- Unroller.vectorize(&LVL);<br>
+ emitOptimizationRemarkAnalysis(<br>
+ F->getContext(), DEBUG_TYPE, *F, L->getStartLoc(),<br>
+ "not beneficial to vectorize and user disabled interleaving");<br>
+ return false;<br>
} else {<br>
// If we decided that it is *legal* to vectorize the loop then do it.<br>
InnerLoopVectorizer LB(L, SE, LI, DT, TLI, TTI, VF.Width, UF);<br>
Index: test/Transforms/LoopVectorize/unroll.ll<br>
===================================================================<br>
--- test/Transforms/LoopVectorize/unroll.ll<br>
+++ test/Transforms/LoopVectorize/unroll.ll<br>
@@ -0,0 +1,37 @@<br>
+; This test makes sure that loop will not be unrolled in vectorization if VF computed<br>
+; equals to 1.<br>
+; RUN: opt < %s -loop-vectorize -S | FileCheck %s<br>
+<br>
+; Make sure there are no geps being merged.<br>
+; CHECK-LABEL: @foo(<br>
+; CHECK: getelementptr<br>
+; CHECK-NOT: getelementptr<br>
+<br>
+@N = common global i32 0, align 4<br>
+@a = common global [1000 x i32] zeroinitializer, align 16<br>
+<br>
+define void @foo() #0 {<br>
+entry:<br>
+ %0 = load i32, i32* @N, align 4<br>
+ %cmp5 = icmp sgt i32 %0, 0<br>
+ br i1 %cmp5, label %<a href="http://for.body.lr.ph" target="_blank">for.body.lr.ph</a>, label %for.end<br>
+<br>
+<a href="http://for.body.lr.ph" target="_blank">for.body.lr.ph</a>: ; preds = %entry<br>
+ %conv = sext i32 %0 to i64<br>
+ br label %for.body<br>
+<br>
+for.body: ; preds = %<a href="http://for.body.lr.ph" target="_blank">for.body.lr.ph</a>, %for.body<br>
+ %i.06 = phi i64 [ 0, %<a href="http://for.body.lr.ph" target="_blank">for.body.lr.ph</a> ], [ %inc, %for.body ]<br>
+ %mul = mul nuw nsw i64 %i.06, 7<br>
+ %arrayidx = getelementptr inbounds [1000 x i32], [1000 x i32]* @a, i64 0, i64 %mul<br>
+ store i32 3, i32* %arrayidx, align 4<br>
+ %inc = add nuw nsw i64 %i.06, 1<br>
+ %cmp = icmp slt i64 %inc, %conv<br>
+ br i1 %cmp, label %for.body, label %for.end.loopexit<br>
+<br>
+for.end.loopexit: ; preds = %for.body<br>
+ br label %for.end<br>
+<br>
+for.end: ; preds = %for.end.loopexit, %entry<br>
+ ret void<br>
+}<br>
<br>
EMAIL PREFERENCES<br>
<a href="http://reviews.llvm.org/settings/panel/emailpreferences/" target="_blank">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>