[llvm] r247570 - [MergeFuncs] Fix bug in merging GetElementPointers

[David Blaikie via llvm-commits]

On Mon, Sep 14, 2015 at 8:37 AM, JF Bastien via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: jfb
> Date: Mon Sep 14 10:37:48 2015
> New Revision: 247570
>
> URL: http://llvm.org/viewvc/llvm-project?rev=247570&view=rev
> Log:
> [MergeFuncs] Fix bug in merging GetElementPointers
>
> GetElementPointers must have the first argument's type compared
> for structural equivalence. Previously the code erroneously compared the
> pointer's type, but this code was dead because all pointer types (of the
> same address space) are the same. The pointee must be compared instead
> (using the type stored in the GEP, not from the pointer type which will
> be erased anyway).
>

Thanks so much for being aware of this/using APIs that'll be forwards
compatible with the eventual removal of typed pointers.

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.


>
> Author: jrkoenig
> Reviewers: dschuff, nlewycky, jfb
> Subscribers: nlewycky, llvm-commits
> Differential revision: http://reviews.llvm.org/D12820
>
> Added:
>     llvm/trunk/test/Transforms/MergeFunc/gep-base-type.ll
> Modified:
>     llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp
>
> Modified: llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp?rev=247570&r1=247569&r2=247570&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp (original)
> +++ llvm/trunk/lib/Transforms/IPO/MergeFunctions.cpp Mon Sep 14 10:37:48
> 2015
> @@ -1028,8 +1028,8 @@ int FunctionComparator::cmpGEPs(const GE
>    if (GEPL->accumulateConstantOffset(DL, OffsetL) &&
>        GEPR->accumulateConstantOffset(DL, OffsetR))
>      return cmpAPInts(OffsetL, OffsetR);
> -  if (int Res = cmpTypes(GEPL->getPointerOperand()->getType(),
> -                         GEPR->getPointerOperand()->getType()))
> +  if (int Res = cmpTypes(GEPL->getSourceElementType(),
> +                         GEPR->getSourceElementType()))
>      return Res;
>
>    if (int Res = cmpNumbers(GEPL->getNumOperands(),
> GEPR->getNumOperands()))
>
> Added: llvm/trunk/test/Transforms/MergeFunc/gep-base-type.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/MergeFunc/gep-base-type.ll?rev=247570&view=auto
>
> ==============================================================================
> --- llvm/trunk/test/Transforms/MergeFunc/gep-base-type.ll (added)
> +++ llvm/trunk/test/Transforms/MergeFunc/gep-base-type.ll Mon Sep 14
> 10:37:48 2015
> @@ -0,0 +1,46 @@
> +; RUN: opt -mergefunc -S < %s | FileCheck %s
> +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"
> +
> +; These should not be merged, the type of the GEP pointer argument does
> not have
> +; the same stride.
> +
> +%"struct1" = type <{ i8*, i32, [4 x i8] }>
> +%"struct2" = type { i8*, { i64, i64 } }
> +
> +define internal %struct2* @Ffunc(%struct2* %P, i64 %i) {
> +; CHECK-LABEL: @Ffunc(
> +; CHECK-NEXT: getelementptr
> +; CHECK-NEXT: getelementptr
> +; CHECK-NEXT: getelementptr
> +; CHECK-NEXT: getelementptr
> +; CHECK-NEXT: getelementptr
> +; CHECK-NEXT: getelementptr
> +; CHECK-NEXT: ret
> +  %1 = getelementptr inbounds %"struct2", %"struct2"* %P, i64 %i
> +  %2 = getelementptr inbounds %"struct2", %"struct2"* %P, i64 %i
> +  %3 = getelementptr inbounds %"struct2", %"struct2"* %P, i64 %i
> +  %4 = getelementptr inbounds %"struct2", %"struct2"* %P, i64 %i
> +  %5 = getelementptr inbounds %"struct2", %"struct2"* %P, i64 %i
> +  %6 = getelementptr inbounds %"struct2", %"struct2"* %P, i64 %i
> +  ret %struct2* %6
> +}
> +
> +
> +define internal %struct1* @Gfunc(%struct1* %P, i64 %i) {
> +; CHECK-LABEL: @Gfunc(
> +; CHECK-NEXT: getelementptr
> +; CHECK-NEXT: getelementptr
> +; CHECK-NEXT: getelementptr
> +; CHECK-NEXT: getelementptr
> +; CHECK-NEXT: getelementptr
> +; CHECK-NEXT: getelementptr
> +; CHECK-NEXT: ret
> +  %1 = getelementptr inbounds %"struct1", %"struct1"* %P, i64 %i
> +  %2 = getelementptr inbounds %"struct1", %"struct1"* %P, i64 %i
> +  %3 = getelementptr inbounds %"struct1", %"struct1"* %P, i64 %i
> +  %4 = getelementptr inbounds %"struct1", %"struct1"* %P, i64 %i
> +  %5 = getelementptr inbounds %"struct1", %"struct1"* %P, i64 %i
> +  %6 = getelementptr inbounds %"struct1", %"struct1"* %P, i64 %i
> +  ret %struct1* %6
> +}
> +
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150914/0596fdf5/attachment.html>