[llvm] r270306 - [GuardWidening] Fix incorrect use of remove_if
Sanjoy Das via llvm-commits
llvm-commits at lists.llvm.org
Fri May 20 19:24:46 PDT 2016
Author: sanjoy
Date: Fri May 20 21:24:44 2016
New Revision: 270306
URL: http://llvm.org/viewvc/llvm-project?rev=270306&view=rev
Log:
[GuardWidening] Fix incorrect use of remove_if
I had used `std::remove_if` under the assumption that it moves the
predicate matching elements to the end, but actaully the elements
remaining towards the end (after the iterator returned by
`std::remove_if`) are indeterminate. Fix the bug (and make the code
more straightforward) by using a temporary SmallVector, and add a test
case demonstrating the issue.
Modified:
llvm/trunk/lib/Transforms/Scalar/GuardWidening.cpp
llvm/trunk/test/Transforms/GuardWidening/range-check-merging.ll
Modified: llvm/trunk/lib/Transforms/Scalar/GuardWidening.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/GuardWidening.cpp?rev=270306&r1=270305&r2=270306&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/GuardWidening.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/GuardWidening.cpp Fri May 20 21:24:44 2016
@@ -551,26 +551,33 @@ bool GuardWideningImpl::combineRangeChec
SmallVectorImpl<GuardWideningImpl::RangeCheck> &RangeChecksOut) {
unsigned OldCount = Checks.size();
while (!Checks.empty()) {
- Value *Base = Checks[0].Base;
- Value *Length = Checks[0].Length;
- auto ChecksStart =
- remove_if(Checks, [&](GuardWideningImpl::RangeCheck &RC) {
- return RC.Base == Base && RC.Length == Length;
- });
-
- unsigned CheckCount = std::distance(ChecksStart, Checks.end());
- assert(CheckCount != 0 && "We know we have at least one!");
-
- if (CheckCount < 3) {
- RangeChecksOut.insert(RangeChecksOut.end(), ChecksStart, Checks.end());
- Checks.erase(ChecksStart, Checks.end());
+ // Pick all of the range checks with a specific base and length, and try to
+ // merge them.
+ Value *CurrentBase = Checks.front().Base;
+ Value *CurrentLength = Checks.front().Length;
+
+ SmallVector<GuardWideningImpl::RangeCheck, 3> CurrentChecks;
+
+ auto IsCurrentCheck = [&](GuardWideningImpl::RangeCheck &RC) {
+ return RC.Base == CurrentBase && RC.Length == CurrentLength;
+ };
+
+ std::copy_if(Checks.begin(), Checks.end(),
+ std::back_inserter(CurrentChecks), IsCurrentCheck);
+ Checks.erase(remove_if(Checks, IsCurrentCheck), Checks.end());
+
+ assert(CurrentChecks.size() != 0 && "We know we have at least one!");
+
+ if (CurrentChecks.size() < 3) {
+ RangeChecksOut.insert(RangeChecksOut.end(), CurrentChecks.begin(),
+ CurrentChecks.end());
continue;
}
- // CheckCount will typically be 3 here, but so far there has been no need to
- // hard-code that fact.
+ // CurrentChecks.size() will typically be 3 here, but so far there has been
+ // no need to hard-code that fact.
- std::sort(ChecksStart, Checks.end(),
+ std::sort(CurrentChecks.begin(), CurrentChecks.end(),
[&](const GuardWideningImpl::RangeCheck &LHS,
const GuardWideningImpl::RangeCheck &RHS) {
return LHS.Offset->getValue().slt(RHS.Offset->getValue());
@@ -578,8 +585,8 @@ bool GuardWideningImpl::combineRangeChec
// Note: std::sort should not invalidate the ChecksStart iterator.
- ConstantInt *MinOffset = ChecksStart->Offset,
- *MaxOffset = Checks.back().Offset;
+ ConstantInt *MinOffset = CurrentChecks.front().Offset,
+ *MaxOffset = CurrentChecks.back().Offset;
unsigned BitWidth = MaxOffset->getValue().getBitWidth();
if ((MaxOffset->getValue() - MinOffset->getValue())
@@ -593,7 +600,8 @@ bool GuardWideningImpl::combineRangeChec
};
if (MaxDiff.isMinValue() ||
- !std::all_of(std::next(ChecksStart), Checks.end(), OffsetOK))
+ !std::all_of(std::next(CurrentChecks.begin()), CurrentChecks.end(),
+ OffsetOK))
return false;
// We have a series of f+1 checks as:
@@ -631,12 +639,8 @@ bool GuardWideningImpl::combineRangeChec
// For Chk_0 to succeed, we'd have to have k_f-k_0 (the range highlighted
// with 'x' above) to be at least >u INT_MIN.
- RangeChecksOut.emplace_back(Base, MinOffset, Length,
- ChecksStart->CheckInst);
- RangeChecksOut.emplace_back(Base, MaxOffset, Length,
- Checks.back().CheckInst);
-
- Checks.erase(ChecksStart, Checks.end());
+ RangeChecksOut.emplace_back(CurrentChecks.front());
+ RangeChecksOut.emplace_back(CurrentChecks.back());
}
assert(RangeChecksOut.size() <= OldCount && "We pessimized!");
Modified: llvm/trunk/test/Transforms/GuardWidening/range-check-merging.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GuardWidening/range-check-merging.ll?rev=270306&r1=270305&r2=270306&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/GuardWidening/range-check-merging.ll (original)
+++ llvm/trunk/test/Transforms/GuardWidening/range-check-merging.ll Fri May 20 21:24:44 2016
@@ -194,4 +194,42 @@ entry:
ret void
}
+
+define void @f_7(i32 %x, i32* %length_buf) {
+; CHECK-LABEL: @f_7(
+
+; CHECK: [[COND_0:%[^ ]+]] = and i1 %chk3.b, %chk0.b
+; CHECK: [[COND_1:%[^ ]+]] = and i1 %chk0.a, [[COND_0]]
+; CHECK: [[COND_2:%[^ ]+]] = and i1 %chk3.a, [[COND_1]]
+; CHECK: call void (i1, ...) @llvm.experimental.guard(i1 [[COND_2]]) [ "deopt"() ]
+
+entry:
+ %length_a = load volatile i32, i32* %length_buf, !range !0
+ %length_b = load volatile i32, i32* %length_buf, !range !0
+ %chk0.a = icmp ult i32 %x, %length_a
+ %chk0.b = icmp ult i32 %x, %length_b
+ %chk0 = and i1 %chk0.a, %chk0.b
+ call void(i1, ...) @llvm.experimental.guard(i1 %chk0) [ "deopt"() ]
+
+ %x.inc1 = add i32 %x, 1
+ %chk1.a = icmp ult i32 %x.inc1, %length_a
+ %chk1.b = icmp ult i32 %x.inc1, %length_b
+ %chk1 = and i1 %chk1.a, %chk1.b
+ call void(i1, ...) @llvm.experimental.guard(i1 %chk1) [ "deopt"() ]
+
+ %x.inc2 = add i32 %x, 2
+ %chk2.a = icmp ult i32 %x.inc2, %length_a
+ %chk2.b = icmp ult i32 %x.inc2, %length_b
+ %chk2 = and i1 %chk2.a, %chk2.b
+ call void(i1, ...) @llvm.experimental.guard(i1 %chk2) [ "deopt"() ]
+
+ %x.inc3 = add i32 %x, 3
+ %chk3.a = icmp ult i32 %x.inc3, %length_a
+ %chk3.b = icmp ult i32 %x.inc3, %length_b
+ %chk3 = and i1 %chk3.a, %chk3.b
+ call void(i1, ...) @llvm.experimental.guard(i1 %chk3) [ "deopt"() ]
+ ret void
+}
+
+
!0 = !{i32 0, i32 2147483648}
More information about the llvm-commits
mailing list