<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">
Hi,
<div class=""><br class="">
</div>
<div class="">In my fix I don't look through addrspace casts which change pointer size. It’s just of one of a possible conservative fixes for the assert in GetPointerBaseWithConstantOffset. Since the semantics of different addrspaces is not defined, I guess
we can look through them even if the pointer size is changed. If you have a scenario when this kind of optimization matters, you can proceed with your solution. </div>
<div class=""><br class="">
</div>
<div class="">Note, that we have similar logic in Value::stripAndAccumulateInBoundsConstantOffsets and ValueTracking::isDereferenceableAndAlignedPointer. I have a patch for review which unifies their behavior. Can’t send a link right now because
<a href="http://reviews.llvm.org" class="">reviews.llvm.org</a> seems to be down. If you decide to look through addrspace cast even if it changes pointer size it will be nice to fix these two as well.</div>
<div class=""><br class="">
</div>
<div class="">Artur</div>
<div class=""><br class="">
<div>
<blockquote type="cite" class="">
<div class="">On 29 Sep 2016, at 00:12, Tom Stellard <<a href="mailto:tom@stellard.net" class="">tom@stellard.net</a>> wrote:</div>
<br class="Apple-interchange-newline">
<div class="">On Wed, Sep 28, 2016 at 05:57:19PM -0000, Artur Pilipenko via llvm-commits wrote:<br class="">
<blockquote type="cite" class="">Author: apilipenko<br class="">
Date: Wed Sep 28 12:57:16 2016<br class="">
New Revision: 282612<br class="">
<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=282612&view=rev" class="">http://llvm.org/viewvc/llvm-project?rev=282612&view=rev</a><br class="">
Log:<br class="">
Don't look through addrspacecast in GetPointerBaseWithConstantOffset<br class="">
<br class="">
Pointers in different addrspaces can have different sizes, so it's not valid to look through addrspace cast calculating base and offset for a value.<br class="">
<br class="">
</blockquote>
<br class="">
Hi,<br class="">
<br class="">
Why is not valid to look through addrspacecasts? I had proposed an alternative fix<br class="">
for this problem here: <a href="https://reviews.llvm.org/D24772" class="">https://reviews.llvm.org/D24772</a> which still allows this.<br class="">
<br class="">
-Tom<br class="">
<br class="">
<blockquote type="cite" class="">This is similar to D13008.<br class="">
<br class="">
Reviewed By: reames<br class="">
<br class="">
Differential Revision: <a href="https://reviews.llvm.org/D24729" class="">https://reviews.llvm.org/D24729</a><br class="">
<br class="">
Added:<br class="">
llvm/trunk/test/Transforms/GVN/PRE/rle-addrspace-cast.ll<br class="">
llvm/trunk/test/Transforms/GVN/addrspace-cast.ll<br class="">
Modified:<br class="">
llvm/trunk/lib/Analysis/ValueTracking.cpp<br class="">
llvm/trunk/test/Transforms/GVN/PRE/rle.ll<br class="">
<br class="">
Modified: llvm/trunk/lib/Analysis/ValueTracking.cpp<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ValueTracking.cpp?rev=282612&r1=282611&r2=282612&view=diff" class="">
http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ValueTracking.cpp?rev=282612&r1=282611&r2=282612&view=diff</a><br class="">
==============================================================================<br class="">
--- llvm/trunk/lib/Analysis/ValueTracking.cpp (original)<br class="">
+++ llvm/trunk/lib/Analysis/ValueTracking.cpp Wed Sep 28 12:57:16 2016<br class="">
@@ -2848,9 +2848,14 @@ Value *llvm::GetPointerBaseWithConstantO<br class="">
ByteOffset += GEPOffset;<br class="">
<br class="">
Ptr = GEP->getPointerOperand();<br class="">
- } else if (Operator::getOpcode(Ptr) == Instruction::BitCast ||<br class="">
- Operator::getOpcode(Ptr) == Instruction::AddrSpaceCast) {<br class="">
+ } else if (Operator::getOpcode(Ptr) == Instruction::BitCast) {<br class="">
Ptr = cast<Operator>(Ptr)->getOperand(0);<br class="">
+ } else if (AddrSpaceCastInst *ASCI = dyn_cast<AddrSpaceCastInst>(Ptr)) {<br class="">
+ Value *SourcePtr = ASCI->getPointerOperand();<br class="">
+ // Don't look through addrspace cast which changes pointer size<br class="">
+ if (BitWidth != DL.getPointerTypeSizeInBits(SourcePtr->getType()))<br class="">
+ break;<br class="">
+ Ptr = SourcePtr;<br class="">
} else if (GlobalAlias *GA = dyn_cast<GlobalAlias>(Ptr)) {<br class="">
if (GA->isInterposable())<br class="">
break;<br class="">
<br class="">
Added: llvm/trunk/test/Transforms/GVN/PRE/rle-addrspace-cast.ll<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GVN/PRE/rle-addrspace-cast.ll?rev=282612&view=auto" class="">
http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GVN/PRE/rle-addrspace-cast.ll?rev=282612&view=auto</a><br class="">
==============================================================================<br class="">
--- llvm/trunk/test/Transforms/GVN/PRE/rle-addrspace-cast.ll (added)<br class="">
+++ llvm/trunk/test/Transforms/GVN/PRE/rle-addrspace-cast.ll Wed Sep 28 12:57:16 2016<br class="">
@@ -0,0 +1,14 @@<br class="">
+; RUN: opt < %s -default-data-layout="e-p:32:32:32-p1:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:32:64-n8:16:32" -basicaa -gvn -S -die | FileCheck %s<br class="">
+<br class="">
+define i8 @coerce_offset0_addrspacecast(i32 %V, i32* %P) {<br class="">
+ store i32 %V, i32* %P<br class="">
+<br class="">
+ %P2 = addrspacecast i32* %P to i8 addrspace(1)*<br class="">
+ %P3 = getelementptr i8, i8 addrspace(1)* %P2, i32 2<br class="">
+<br class="">
+ %A = load i8, i8 addrspace(1)* %P3<br class="">
+ ret i8 %A<br class="">
+; CHECK-LABEL: @coerce_offset0_addrspacecast(<br class="">
+; CHECK-NOT: load<br class="">
+; CHECK: ret i8<br class="">
+}<br class="">
<br class="">
Modified: llvm/trunk/test/Transforms/GVN/PRE/rle.ll<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GVN/PRE/rle.ll?rev=282612&r1=282611&r2=282612&view=diff" class="">
http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GVN/PRE/rle.ll?rev=282612&r1=282611&r2=282612&view=diff</a><br class="">
==============================================================================<br class="">
--- llvm/trunk/test/Transforms/GVN/PRE/rle.ll (original)<br class="">
+++ llvm/trunk/test/Transforms/GVN/PRE/rle.ll Wed Sep 28 12:57:16 2016<br class="">
@@ -318,19 +318,6 @@ define i8 @coerce_offset0(i32 %V, i32* %<br class="">
; CHECK: ret i8<br class="">
}<br class="">
<br class="">
-define i8 @coerce_offset0_addrspacecast(i32 %V, i32* %P) {<br class="">
- store i32 %V, i32* %P<br class="">
-<br class="">
- %P2 = addrspacecast i32* %P to i8 addrspace(1)*<br class="">
- %P3 = getelementptr i8, i8 addrspace(1)* %P2, i32 2<br class="">
-<br class="">
- %A = load i8, i8 addrspace(1)* %P3<br class="">
- ret i8 %A<br class="">
-; CHECK-LABEL: @coerce_offset0_addrspacecast(<br class="">
-; CHECK-NOT: load<br class="">
-; CHECK: ret i8<br class="">
-}<br class="">
-<br class="">
;; non-local i32/float -> i8 load forwarding.<br class="">
define i8 @coerce_offset_nonlocal0(i32* %P, i1 %cond) {<br class="">
%P2 = bitcast i32* %P to float*<br class="">
<br class="">
Added: llvm/trunk/test/Transforms/GVN/addrspace-cast.ll<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GVN/addrspace-cast.ll?rev=282612&view=auto" class="">
http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GVN/addrspace-cast.ll?rev=282612&view=auto</a><br class="">
==============================================================================<br class="">
--- llvm/trunk/test/Transforms/GVN/addrspace-cast.ll (added)<br class="">
+++ llvm/trunk/test/Transforms/GVN/addrspace-cast.ll Wed Sep 28 12:57:16 2016<br class="">
@@ -0,0 +1,21 @@<br class="">
+; RUN: opt < %s -gvn -S | FileCheck %s<br class="">
+target datalayout = "e-m:e-p:16:16-p1:32:16-i32:16-i64:16-n8:16"<br class="">
+<br class="">
+; In cases where two address spaces do not have the same size pointer, the<br class="">
+; input for the addrspacecast should not be used as a substitute for itself<br class="">
+; when manipulating the pointer.<br class="">
+<br class="">
+; Check that we don't hit the assert in this scenario<br class="">
+define i8 @test(i32 %V, i32* %P) {<br class="">
+; CHECK-LABEL: @test(<br class="">
+; CHECK: load<br class="">
+ %P1 = getelementptr inbounds i32, i32* %P, i16 16<br class="">
+<br class="">
+ store i32 %V, i32* %P1<br class="">
+<br class="">
+ %P2 = addrspacecast i32* %P1 to i8 addrspace(1)*<br class="">
+ %P3 = getelementptr i8, i8 addrspace(1)* %P2, i32 2<br class="">
+<br class="">
+ %A = load i8, i8 addrspace(1)* %P3<br class="">
+ ret i8 %A<br class="">
+}<br class="">
<br class="">
<br class="">
_______________________________________________<br class="">
llvm-commits mailing list<br class="">
<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a><br class="">
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits<br class="">
</blockquote>
</div>
</blockquote>
</div>
<br class="">
</div>
</body>
</html>