<div dir="ltr">Thanks. Patches coming. </div><div class="gmail_extra"><br><div class="gmail_quote">On Sat, Jul 9, 2016 at 1:25 PM, Arsenault, Matthew <span dir="ltr"><<a href="mailto:Matthew.Arsenault@amd.com" target="_blank">Matthew.Arsenault@amd.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">



<div>
<div style="padding-left:16px;padding-right:16px;padding-bottom:8px">
<div>The getPointerSizeInBits call is missing the address space </div>
<div><br>
</div>
<div>-Matt<br>
<br>
<div></div>
<br>
</div>
</div>
<div class="gmail_quote">_____________________________<br>
From: Jingyue Wu via llvm-commits <<a dir="ltr" href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>><br>
Sent: Saturday, July 9, 2016 12:20<br>
Subject: [llvm] r274982 - [SLSR] Fix crash on handling 128-bit integers.<br>
To: <<a dir="ltr" href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>><div><div class="h5"><br>
<br>
<br>
Author: jingyue<br>
Date: Sat Jul 9 14:13:18 2016<br>
New Revision: 274982<br>
<br>
URL: <a dir="ltr" href="http://llvm.org/viewvc/llvm-project?rev=274982&view=rev" target="_blank">
http://llvm.org/viewvc/llvm-project?rev=274982&view=rev</a><br>
Log:<br>
[SLSR] Fix crash on handling 128-bit integers.<br>
<br>
ConstantInt::getSExtValue may fail on >64-bit integers. Add checks to call<br>
getSExtValue only on narrow integers.<br>
<br>
As a minor aside, simplify slsr-gep.ll to remove unnecessary load instructions.<br>
<br>
Modified:<br>
llvm/trunk/lib/Transforms/Scalar/StraightLineStrengthReduce.cpp<br>
llvm/trunk/test/Transforms/StraightLineStrengthReduce/slsr-add.ll<br>
llvm/trunk/test/Transforms/StraightLineStrengthReduce/slsr-gep.ll<br>
<br>
Modified: llvm/trunk/lib/Transforms/Scalar/StraightLineStrengthReduce.cpp<br>
URL: <a dir="ltr" href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/StraightLineStrengthReduce.cpp?rev=274982&r1=274981&r2=274982&view=diff" target="_blank">
http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/StraightLineStrengthReduce.cpp?rev=274982&r1=274981&r2=274982&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/Scalar/StraightLineStrengthReduce.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/Scalar/StraightLineStrengthReduce.cpp Sat Jul 9 14:13:18 2016<br>
@@ -246,7 +246,9 @@ static bool isGEPFoldable(GetElementPtrI<br>
// Returns whether (Base + Index * Stride) can be folded to an addressing mode.<br>
static bool isAddFoldable(const SCEV *Base, ConstantInt *Index, Value *Stride,<br>
TargetTransformInfo *TTI) {<br>
- return TTI->isLegalAddressingMode(Base->getType(), nullptr, 0, true,<br>
+ // Index->getSExtValue() may crash if Index is wider than 64-bit.<br>
+ return Index->getBitWidth() <= 64 &&<br>
+ TTI->isLegalAddressingMode(Base->getType(), nullptr, 0, true,<br>
Index->getSExtValue(), UnknownAddressSpace);<br>
}<br>
<br>
@@ -502,13 +504,23 @@ void StraightLineStrengthReduce::allocat<br>
IndexExprs, GEP->isInBounds());<br>
Value *ArrayIdx = GEP->getOperand(I);<br>
uint64_t ElementSize = DL->getTypeAllocSize(*GTI);<br>
- factorArrayIndex(ArrayIdx, BaseExpr, ElementSize, GEP);<br>
+ if (ArrayIdx->getType()->getIntegerBitWidth() <=<br>
+ DL->getPointerSizeInBits()) {<br>
+ // Skip factoring if ArrayIdx is wider than the pointer size, because<br>
+ // ArrayIdx is implicitly truncated to the pointer size.<br>
+ factorArrayIndex(ArrayIdx, BaseExpr, ElementSize, GEP);<br>
+ }<br>
// When ArrayIdx is the sext of a value, we try to factor that value as<br>
// well. Handling this case is important because array indices are<br>
// typically sign-extended to the pointer size.<br>
Value *TruncatedArrayIdx = nullptr;<br>
- if (match(ArrayIdx, m_SExt(m_Value(TruncatedArrayIdx))))<br>
+ if (match(ArrayIdx, m_SExt(m_Value(TruncatedArrayIdx))) &&<br>
+ TruncatedArrayIdx->getType()->getIntegerBitWidth() <=<br>
+ DL->getPointerSizeInBits()) {<br>
+ // Skip factoring if TruncatedArrayIdx is wider than the pointer size,<br>
+ // because TruncatedArrayIdx is implicitly truncated to the pointer size.<br>
factorArrayIndex(TruncatedArrayIdx, BaseExpr, ElementSize, GEP);<br>
+ }<br>
<br>
IndexExprs[I - 1] = OrigIndexExpr;<br>
}<br>
@@ -535,10 +547,11 @@ Value *StraightLineStrengthReduce::emitB<br>
if (Basis.CandidateKind == Candidate::GEP) {<br>
APInt ElementSize(<br>
IndexOffset.getBitWidth(),<br>
- DL->getTypeAllocSize(cast<GetElementPtrInst>(Basis.Ins)->getResultElementType()));<br>
+ DL->getTypeAllocSize(<br>
+ cast<GetElementPtrInst>(Basis.Ins)->getResultElementType()));<br>
APInt Q, R;<br>
APInt::sdivrem(IndexOffset, ElementSize, Q, R);<br>
- if (R.getSExtValue() == 0)<br>
+ if (R == 0)<br>
IndexOffset = Q;<br>
else<br>
BumpWithUglyGEP = true;<br>
@@ -546,10 +559,10 @@ Value *StraightLineStrengthReduce::emitB<br>
<br>
// Compute Bump = C - Basis = (i' - i) * S.<br>
// Common case 1: if (i' - i) is 1, Bump = S.<br>
- if (IndexOffset.getSExtValue() == 1)<br>
+ if (IndexOffset == 1)<br>
return C.Stride;<br>
// Common case 2: if (i' - i) is -1, Bump = -S.<br>
- if (IndexOffset.getSExtValue() == -1)<br>
+ if (IndexOffset.isAllOnesValue())<br>
return Builder.CreateNeg(C.Stride);<br>
<br>
// Otherwise, Bump = (i' - i) * sext/trunc(S). Note that (i' - i) and S may<br>
<br>
Modified: llvm/trunk/test/Transforms/StraightLineStrengthReduce/slsr-add.ll<br>
URL: <a dir="ltr" href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/StraightLineStrengthReduce/slsr-add.ll?rev=274982&r1=274981&r2=274982&view=diff" target="_blank">
http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/StraightLineStrengthReduce/slsr-add.ll?rev=274982&r1=274981&r2=274982&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/StraightLineStrengthReduce/slsr-add.ll (original)<br>
+++ llvm/trunk/test/Transforms/StraightLineStrengthReduce/slsr-add.ll Sat Jul 9 14:13:18 2016<br>
@@ -98,4 +98,19 @@ define void @simple_enough(i32 %b, i32 %<br>
ret void<br>
}<br>
<br>
+define void @slsr_strided_add_128bit(i128 %b, i128 %s) {<br>
+; CHECK-LABEL: @slsr_strided_add_128bit(<br>
+ %s125 = shl i128 %s, 125<br>
+ %s126 = shl i128 %s, 126<br>
+ %1 = add i128 %b, %s125<br>
+; CHECK: [[t1:%[a-zA-Z0-9]+]] = add i128 %b, %s125<br>
+ call void @bar(i128 %1)<br>
+ %2 = add i128 %b, %s126<br>
+; CHECK: [[t2:%[a-zA-Z0-9]+]] = add i128 [[t1]], %s125<br>
+ call void @bar(i128 %2)<br>
+; CHECK: call void @bar(i128 [[t2]])<br>
+ ret void<br>
+}<br>
+<br>
declare void @foo(i32)<br>
+declare void @bar(i128)<br>
<br>
Modified: llvm/trunk/test/Transforms/StraightLineStrengthReduce/slsr-gep.ll<br>
URL: <a dir="ltr" href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/StraightLineStrengthReduce/slsr-gep.ll?rev=274982&r1=274981&r2=274982&view=diff" target="_blank">
http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/StraightLineStrengthReduce/slsr-gep.ll?rev=274982&r1=274981&r2=274982&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/StraightLineStrengthReduce/slsr-gep.ll (original)<br>
+++ llvm/trunk/test/Transforms/StraightLineStrengthReduce/slsr-gep.ll Sat Jul 9 14:13:18 2016<br>
@@ -16,21 +16,18 @@ define void @slsr_gep(i32* %input, i64 %<br>
; CHECK-LABEL: @slsr_gep(<br>
; v0 = input[0];<br>
%p0 = getelementptr inbounds i32, i32* %input, i64 0<br>
- %v0 = load i32, i32* %p0<br>
- call void @foo(i32 %v0)<br>
+ call void @foo(i32* %p0)<br>
<br>
; v1 = input[s];<br>
%p1 = getelementptr inbounds i32, i32* %input, i64 %s<br>
; CHECK: %p1 = getelementptr inbounds i32, i32* %input, i64 %s<br>
- %v1 = load i32, i32* %p1<br>
- call void @foo(i32 %v1)<br>
+ call void @foo(i32* %p1)<br>
<br>
; v2 = input[s * 2];<br>
%s2 = shl nsw i64 %s, 1<br>
%p2 = getelementptr inbounds i32, i32* %input, i64 %s2<br>
; CHECK: %p2 = getelementptr inbounds i32, i32* %p1, i64 %s<br>
- %v2 = load i32, i32* %p2<br>
- call void @foo(i32 %v2)<br>
+ call void @foo(i32* %p2)<br>
<br>
ret void<br>
}<br>
@@ -49,23 +46,20 @@ define void @slsr_gep_sext(i32* %input,<br>
; CHECK-LABEL: @slsr_gep_sext(<br>
; v0 = input[0];<br>
%p0 = getelementptr inbounds i32, i32* %input, i64 0<br>
- %v0 = load i32, i32* %p0<br>
- call void @foo(i32 %v0)<br>
+ call void @foo(i32* %p0)<br>
<br>
; v1 = input[s];<br>
%t = sext i32 %s to i64<br>
%p1 = getelementptr inbounds i32, i32* %input, i64 %t<br>
; CHECK: %p1 = getelementptr inbounds i32, i32* %input, i64 %t<br>
- %v1 = load i32, i32* %p1<br>
- call void @foo(i32 %v1)<br>
+ call void @foo(i32* %p1)<br>
<br>
; v2 = input[s * 2];<br>
%s2 = shl nsw i32 %s, 1<br>
%t2 = sext i32 %s2 to i64<br>
%p2 = getelementptr inbounds i32, i32* %input, i64 %t2<br>
; CHECK: %p2 = getelementptr inbounds i32, i32* %p1, i64 %t<br>
- %v2 = load i32, i32* %p2<br>
- call void @foo(i32 %v2)<br>
+ call void @foo(i32* %p2)<br>
<br>
ret void<br>
}<br>
@@ -85,23 +79,20 @@ define void @slsr_gep_2d([10 x [5 x i32]<br>
; CHECK-LABEL: @slsr_gep_2d(<br>
; v0 = input[s][t];<br>
%p0 = getelementptr inbounds [10 x [5 x i32]], [10 x [5 x i32]]* %input, i64 0, i64 %s, i64 %t<br>
- %v0 = load i32, i32* %p0<br>
- call void @foo(i32 %v0)<br>
+ call void @foo(i32* %p0)<br>
<br>
; v1 = input[s * 2][t];<br>
%s2 = shl nsw i64 %s, 1<br>
; CHECK: [[BUMP:%[a-zA-Z0-9]+]] = mul i64 %s, 5<br>
%p1 = getelementptr inbounds [10 x [5 x i32]], [10 x [5 x i32]]* %input, i64 0, i64 %s2, i64 %t<br>
; CHECK: %p1 = getelementptr inbounds i32, i32* %p0, i64 [[BUMP]]<br>
- %v1 = load i32, i32* %p1<br>
- call void @foo(i32 %v1)<br>
+ call void @foo(i32* %p1)<br>
<br>
; v3 = input[s * 3][t];<br>
%s3 = mul nsw i64 %s, 3<br>
%p2 = getelementptr inbounds [10 x [5 x i32]], [10 x [5 x i32]]* %input, i64 0, i64 %s3, i64 %t<br>
; CHECK: %p2 = getelementptr inbounds i32, i32* %p1, i64 [[BUMP]]<br>
- %v2 = load i32, i32* %p2<br>
- call void @foo(i32 %v2)<br>
+ call void @foo(i32* %p2)<br>
<br>
ret void<br>
}<br>
@@ -118,23 +109,20 @@ define void @slsr_gep_uglygep([10 x [5 x<br>
; CHECK-LABEL: @slsr_gep_uglygep(<br>
; v0 = input[s][t].f1;<br>
%p0 = getelementptr inbounds [10 x [5 x %struct.S]], [10 x [5 x %struct.S]]* %input, i64 0, i64 %s, i64 %t, i32 0<br>
- %v0 = load i64, i64* %p0<br>
- call void @bar(i64 %v0)<br>
+ call void @bar(i64* %p0)<br>
<br>
; v1 = input[s * 2][t].f1;<br>
%s2 = shl nsw i64 %s, 1<br>
; CHECK: [[BUMP:%[a-zA-Z0-9]+]] = mul i64 %s, 60<br>
%p1 = getelementptr inbounds [10 x [5 x %struct.S]], [10 x [5 x %struct.S]]* %input, i64 0, i64 %s2, i64 %t, i32 0<br>
; CHECK: getelementptr inbounds i8, i8* %{{[0-9]+}}, i64 [[BUMP]]<br>
- %v1 = load i64, i64* %p1<br>
- call void @bar(i64 %v1)<br>
+ call void @bar(i64* %p1)<br>
<br>
; v2 = input[s * 3][t].f1;<br>
%s3 = mul nsw i64 %s, 3<br>
%p2 = getelementptr inbounds [10 x [5 x %struct.S]], [10 x [5 x %struct.S]]* %input, i64 0, i64 %s3, i64 %t, i32 0<br>
; CHECK: getelementptr inbounds i8, i8* %{{[0-9]+}}, i64 [[BUMP]]<br>
- %v2 = load i64, i64* %p2<br>
- call void @bar(i64 %v2)<br>
+ call void @bar(i64* %p2)<br>
<br>
ret void<br>
}<br>
@@ -143,26 +131,44 @@ define void @slsr_out_of_bounds_gep(i32*<br>
; CHECK-LABEL: @slsr_out_of_bounds_gep(<br>
; v0 = input[0];<br>
%p0 = getelementptr i32, i32* %input, i64 0<br>
- %v0 = load i32, i32* %p0<br>
- call void @foo(i32 %v0)<br>
+ call void @foo(i32* %p0)<br>
<br>
; v1 = input[(long)s];<br>
%t = sext i32 %s to i64<br>
%p1 = getelementptr i32, i32* %input, i64 %t<br>
; CHECK: %p1 = getelementptr i32, i32* %input, i64 %t<br>
- %v1 = load i32, i32* %p1<br>
- call void @foo(i32 %v1)<br>
+ call void @foo(i32* %p1)<br>
<br>
; v2 = input[(long)(s * 2)];<br>
%s2 = shl nsw i32 %s, 1<br>
%t2 = sext i32 %s2 to i64<br>
%p2 = getelementptr i32, i32* %input, i64 %t2<br>
; CHECK: %p2 = getelementptr i32, i32* %p1, i64 %t<br>
- %v2 = load i32, i32* %p2<br>
- call void @foo(i32 %v2)<br>
+ call void @foo(i32* %p2)<br>
<br>
ret void<br>
}<br>
<br>
-declare void @foo(i32)<br>
-declare void @bar(i64)<br>
+define void @slsr_gep_128bit(i32* %input, i128 %s) {<br>
+; CHECK-LABEL: @slsr_gep_128bit(<br>
+ ; p0 = &input[0]<br>
+ %p0 = getelementptr inbounds i32, i32* %input, i128 0<br>
+ call void @foo(i32* %p0)<br>
+<br>
+ ; p1 = &input[s << 125]<br>
+ %s125 = shl nsw i128 %s, 125<br>
+ %p1 = getelementptr inbounds i32, i32* %input, i128 %s125<br>
+; CHECK: %p1 = getelementptr inbounds i32, i32* %input, i128 %s125<br>
+ call void @foo(i32* %p1)<br>
+<br>
+ ; p2 = &input[s << 126]<br>
+ %s126 = shl nsw i128 %s, 126<br>
+ %p2 = getelementptr inbounds i32, i32* %input, i128 %s126<br>
+; CHECK: %p2 = getelementptr inbounds i32, i32* %input, i128 %s126<br>
+ call void @foo(i32* %p2)<br>
+<br>
+ ret void<br>
+}<br>
+<br>
+declare void @foo(i32*)<br>
+declare void @bar(i64*)<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a dir="ltr" href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a dir="ltr" href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
<br>
<br>
</div></div></div>
</div>

</blockquote></div><br></div>