[llvm] cfe8f8e - Revert "Mark gc.relocate and gc.result as readnone"
Philip Reames via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 12 12:52:06 PST 2021
Thanks for the revert here. I added a comment to the original review
with links to the fallout for context.
Once you get a chance, I'd like to see a test case for the LICM issue.
I think I can guess what's going on from your commit comment, but I'd
like to confirm.
Philip
On 3/12/21 1:59 AM, Serguei Katkov via llvm-commits wrote:
> Author: Serguei Katkov
> Date: 2021-03-12T16:59:17+07:00
> New Revision: cfe8f8e0f010077f5942bce88a2fd331b90ccea7
>
> URL: https://github.com/llvm/llvm-project/commit/cfe8f8e0f010077f5942bce88a2fd331b90ccea7
> DIFF: https://github.com/llvm/llvm-project/commit/cfe8f8e0f010077f5942bce88a2fd331b90ccea7.diff
>
> LOG: Revert "Mark gc.relocate and gc.result as readnone"
>
> As readnone function they become movable and LICM can hoist them
> out of a loop. As a result in LCSSA form phi node of type token
> is created. No one is ready that GCRelocate first operand is phi node
> but expects to be token.
>
> GVN test were also updated, it seems it does not do what is expected.
> Test for LICM is also added.
>
> This reverts commit f352463ade6e49c3b0275f296d9190d828b7630b.
>
> Added:
> llvm/test/Transforms/LICM/gc-relocate.ll
>
> Modified:
> llvm/include/llvm/IR/Intrinsics.td
> llvm/lib/Transforms/Scalar/EarlyCSE.cpp
> llvm/test/Transforms/EarlyCSE/gc_relocate.ll
> llvm/test/Transforms/GVN/gc_relocate.ll
>
> Removed:
>
>
>
> ################################################################################
> diff --git a/llvm/include/llvm/IR/Intrinsics.td b/llvm/include/llvm/IR/Intrinsics.td
> index 9102b97480bd..4bbffdd0b734 100644
> --- a/llvm/include/llvm/IR/Intrinsics.td
> +++ b/llvm/include/llvm/IR/Intrinsics.td
> @@ -1182,11 +1182,11 @@ def int_experimental_gc_statepoint : Intrinsic<[llvm_token_ty],
> ImmArg<ArgIndex<4>>]>;
>
> def int_experimental_gc_result : Intrinsic<[llvm_any_ty], [llvm_token_ty],
> - [IntrNoMem]>;
> + [IntrReadMem]>;
> def int_experimental_gc_relocate : Intrinsic<[llvm_any_ty],
> [llvm_token_ty, llvm_i32_ty,
> llvm_i32_ty],
> - [IntrNoMem, ImmArg<ArgIndex<1>>,
> + [IntrReadMem, ImmArg<ArgIndex<1>>,
> ImmArg<ArgIndex<2>>]>;
>
> //===------------------------ Coroutine Intrinsics ---------------===//
>
> diff --git a/llvm/lib/Transforms/Scalar/EarlyCSE.cpp b/llvm/lib/Transforms/Scalar/EarlyCSE.cpp
> index 125e00482210..180a82917fa9 100644
> --- a/llvm/lib/Transforms/Scalar/EarlyCSE.cpp
> +++ b/llvm/lib/Transforms/Scalar/EarlyCSE.cpp
> @@ -280,13 +280,6 @@ static unsigned getHashValueImpl(SimpleValue Val) {
> return hash_combine(II->getOpcode(), LHS, RHS);
> }
>
> - // gc.relocate is 'special' call: its second and third operands are
> - // not real values, but indices into statepoint's argument list.
> - // Get values they point to.
> - if (const GCRelocateInst *GCR = dyn_cast<GCRelocateInst>(Inst))
> - return hash_combine(GCR->getOpcode(), GCR->getOperand(0),
> - GCR->getBasePtr(), GCR->getDerivedPtr());
> -
> // Mix in the opcode.
> return hash_combine(
> Inst->getOpcode(),
> @@ -348,13 +341,6 @@ static bool isEqualImpl(SimpleValue LHS, SimpleValue RHS) {
> LII->getArgOperand(1) == RII->getArgOperand(0);
> }
>
> - // See comment above in `getHashValue()`.
> - if (const GCRelocateInst *GCR1 = dyn_cast<GCRelocateInst>(LHSI))
> - if (const GCRelocateInst *GCR2 = dyn_cast<GCRelocateInst>(RHSI))
> - return GCR1->getOperand(0) == GCR2->getOperand(0) &&
> - GCR1->getBasePtr() == GCR2->getBasePtr() &&
> - GCR1->getDerivedPtr() == GCR2->getDerivedPtr();
> -
> // Min/max can occur with commuted operands, non-canonical predicates,
> // and/or non-canonical operands.
> // Selects can be non-trivially equivalent via inverted conditions and swaps.
> @@ -468,6 +454,13 @@ template <> struct DenseMapInfo<CallValue> {
> unsigned DenseMapInfo<CallValue>::getHashValue(CallValue Val) {
> Instruction *Inst = Val.Inst;
>
> + // gc.relocate is 'special' call: its second and third operands are
> + // not real values, but indices into statepoint's argument list.
> + // Get values they point to.
> + if (const GCRelocateInst *GCR = dyn_cast<GCRelocateInst>(Inst))
> + return hash_combine(GCR->getOpcode(), GCR->getOperand(0),
> + GCR->getBasePtr(), GCR->getDerivedPtr());
> +
> // Hash all of the operands as pointers and mix in the opcode.
> return hash_combine(
> Inst->getOpcode(),
> @@ -479,6 +472,13 @@ bool DenseMapInfo<CallValue>::isEqual(CallValue LHS, CallValue RHS) {
> if (LHS.isSentinel() || RHS.isSentinel())
> return LHSI == RHSI;
>
> + // See comment above in `getHashValue()`.
> + if (const GCRelocateInst *GCR1 = dyn_cast<GCRelocateInst>(LHSI))
> + if (const GCRelocateInst *GCR2 = dyn_cast<GCRelocateInst>(RHSI))
> + return GCR1->getOperand(0) == GCR2->getOperand(0) &&
> + GCR1->getBasePtr() == GCR2->getBasePtr() &&
> + GCR1->getDerivedPtr() == GCR2->getDerivedPtr();
> +
> return LHSI->isIdenticalTo(RHSI);
> }
>
>
> diff --git a/llvm/test/Transforms/EarlyCSE/gc_relocate.ll b/llvm/test/Transforms/EarlyCSE/gc_relocate.ll
> index ae9001c9db66..df32d3f85b1e 100644
> --- a/llvm/test/Transforms/EarlyCSE/gc_relocate.ll
> +++ b/llvm/test/Transforms/EarlyCSE/gc_relocate.ll
> @@ -30,8 +30,11 @@ define i1 @test_readnone(i32 addrspace(1)* %in) gc "statepoint-example" {
> ; CHECK-NEXT: [[SAFEPOINT_TOKEN:%.*]] = call token (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* @func, i32 0, i32 0, i32 0, i32 0) [ "gc-live"(i32 addrspace(1)* [[IN:%.*]]) ]
> ; CHECK-NEXT: [[A:%.*]] = call i32 addrspace(1)* @llvm.experimental.gc.relocate.p1i32(token [[SAFEPOINT_TOKEN]], i32 0, i32 0)
> ; CHECK-NEXT: store i32 0, i32* @G, align 4
> +; CHECK-NEXT: [[B:%.*]] = call i32 addrspace(1)* @llvm.experimental.gc.relocate.p1i32(token [[SAFEPOINT_TOKEN]], i32 0, i32 0)
> ; CHECK-NEXT: [[CMP1:%.*]] = icmp eq i32 addrspace(1)* [[A]], null
> -; CHECK-NEXT: ret i1 [[CMP1]]
> +; CHECK-NEXT: [[CMP2:%.*]] = icmp eq i32 addrspace(1)* [[B]], null
> +; CHECK-NEXT: [[CMP:%.*]] = and i1 [[CMP1]], [[CMP2]]
> +; CHECK-NEXT: ret i1 [[CMP]]
> ;
> entry:
> %safepoint_token = call token (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* @func, i32 0, i32 0, i32 0, i32 0) ["gc-live"(i32 addrspace(1)* %in)]
>
> diff --git a/llvm/test/Transforms/GVN/gc_relocate.ll b/llvm/test/Transforms/GVN/gc_relocate.ll
> index bd279bdf42bb..53b9e5f300fc 100644
> --- a/llvm/test/Transforms/GVN/gc_relocate.ll
> +++ b/llvm/test/Transforms/GVN/gc_relocate.ll
> @@ -30,8 +30,11 @@ define i1 @test_readnone(i32 addrspace(1)* %in) gc "statepoint-example" {
> ; CHECK-NEXT: [[SAFEPOINT_TOKEN:%.*]] = call token (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* @func, i32 0, i32 0, i32 0, i32 0) [ "gc-live"(i32 addrspace(1)* [[IN:%.*]]) ]
> ; CHECK-NEXT: [[A:%.*]] = call i32 addrspace(1)* @llvm.experimental.gc.relocate.p1i32(token [[SAFEPOINT_TOKEN]], i32 0, i32 0)
> ; CHECK-NEXT: store i32 0, i32* @G, align 4
> +; CHECK-NEXT: [[B:%.*]] = call i32 addrspace(1)* @llvm.experimental.gc.relocate.p1i32(token [[SAFEPOINT_TOKEN]], i32 0, i32 0)
> ; CHECK-NEXT: [[CMP1:%.*]] = icmp eq i32 addrspace(1)* [[A]], null
> -; CHECK-NEXT: ret i1 [[CMP1]]
> +; CHECK-NEXT: [[CMP2:%.*]] = icmp eq i32 addrspace(1)* [[B]], null
> +; CHECK-NEXT: [[CMP:%.*]] = and i1 [[CMP1]], [[CMP2]]
> +; CHECK-NEXT: ret i1 [[CMP]]
> ;
> entry:
> %safepoint_token = call token (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* @func, i32 0, i32 0, i32 0, i32 0) ["gc-live"(i32 addrspace(1)* %in)]
> @@ -49,10 +52,14 @@ define i1 @test_call(i32 addrspace(1)* %in) gc "statepoint-example" {
> ; CHECK-NEXT: entry:
> ; CHECK-NEXT: [[SAFEPOINT_TOKEN:%.*]] = call token (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* @func, i32 0, i32 0, i32 0, i32 0) [ "gc-live"(i32 addrspace(1)* [[IN:%.*]], i32 addrspace(1)* [[IN]]) ]
> ; CHECK-NEXT: [[BASE:%.*]] = call i32 addrspace(1)* @llvm.experimental.gc.relocate.p1i32(token [[SAFEPOINT_TOKEN]], i32 0, i32 0)
> -; CHECK-NEXT: [[SAFEPOINT_TOKEN2:%.*]] = call token (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* @func, i32 0, i32 0, i32 0, i32 0) [ "gc-live"(i32 addrspace(1)* [[BASE]], i32 addrspace(1)* [[BASE]]) ]
> +; CHECK-NEXT: [[DERIVED:%.*]] = call i32 addrspace(1)* @llvm.experimental.gc.relocate.p1i32(token [[SAFEPOINT_TOKEN]], i32 0, i32 1)
> +; CHECK-NEXT: [[SAFEPOINT_TOKEN2:%.*]] = call token (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* @func, i32 0, i32 0, i32 0, i32 0) [ "gc-live"(i32 addrspace(1)* [[BASE]], i32 addrspace(1)* [[DERIVED]]) ]
> ; CHECK-NEXT: [[BASE_RELOC:%.*]] = call i32 addrspace(1)* @llvm.experimental.gc.relocate.p1i32(token [[SAFEPOINT_TOKEN2]], i32 0, i32 0)
> +; CHECK-NEXT: [[DERIVED_RELOC:%.*]] = call i32 addrspace(1)* @llvm.experimental.gc.relocate.p1i32(token [[SAFEPOINT_TOKEN2]], i32 0, i32 1)
> ; CHECK-NEXT: [[CMP1:%.*]] = icmp eq i32 addrspace(1)* [[BASE_RELOC]], null
> -; CHECK-NEXT: ret i1 [[CMP1]]
> +; CHECK-NEXT: [[CMP2:%.*]] = icmp eq i32 addrspace(1)* [[DERIVED_RELOC]], null
> +; CHECK-NEXT: [[CMP:%.*]] = and i1 [[CMP1]], [[CMP2]]
> +; CHECK-NEXT: ret i1 [[CMP]]
> ;
> entry:
> %safepoint_token = call token (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 0, i32 0, void ()* @func, i32 0, i32 0, i32 0, i32 0) ["gc-live"(i32 addrspace(1)* %in, i32 addrspace(1)* %in)]
>
> diff --git a/llvm/test/Transforms/LICM/gc-relocate.ll b/llvm/test/Transforms/LICM/gc-relocate.ll
> new file mode 100644
> index 000000000000..ac5c8df47bee
> --- /dev/null
> +++ b/llvm/test/Transforms/LICM/gc-relocate.ll
> @@ -0,0 +1,39 @@
> +; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
> +; RUN: opt -S -basic-aa -licm < %s | FileCheck %s
> +
> +target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128-ni:1-p2:32:8:8:32-ni:2"
> +target triple = "x86_64-unknown-linux-gnu"
> +
> +define i8 addrspace(1)* @test(i8 addrspace(1)* %arg) #0 gc "statepoint-example" personality i32* ()* @wobble {
> +; CHECK-LABEL: @test(
> +; CHECK-NEXT: bb:
> +; CHECK-NEXT: br label [[BB1:%.*]]
> +; CHECK: bb1:
> +; CHECK-NEXT: [[TMP:%.*]] = call token (i64, i32, i32 (i32, i8 addrspace(1)*, i32, i32, i32)*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_i32i32p1i8i32i32i32f(i64 1, i32 16, i32 (i32, i8 addrspace(1)*, i32, i32, i32)* nonnull @zot, i32 5, i32 0, i32 undef, i8 addrspace(1)* undef, i32 undef, i32 undef, i32 undef, i32 0, i32 0) [ "deopt"(i32 0, i32 0, i32 0, i32 235, i32 3, i32 32, i32 0, i32 0, i8 addrspace(1)* undef, i32 3, i32 undef, i32 3, float undef, i32 0, i8 addrspace(1)* undef, i32 7, i8* null, i32 3, i32 undef, i32 3, i32 undef, i32 3, i32 undef, i32 3, i32 undef, i32 3, i32 undef, i32 3, i32 undef, i32 3, i32 undef, i32 3, i32 undef, i32 3, i32 undef, i32 3, i32 undef, i32 3, i32 undef, i32 3, i32 undef, i32 3, i32 undef, i32 0, i8 addrspace(1)* undef, i32 4, double undef, i32 7, i8* null, i32 0, i8 addrspace(1)* undef, i32 3, float undef, i32 0, i8 addrspace(1)* undef, i32 0, i8 addrspace(1)* undef, i32 0, i8 addrspace(1)* undef, i32 0, i8 addrspace(1)* undef, i32 0, i8 addrspace(1)* undef, i32 0, i8 addrspace(1)* null, i32 3, i32 -15108, i32 7, i8* null, i32 7, i8* null, i32 7, i8* null, i32 7, i8* null, i32 7, i8* null), "gc-live"(i8 addrspace(1)* [[ARG:%.*]]) ]
> +; CHECK-NEXT: [[RES:%.*]] = call coldcc i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token [[TMP]], i32 0, i32 0)
> +; CHECK-NEXT: br i1 false, label [[BB1]], label [[BB2:%.*]]
> +; CHECK: bb2:
> +; CHECK-NEXT: [[RES_LCSSA:%.*]] = phi i8 addrspace(1)* [ [[RES]], [[BB1]] ]
> +; CHECK-NEXT: ret i8 addrspace(1)* [[RES_LCSSA]]
> +;
> +bb:
> + br label %bb1
> +
> +bb1:
> + %tmp = call token (i64, i32, i32 (i32, i8 addrspace(1)*, i32, i32, i32)*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_i32i32p1i8i32i32i32f(i64 1, i32 16, i32 (i32, i8 addrspace(1)*, i32, i32, i32)* nonnull @zot, i32 5, i32 0, i32 undef, i8 addrspace(1)* undef, i32 undef, i32 undef, i32 undef, i32 0, i32 0) [ "deopt"(i32 0, i32 0, i32 0, i32 235, i32 3, i32 32, i32 0, i32 0, i8 addrspace(1)* undef, i32 3, i32 undef, i32 3, float undef, i32 0, i8 addrspace(1)* undef, i32 7, i8* null, i32 3, i32 undef, i32 3, i32 undef, i32 3, i32 undef, i32 3, i32 undef, i32 3, i32 undef, i32 3, i32 undef, i32 3, i32 undef, i32 3, i32 undef, i32 3, i32 undef, i32 3, i32 undef, i32 3, i32 undef, i32 3, i32 undef, i32 3, i32 undef, i32 0, i8 addrspace(1)* undef, i32 4, double undef, i32 7, i8* null, i32 0, i8 addrspace(1)* undef, i32 3, float undef, i32 0, i8 addrspace(1)* undef, i32 0, i8 addrspace(1)* undef, i32 0, i8 addrspace(1)* undef, i32 0, i8 addrspace(1)* undef, i32 0, i8 addrspace(1)* undef, i32 0, i8 addrspace(1)* null, i32 3, i32 -15108, i32 7, i8* null, i32 7, i8* null, i32 7, i8* null, i32 7, i8* null, i32 7, i8* null), "gc-live"(i8 addrspace(1)* %arg) ]
> + %res = call coldcc i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token %tmp, i32 0, i32 0)
> + br i1 undef, label %bb1, label %bb2
> +
> +bb2:
> + ret i8 addrspace(1)* %res
> +}
> +
> +declare i32* @wobble()
> +declare i32 @zot(i32, i8 addrspace(1)*, i32, i32, i32)
> +declare token @llvm.experimental.gc.statepoint.p0f_isVoidi32f(i64, i32, void (i32)*, i32, i32, ...)
> +
> +; Function Attrs: nounwind readnone
> +declare i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token, i32, i32) #0
> +declare token @llvm.experimental.gc.statepoint.p0f_i32i32p1i8i32i32i32f(i64, i32, i32 (i32, i8 addrspace(1)*, i32, i32, i32)*, i32, i32, ...)
> +
> +attributes #0 = { nounwind readnone }
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list