RPCS3/llvm-mirror
1
0
Fork 0
mirror of https://github.com/RPCS3/llvm-mirror.git synced 2025-01-31 20:51:52 +01:00
Code Issues Projects Releases Wiki Activity

[GuardWidening] Fix incorrect use of remove_if

Browse Source 
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.

llvm-svn: 270306
This commit is contained in:
Sanjoy Das 2016-05-21 02:24:44 +00:00
parent 44230570f6
commit 9b7c91f0ad
2 changed files with 65 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

50
lib/Transforms/Scalar/GuardWidening.cpp
View File

@ -551,26 +551,33 @@ bool GuardWideningImpl::combineRangeChecks(
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;
});
// 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;
unsigned CheckCount = std::distance(ChecksStart, Checks.end());
assert(CheckCount != 0 && "We know we have at least one!");
SmallVector<GuardWideningImpl::RangeCheck, 3> CurrentChecks;
if (CheckCount < 3) {
RangeChecksOut.insert(RangeChecksOut.end(), ChecksStart, Checks.end());
Checks.erase(ChecksStart, Checks.end());
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::combineRangeChecks(
// 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::combineRangeChecks(
};
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::combineRangeChecks(
// 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!");

38
test/Transforms/GuardWidening/range-check-merging.ll
View File

@ -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}