This is an archive of the discontinued LLVM Phabricator instance.

Differential D9007

Don't unroll loops in loop vectorization pass when VF is one.
ClosedPublic

Authored by wmi on Apr 13 2015, 2:37 PM.

Details

Reviewers
hfinkel
Commits
rG6e3344ed98a3: Limiting gep merging to fix the performance problem described in https://llvm.
rL235451: Limiting gep merging to fix the performance problem described in
Summary

The patch is to fix the problem described in https://llvm.org/bugs/show_bug.cgi?id=23217

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.

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.

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.

Diff Detail

Repository
rL LLVM

Event Timeline

wmi updated this revision to Diff 23696.Apr 13 2015, 2:37 PM
wmi retitled this revision from to Don't unroll loops in loop vectorization pass when VF is one..
wmi updated this object.
wmi edited the test plan for this revision. (Show Details)
wmi added a reviewer: hfinkel.
wmi set the repository for this revision to rL LLVM.
wmi added a subscriber: Unknown Object (MLST).
wmi updated this revision to Diff 24098.Apr 20 2015, 10:06 PM

Update the patch to disable interleave in loop vectorizer when VF==1 only for x86 architecture.
Update test/Transforms/LoopVectorize/X86/unroll-small-loops.ll.

Closed by commit rL235451: Limiting gep merging to fix the performance problem described in (authored by wmi). · Explain WhyApr 21 2015, 3:40 PM
This revision was automatically updated to reflect the committed changes.
wmi added a comment.Apr 28 2015, 10:45 AM

Ping.

hfinkel edited edge metadata.May 5 2015, 7:31 PM
In D9007#162733, @wmi wrote:

Ping.

What's going on here? This is a closed review for something it claims was already committed. Did you upload the patch to the wrong place?

wmi added a comment.May 5 2015, 8:20 PM

Sorry, my bad. I mistakenly used the review number here in another
commit. This patch is still pending for review.

Wei.

hfinkel added a comment.May 5 2015, 8:28 PM
In D9007#166472, @wmi wrote:

Sorry, my bad. I mistakenly used the review number here in another
commit. This patch is still pending for review.

Okay, can you please repost with the correct summary?

Wei.

Revision Contents

PathSize
llvm/
trunk/
lib/
Transforms/
InstCombine/
5 lines
test/
Transforms/
InstCombine/
20 lines
48 lines
4 lines

Diff 24180

llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp

