Fwd: [PATCH] Teach MergeFunctions about address spaces
Stepan Dyatkovskiy
stpworld at narod.ru
Mon Sep 16 06:22:08 PDT 2013
Hi Sean,
The patch itself looks good. You put the difference between pointers
from different address spaces, you also had fixed GEP. That's good.
Though I have a question (perhaps not to you only).
Currently, MergeFunctions allows casts from from Pointer to Int, so we
mark next function types as equals (in case of 64bit memory address):
void foo(i64 ptrvalue)
void voo(i8* ptr)
How we should treat in this case pointer from addrspace(1)?
void voo2(i8 addrspace(1)* ptr)
Should it be equal to 'foo'? If so, taking into account transitivity of
'equal' it also would be equal to 'voo'.
-Stepan.
Sean Silva wrote:
> Hi Stepan,
>
> I think that it would be good if you reviewed this, since you're
> currently working on mergefunc.
>
> -- Sean Silva
>
> ---------- Forwarded message ----------
> From: *Matt Arsenault* <Matthew.Arsenault at amd.com
> <mailto:Matthew.Arsenault at amd.com>>
> Date: Fri, Sep 13, 2013 at 3:51 PM
> Subject: [PATCH] Teach MergeFunctions about address spaces
> To: Matthew.Arsenault at amd.com <mailto:Matthew.Arsenault at amd.com>
> Cc: llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>
>
> http://llvm-reviews.chandlerc.com/D1679
>
> Files:
> lib/Transforms/IPO/MergeFunctions.cpp
> test/Transforms/MergeFunc/address-spaces.ll
> test/Transforms/MergeFunc/inttoptr-address-space-2.ll
> test/Transforms/MergeFunc/inttoptr-address-space.ll
>
> Index: lib/Transforms/IPO/MergeFunctions.cpp
> ===================================================================
> --- lib/Transforms/IPO/MergeFunctions.cpp
> +++ lib/Transforms/IPO/MergeFunctions.cpp
> @@ -214,9 +214,12 @@
> return true;
> if (Ty1->getTypeID() != Ty2->getTypeID()) {
> if (TD) {
> - LLVMContext &Ctx = Ty1->getContext();
> - if (isa<PointerType>(Ty1) && Ty2 == TD->getIntPtrType(Ctx))
> return true;
> - if (isa<PointerType>(Ty2) && Ty1 == TD->getIntPtrType(Ctx))
> return true;
> +
> + if (isa<PointerType>(Ty1) && Ty2 == TD->getIntPtrType(Ty1))
> + return true;
> +
> + if (isa<PointerType>(Ty2) && Ty1 == TD->getIntPtrType(Ty2))
> + return true;
> }
> return false;
> }
> @@ -352,14 +355,19 @@
> // Determine whether two GEP operations perform the same underlying
> arithmetic.
> bool FunctionComparator::isEquivalentGEP(const GEPOperator *GEP1,
> const GEPOperator *GEP2) {
> - // When we have target data, we can reduce the GEP down to the value
> in bytes
> - // added to the address.
> - unsigned BitWidth = TD ? TD->getPointerSizeInBits() : 1;
> - APInt Offset1(BitWidth, 0), Offset2(BitWidth, 0);
> - if (TD &&
> - GEP1->accumulateConstantOffset(*TD, Offset1) &&
> - GEP2->accumulateConstantOffset(*TD, Offset2)) {
> - return Offset1 == Offset2;
> + unsigned AS = GEP1->getPointerAddressSpace();
> + if (AS != GEP2->getPointerAddressSpace())
> + return false;
> +
> + if (TD) {
> + // When we have target data, we can reduce the GEP down to the
> value in bytes
> + // added to the address.
> + unsigned BitWidth = TD ? TD->getPointerSizeInBits(AS) : 1;
> + APInt Offset1(BitWidth, 0), Offset2(BitWidth, 0);
> + if (GEP1->accumulateConstantOffset(*TD, Offset1) &&
> + GEP2->accumulateConstantOffset(*TD, Offset2)) {
> + return Offset1 == Offset2;
> + }
> }
>
> if (GEP1->getPointerOperand()->getType() !=
> Index: test/Transforms/MergeFunc/address-spaces.ll
> ===================================================================
> --- /dev/null
> +++ test/Transforms/MergeFunc/address-spaces.ll
> @@ -0,0 +1,35 @@
> +; RUN: opt -S -mergefunc < %s | FileCheck %s
> +
> +target datalayout = "p:32:32:32-p1:32:32:32-p2:16:16:16"
> +
> +declare void @foo(i32) nounwind
> +
> +; None of these functions should be merged
> +
> +define i32 @store_as0(i32* %x) {
> +; CHECK-LABEL: @store_as0(
> +; CHECK: call void @foo(
> + %gep = getelementptr i32* %x, i32 4
> + %y = load i32* %gep
> + call void @foo(i32 %y) nounwind
> + ret i32 %y
> +}
> +
> +define i32 @store_as1(i32 addrspace(1)* %x) {
> +; CHECK-LABEL: @store_as1(
> +; CHECK: call void @foo(
> + %gep = getelementptr i32 addrspace(1)* %x, i32 4
> + %y = load i32 addrspace(1)* %gep
> + call void @foo(i32 %y) nounwind
> + ret i32 %y
> +}
> +
> +define i32 @store_as2(i32 addrspace(2)* %x) {
> +; CHECK-LABEL: @store_as2(
> +; CHECK: call void @foo(
> + %gep = getelementptr i32 addrspace(2)* %x, i32 4
> + %y = load i32 addrspace(2)* %gep
> + call void @foo(i32 %y) nounwind
> + ret i32 %y
> +}
> +
> Index: test/Transforms/MergeFunc/inttoptr-address-space-2.ll
> ===================================================================
> --- /dev/null
> +++ test/Transforms/MergeFunc/inttoptr-address-space-2.ll
> @@ -0,0 +1,29 @@
> +; RUN: opt -mergefunc -S < %s | FileCheck %s
> +target datalayout =
> "e-p:32:32:32-p1:16:16:16-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:32:64-v64:64:64-n8:16:32-S128"
> +
> +%.qux.2496 = type { i16, %.qux.2497 }
> +%.qux.2497 = type { i8, i16 }
> +%.qux.2585 = type { i16, i16, i8 addrspace(1)* }
> +
> + at g2 = external addrspace(1) constant [9 x i8], align 1
> + at g3 = internal hidden addrspace(1) constant [1 x i8*] [i8* bitcast (i8
> addrspace(1)* (%.qux.2585 addrspace(1)*)* @func35 to i8*)]
> +
> +
> +define internal hidden i16 @func10(%.qux.2496 addrspace(1)* nocapture
> %this) align 2 {
> +bb:
> + %tmp = getelementptr inbounds %.qux.2496 addrspace(1)* %this, i32 0,
> i32 1, i32 1
> + %tmp1 = load i16 addrspace(1)* %tmp, align 4
> + ret i16 %tmp1
> +}
> +
> +; Checks that this can be merged with an address space differently
> sized than 0
> +define internal hidden i8 addrspace(1)* @func35(%.qux.2585
> addrspace(1)* nocapture %this) align 2 {
> +bb:
> +; CHECK-LABEL: @func35(
> +; CHECK: %[[V2:.+]] = bitcast %.qux.2585 addrspace(1)* %{{.*}} to
> %.qux.2496 addrspace(1)*
> +; CHECK: %[[V3:.+]] = tail call i16 @func10(%.qux.2496 addrspace(1)*
> %[[V2]])
> +; CHECK: %{{.*}} = inttoptr i16 %[[V3]] to i8 addrspace(1)*
> + %tmp = getelementptr inbounds %.qux.2585 addrspace(1)* %this, i32 0,
> i32 2
> + %tmp1 = load i8 addrspace(1)* addrspace(1)* %tmp, align 4
> + ret i8 addrspace(1)* %tmp1
> +}
> Index: test/Transforms/MergeFunc/inttoptr-address-space.ll
> ===================================================================
> --- /dev/null
> +++ test/Transforms/MergeFunc/inttoptr-address-space.ll
> @@ -0,0 +1,29 @@
> +; RUN: opt -mergefunc -S < %s | FileCheck %s
> +target datalayout =
> "e-p:32:32:32-p1:16:16:16-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:32:64-v64:64:64-n8:16:32-S128"
> +
> +%.qux.2496 = type { i32, %.qux.2497 }
> +%.qux.2497 = type { i8, i32 }
> +%.qux.2585 = type { i32, i32, i8* }
> +
> + at g2 = external addrspace(1) constant [9 x i8], align 1
> + at g3 = internal hidden unnamed_addr constant [1 x i8*] [i8* bitcast (i8*
> (%.qux.2585 addrspace(1)*)* @func35 to i8*)]
> +
> +
> +define internal hidden i32 @func10(%.qux.2496 addrspace(1)* nocapture
> %this) align 2 {
> +bb:
> + %tmp = getelementptr inbounds %.qux.2496 addrspace(1)* %this, i32 0,
> i32 1, i32 1
> + %tmp1 = load i32 addrspace(1)* %tmp, align 4
> + ret i32 %tmp1
> +}
> +
> +; Check for pointer bitwidth equal assertion failure
> +define internal hidden i8* @func35(%.qux.2585 addrspace(1)* nocapture
> %this) align 2 {
> +bb:
> +; CHECK-LABEL: @func35(
> +; CHECK: %[[V2:.+]] = bitcast %.qux.2585 addrspace(1)* %{{.*}} to
> %.qux.2496 addrspace(1)*
> +; CHECK: %[[V3:.+]] = tail call i32 @func10(%.qux.2496 addrspace(1)*
> %[[V2]])
> +; CHECK: %{{.*}} = inttoptr i32 %[[V3]] to i8*
> + %tmp = getelementptr inbounds %.qux.2585 addrspace(1)* %this, i32 0,
> i32 2
> + %tmp1 = load i8* addrspace(1)* %tmp, align 4
> + ret i8* %tmp1
> +}
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
More information about the llvm-commits
mailing list