maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

[ValueTracking] Fix a misuse of APInt in GetPointerBaseWithConstantOffset 

GetPointerBaseWithConstantOffset include this code, where ByteOffset
and GEPOffset are both of type llvm::APInt :

  ByteOffset += GEPOffset.getSExtValue();

The problem with this line is that getSExtValue() returns an int64_t, but
the += matches an overload for uint64_t. The problem is that the resulting
APInt is no longer considered to be signed. That in turn causes assertion
failures later on if the relevant pointer type is > 64 bits in width and
the GEPOffset was negative.

Changing it to

  ByteOffset += GEPOffset.sextOrTrunc(ByteOffset.getBitWidth());

resolves the issue and explicitly performs the sign-extending
or truncation. Additionally, instead of asserting later if the result
is > 64 bits, it breaks out of the loop in that case.

See also
 https://reviews.llvm.org/D24729
 https://reviews.llvm.org/D24772

This commit must be merged after D38662 in order for the test to pass.

Patch by Michael Ferguson <mpfergu@gmail.com>.

Reviewers: reames, sanjoy, hfinkel

Reviewed By: hfinkel

Differential Revision: https://reviews.llvm.org/D38501

llvm-svn: 350395
This commit is contained in:
Florian Hahn 2019-01-04 14:53:22 +00:00
parent da2a76562d
commit 7902405c42
2 changed files with 51 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
llvm/lib/Analysis/ValueTracking.cpp
View File

@ -3389,7 +3389,14 @@ Value *llvm::GetPointerBaseWithConstantOffset(Value *Ptr, int64_t &Offset,
if (!GEP->accumulateConstantOffset(DL, GEPOffset))
break;
ByteOffset += GEPOffset.getSExtValue();
APInt OrigByteOffset(ByteOffset);
ByteOffset += GEPOffset.sextOrTrunc(ByteOffset.getBitWidth());
if (ByteOffset.getMinSignedBits() > 64) {
// Stop traversal if the pointer offset wouldn't fit into int64_t
// (this should be removed if Offset is updated to an APInt)
ByteOffset = OrigByteOffset;
break;
}
Ptr = GEP->getPointerOperand();
} else if (Operator::getOpcode(Ptr) == Instruction::BitCast ||

43
llvm/test/Analysis/ValueTracking/gep-negative-issue.ll Normal file
View File

@ -0,0 +1,43 @@
; RUN: opt -gvn -S < %s | FileCheck %s
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128-p100:128:64:64-p101:128:64:64"
target triple = "x86_64-unknown-linux-gnu"
%ArrayImpl = type { i64, i64 addrspace(100)*, [1 x i64], [1 x i64], [1 x i64], i64, i64, double addrspace(100)*, double addrspace(100)*, i8, i64 }
%_array = type { i64, %ArrayImpl addrspace(100)*, i8 }
define void @test(i64 %n_chpl) {
entry:
; First section is some code
%0 = getelementptr inbounds %_array, %_array* null, i32 0, i32 1
%1 = load %ArrayImpl addrspace(100)*, %ArrayImpl addrspace(100)** %0
%2 = getelementptr inbounds %ArrayImpl, %ArrayImpl addrspace(100)* %1, i32 0, i32 8
%3 = load double addrspace(100)*, double addrspace(100)* addrspace(100)* %2
%4 = getelementptr inbounds double, double addrspace(100)* %3, i64 -1
; Second section is that code repeated
%x0 = getelementptr inbounds %_array, %_array* null, i32 0, i32 1
%x1 = load %ArrayImpl addrspace(100)*, %ArrayImpl addrspace(100)** %x0
%x2 = getelementptr inbounds %ArrayImpl, %ArrayImpl addrspace(100)* %x1, i32 0, i32 8
%x3 = load double addrspace(100)*, double addrspace(100)* addrspace(100)* %x2
%x4 = getelementptr inbounds double, double addrspace(100)* %x3, i64 -1
; Stores that should be to the same location
store double 0.000000e+00, double addrspace(100)* %4
store double 0.000000e+00, double addrspace(100)* %x4
; Third section is the repeated code again, with a later store
; This third section is necessary to trigger the crash
%y1 = load %ArrayImpl addrspace(100)*, %ArrayImpl addrspace(100)** %0
%y2 = getelementptr inbounds %ArrayImpl, %ArrayImpl addrspace(100)* %y1, i32 0, i32 8
%y3 = load double addrspace(100)*, double addrspace(100)* addrspace(100)* %y2
%y4 = getelementptr inbounds double, double addrspace(100)* %y3, i64 -1
store double 0.000000e+00, double addrspace(100)* %y4
ret void
; CHECK-LABEL: define void @test
; CHECK: getelementptr inbounds double, double addrspace(100)* {{%.*}}, i64 -1
; CHECK-NEXT: store double 0.000000e+00, double addrspace(100)* [[DST:%.*]]
; CHECK-NEXT: store double 0.000000e+00, double addrspace(100)* [[DST]]
; CHECK: load
; CHECK: getelementptr inbounds %ArrayImpl, %ArrayImpl addrspace(100)*
; CHECK: load
; CHECK: getelementptr inbounds double, double addrspace(100)* {{%.*}}, i64 -1
; CHECK: store double 0.000000e+00, double addrspace(100)*
; CHECK: ret
}