Show First 20 Lines • Show All 1,461 Lines • ▼ Show 20 Lines if (EndsWithSequential) {
Sum = SO1; Sum = SO1;
} else { } else {
// If they aren't the same type, then the input hasn't been processed // If they aren't the same type, then the input hasn't been processed
// by the loop above yet (which canonicalizes sequential index types to // by the loop above yet (which canonicalizes sequential index types to
// intptr_t). Just avoid transforming this until the input has been // intptr_t). Just avoid transforming this until the input has been
// normalized. // normalized.
if (SO1->getType() != GO1->getType()) if (SO1->getType() != GO1->getType())
return nullptr; return nullptr;
// Only do the combine when GO1 and SO1 are both constants. Only in
// this case, we are sure the cost after the merge is never more than
// that before the merge.
if (!isa<Constant>(GO1) || !isa<Constant>(SO1))
return nullptr;
Sum = Builder->CreateAdd(SO1, GO1, PtrOp->getName()+".sum"); Sum = Builder->CreateAdd(SO1, GO1, PtrOp->getName()+".sum");
} }
// Update the GEP in place if possible. // Update the GEP in place if possible.
if (Src->getNumOperands() == 2) { if (Src->getNumOperands() == 2) {
GEP.setOperand(0, Src->getOperand(0)); GEP.setOperand(0, Src->getOperand(0));
GEP.setOperand(1, Sum); GEP.setOperand(1, Sum);
return &GEP; return &GEP;
▲ Show 20 Lines • Show All 1,582 Lines • Show Last 20 Lines

llvm/trunk/test/Transforms/InstCombine/descale-zero.ll

; RUN: opt < %s -instcombine -S | FileCheck %s
target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-apple-macosx10.10.0"
define internal i8* @descale_zero() {
entry:
; CHECK: load i8*, i8** inttoptr (i64 48 to i8**), align 16
; CHECK-NEXT: ret i8*
%i16_ptr = load i16*, i16** inttoptr (i64 48 to i16**), align 16
%num = load i64, i64* inttoptr (i64 64 to i64*), align 64
%num_times_2 = shl i64 %num, 1
%num_times_2_plus_4 = add i64 %num_times_2, 4
%i8_ptr = bitcast i16* %i16_ptr to i8*
%i8_ptr_num_times_2_plus_4 = getelementptr i8, i8* %i8_ptr, i64 %num_times_2_plus_4
%num_times_neg2 = mul i64 %num, -2
%num_times_neg2_minus_4 = add i64 %num_times_neg2, -4
%addr = getelementptr i8, i8* %i8_ptr_num_times_2_plus_4, i64 %num_times_neg2_minus_4
ret i8* %addr
}

llvm/trunk/test/Transforms/InstCombine/gep-merge.ll

; This test makes sure that gep(gep ...) merge doesn't come into effect.
; RUN: opt < %s -instcombine -S | FileCheck %s
; Make sure there are no geps being merged.
; CHECK-LABEL: @fn3(
; CHECK: getelementptr
; CHECK: getelementptr
; CHECK: getelementptr
@_ZN2cv1aE = global i8* zeroinitializer, align 8
declare i32 @fn1() #2
declare i32 @fn2() #2
; Function Attrs: uwtable
define linkonce_odr i32 @fn3() {
entry:
%call = call i32 @fn1()
%call1 = call i32 @fn2()
%0 = load i8*, i8** @_ZN2cv1aE, align 8
%idx.ext2 = sext i32 %call1 to i64
%add.ptr3 = getelementptr inbounds i8, i8* %0, i64 %idx.ext2
br label %for.cond5
for.cond5:
%total1 = phi i32 [ 0, %entry ], [ %total2, %for.body7 ]
%x.1 = phi i32 [ 0, %entry ], [ %inc, %for.body7 ]
%cmp6 = icmp slt i32 %x.1, %call
br i1 %cmp6, label %for.body7, label %for.cond34
for.body7: ; preds = %for.cond5
%mul = mul nsw i32 %x.1, 2
%idxprom = sext i32 %mul to i64
%arrayidx = getelementptr inbounds i8, i8* %add.ptr3, i64 %idxprom
%1 = load i8, i8* %arrayidx, align 1
%conv = zext i8 %1 to i32
%sub = sub nsw i32 %mul, 1
%idxprom10 = sext i32 %sub to i64
%arrayidx11 = getelementptr inbounds i8, i8* %add.ptr3, i64 %idxprom10
%2 = load i8, i8* %arrayidx11, align 1
%conv2 = zext i8 %2 to i32
%add1 = add nsw i32 %conv, %conv2
%total2 = add nsw i32 %total1, %add1
%inc = add nsw i32 %x.1, 1
br label %for.cond5
for.cond34:
ret i32 %total1
}

llvm/trunk/test/Transforms/InstCombine/getelementptr.ll

Show First 20 Lines • Show All 98 Lines • ▼ Show 20 Lines; CHECK-NEXT: ret void
ret void ret void
} }
define i32* @test7(i32* %I, i64 %C, i64 %D) { define i32* @test7(i32* %I, i64 %C, i64 %D) {
%A = getelementptr i32, i32* %I, i64 %C %A = getelementptr i32, i32* %I, i64 %C
%B = getelementptr i32, i32* %A, i64 %D %B = getelementptr i32, i32* %A, i64 %D
ret i32* %B ret i32* %B
; CHECK-LABEL: @test7( ; CHECK-LABEL: @test7(
; CHECK: %A.sum = add i64 %C, %D ; CHECK: %A = getelementptr i32, i32* %I, i64 %C
; CHECK: getelementptr i32, i32* %I, i64 %A.sum ; CHECK: %B = getelementptr i32, i32* %A, i64 %D
} }
define i8* @test8([10 x i32]* %X) { define i8* @test8([10 x i32]* %X) {
;; Fold into the cast. ;; Fold into the cast.
%A = getelementptr [10 x i32], [10 x i32]* %X, i64 0, i64 0 %A = getelementptr [10 x i32], [10 x i32]* %X, i64 0, i64 0
%B = bitcast i32* %A to i8* %B = bitcast i32* %A to i8*
ret i8* %B ret i8* %B
; CHECK-LABEL: @test8( ; CHECK-LABEL: @test8(
▲ Show 20 Lines • Show All 794 Lines • Show Last 20 Lines
llvm/trunk/lib/Transforms/InstCombine/InstructionCombining.cpp