<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Sep 14, 2015 at 8:37 AM, JF Bastien via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: jfb<br>
Date: Mon Sep 14 10:37:48 2015<br>
New Revision: 247570<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=247570&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=247570&view=rev</a><br>
Log:<br>
[MergeFuncs] Fix bug in merging GetElementPointers<br>
<br>
GetElementPointers must have the first argument's type compared<br>
for structural equivalence. Previously the code erroneously compared the<br>
pointer's type, but this code was dead because all pointer types (of the<br>
same address space) are the same. The pointee must be compared instead<br>
(using the type stored in the GEP, not from the pointer type which will<br>
be erased anyway).<br></blockquote><div><br>Thanks so much for being aware of this/using APIs that'll be forwards compatible with the eventual removal of typed pointers.<br><br>Just in case: do you care about the GEP pointer operands address space (I assume not) or vector types (eg: a gep over a vector of pointers)? I assume not, but just checking - both those aspects of the pointer operand are still in the pointer operand's type, not in the source element type.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Author: jrkoenig<br>
Reviewers: dschuff, nlewycky, jfb<br>
Subscribers: nlewycky, llvm-commits<br>
Differential revision: <a href="http://reviews.llvm.org/D12820" rel="noreferrer" target="_blank">http://reviews.llvm.org/D12820</a><br>
<br>
Added:<br>
llvm/trunk/test/Transforms/MergeFunc/gep-base-type.ll<br>
Modified:<br>
llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp<br>
<br>
Modified: llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp?rev=247570&r1=247569&r2=247570&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp?rev=247570&r1=247569&r2=247570&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp Mon Sep 14 10:37:48 2015<br>
@@ -1028,8 +1028,8 @@ int FunctionComparator::cmpGEPs(const GE<br>
if (GEPL->accumulateConstantOffset(DL, OffsetL) &&<br>
GEPR->accumulateConstantOffset(DL, OffsetR))<br>
return cmpAPInts(OffsetL, OffsetR);<br>
- if (int Res = cmpTypes(GEPL->getPointerOperand()->getType(),<br>
- GEPR->getPointerOperand()->getType()))<br>
+ if (int Res = cmpTypes(GEPL->getSourceElementType(),<br>
+ GEPR->getSourceElementType()))<br>
return Res;<br>
<br>
if (int Res = cmpNumbers(GEPL->getNumOperands(), GEPR->getNumOperands()))<br>
<br>
Added: llvm/trunk/test/Transforms/MergeFunc/gep-base-type.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/MergeFunc/gep-base-type.ll?rev=247570&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/MergeFunc/gep-base-type.ll?rev=247570&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/MergeFunc/gep-base-type.ll (added)<br>
+++ llvm/trunk/test/Transforms/MergeFunc/gep-base-type.ll Mon Sep 14 10:37:48 2015<br>
@@ -0,0 +1,46 @@<br>
+; RUN: opt -mergefunc -S < %s | FileCheck %s<br>
+target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"<br>
+<br>
+; These should not be merged, the type of the GEP pointer argument does not have<br>
+; the same stride.<br>
+<br>
+%"struct1" = type <{ i8*, i32, [4 x i8] }><br>
+%"struct2" = type { i8*, { i64, i64 } }<br>
+<br>
+define internal %struct2* @Ffunc(%struct2* %P, i64 %i) {<br>
+; CHECK-LABEL: @Ffunc(<br>
+; CHECK-NEXT: getelementptr<br>
+; CHECK-NEXT: getelementptr<br>
+; CHECK-NEXT: getelementptr<br>
+; CHECK-NEXT: getelementptr<br>
+; CHECK-NEXT: getelementptr<br>
+; CHECK-NEXT: getelementptr<br>
+; CHECK-NEXT: ret<br>
+ %1 = getelementptr inbounds %"struct2", %"struct2"* %P, i64 %i<br>
+ %2 = getelementptr inbounds %"struct2", %"struct2"* %P, i64 %i<br>
+ %3 = getelementptr inbounds %"struct2", %"struct2"* %P, i64 %i<br>
+ %4 = getelementptr inbounds %"struct2", %"struct2"* %P, i64 %i<br>
+ %5 = getelementptr inbounds %"struct2", %"struct2"* %P, i64 %i<br>
+ %6 = getelementptr inbounds %"struct2", %"struct2"* %P, i64 %i<br>
+ ret %struct2* %6<br>
+}<br>
+<br>
+<br>
+define internal %struct1* @Gfunc(%struct1* %P, i64 %i) {<br>
+; CHECK-LABEL: @Gfunc(<br>
+; CHECK-NEXT: getelementptr<br>
+; CHECK-NEXT: getelementptr<br>
+; CHECK-NEXT: getelementptr<br>
+; CHECK-NEXT: getelementptr<br>
+; CHECK-NEXT: getelementptr<br>
+; CHECK-NEXT: getelementptr<br>
+; CHECK-NEXT: ret<br>
+ %1 = getelementptr inbounds %"struct1", %"struct1"* %P, i64 %i<br>
+ %2 = getelementptr inbounds %"struct1", %"struct1"* %P, i64 %i<br>
+ %3 = getelementptr inbounds %"struct1", %"struct1"* %P, i64 %i<br>
+ %4 = getelementptr inbounds %"struct1", %"struct1"* %P, i64 %i<br>
+ %5 = getelementptr inbounds %"struct1", %"struct1"* %P, i64 %i<br>
+ %6 = getelementptr inbounds %"struct1", %"struct1"* %P, i64 %i<br>
+ ret %struct1* %6<br>
+}<br>
+<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div></div>