[llvm] cfe8f8e - Revert "Mark gc.relocate and gc.result as readnone"
Serguei Katkov via llvm-commits
llvm-commits at lists.llvm.org
Sun Mar 14 19:19:04 PDT 2021
Hi Philip,
Thank you for your support. I added LICM test llvm/src/llvm/test/Transforms/LICM/gc-relocate.ll together with my revert.
It is simplified version of the test I met.
Thank you,
Serguei.
> -----Original Message-----
> From: Philip Reames <listmail at philipreames.com>
> Sent: Saturday, March 13, 2021 3:52 AM
> To: Serguei Katkov <serguei.katkov at azul.com>; Serguei Katkov
> <llvmlistbot at llvm.org>; llvm-commits at lists.llvm.org
> Subject: Re: [llvm] cfe8f8e - Revert "Mark gc.relocate and gc.result as
> readnone"
>
> 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/cfe8f8e0f010077f5942bce88a
> > 2fd331b90ccea7
> > DIFF:
> > https://github.com/llvm/llvm-project/commit/cfe8f8e0f010077f5942bce88a
> > 2fd331b90ccea7.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:
> > + at 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, ...)
> > + at 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)*
> > + at 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
> > + at 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
> > + at 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