[llvm] 6972e39 - [gvn] CSE gc.relocates based on meaning, not spelling (try 2)

Philip Reames via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 16 11:00:13 PDT 2021


Author: Philip Reames
Date: 2021-03-16T10:59:31-07:00
New Revision: 6972e39d47eccc3e5fc3ded4a1e1b78f74d10af6

URL: https://github.com/llvm/llvm-project/commit/6972e39d47eccc3e5fc3ded4a1e1b78f74d10af6
DIFF: https://github.com/llvm/llvm-project/commit/6972e39d47eccc3e5fc3ded4a1e1b78f74d10af6.diff

LOG: [gvn] CSE gc.relocates based on meaning, not spelling (try 2)

This was (partially) reverted in cfe8f8e0 because the conversion from readonly to readnone in Intrinsics.td exposed a couple of problems.  This change has been reworked to not need that change (via some explicit checks in client code).  This is being done to address the original optimization issue and simplify the testing of the readonly changes.  I'm working on that piece under 49607.

Original commit message follows:

The last two operands to a gc.relocate represent indices into the associated gc.statepoint's gc bundle list. (Effectively, gc.relocates are projections from the gc.statepoints multiple return values.)

We can use this to recognize when two gc.relocates are equivalent (and can be CSEd), even when the indices are non-equal. This is particular useful when considering a chain of multiple statepoints as it lets us eliminate all duplicate gc.relocates in a single pass.

Differential Revision: https://reviews.llvm.org/D97974

Added: 
    

Modified: 
    llvm/lib/Transforms/Scalar/EarlyCSE.cpp
    llvm/lib/Transforms/Scalar/GVN.cpp
    llvm/test/Transforms/EarlyCSE/gc_relocate.ll
    llvm/test/Transforms/GVN/gc_relocate.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Scalar/EarlyCSE.cpp b/llvm/lib/Transforms/Scalar/EarlyCSE.cpp
index 180a82917fa9..9c7d43078821 100644
--- a/llvm/lib/Transforms/Scalar/EarlyCSE.cpp
+++ b/llvm/lib/Transforms/Scalar/EarlyCSE.cpp
@@ -109,6 +109,9 @@ struct SimpleValue {
 
   static bool canHandle(Instruction *Inst) {
     // This can only handle non-void readnone functions.
+    if (isa<GCRelocateInst>(Inst))
+      // Migration assistant for PR49607, to be removed once complete
+      return true;
     if (CallInst *CI = dyn_cast<CallInst>(Inst))
       return CI->doesNotAccessMemory() && !CI->getType()->isVoidTy();
     return isa<CastInst>(Inst) || isa<UnaryOperator>(Inst) ||
@@ -280,6 +283,13 @@ 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(),
@@ -341,6 +351,13 @@ 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.
@@ -454,13 +471,6 @@ 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(),
@@ -472,13 +482,6 @@ 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/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp
index b9171889005a..65f7d0498adf 100644
--- a/llvm/lib/Transforms/Scalar/GVN.cpp
+++ b/llvm/lib/Transforms/Scalar/GVN.cpp
@@ -397,7 +397,9 @@ void GVN::ValueTable::add(Value *V, uint32_t num) {
 }
 
 uint32_t GVN::ValueTable::lookupOrAddCall(CallInst *C) {
-  if (AA->doesNotAccessMemory(C)) {
+  // The gc.relocate specific check is to simplify migration under PR49607, and
+  // is to be removed once complete.
+  if (AA->doesNotAccessMemory(C) || isa<GCRelocateInst>(C)) {
     Expression exp = createExpr(C);
     uint32_t e = assignExpNewValueNum(exp).first;
     valueNumbering[C] = e;

diff  --git a/llvm/test/Transforms/EarlyCSE/gc_relocate.ll b/llvm/test/Transforms/EarlyCSE/gc_relocate.ll
index df32d3f85b1e..ae9001c9db66 100644
--- a/llvm/test/Transforms/EarlyCSE/gc_relocate.ll
+++ b/llvm/test/Transforms/EarlyCSE/gc_relocate.ll
@@ -30,11 +30,8 @@ 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:    [[CMP2:%.*]] = icmp eq i32 addrspace(1)* [[B]], null
-; CHECK-NEXT:    [[CMP:%.*]] = and i1 [[CMP1]], [[CMP2]]
-; CHECK-NEXT:    ret i1 [[CMP]]
+; CHECK-NEXT:    ret i1 [[CMP1]]
 ;
 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 53b9e5f300fc..bd279bdf42bb 100644
--- a/llvm/test/Transforms/GVN/gc_relocate.ll
+++ b/llvm/test/Transforms/GVN/gc_relocate.ll
@@ -30,11 +30,8 @@ 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:    [[CMP2:%.*]] = icmp eq i32 addrspace(1)* [[B]], null
-; CHECK-NEXT:    [[CMP:%.*]] = and i1 [[CMP1]], [[CMP2]]
-; CHECK-NEXT:    ret i1 [[CMP]]
+; CHECK-NEXT:    ret i1 [[CMP1]]
 ;
 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)]
@@ -52,14 +49,10 @@ 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:    [[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:    [[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:    [[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:    [[CMP2:%.*]] = icmp eq i32 addrspace(1)* [[DERIVED_RELOC]], null
-; CHECK-NEXT:    [[CMP:%.*]] = and i1 [[CMP1]], [[CMP2]]
-; CHECK-NEXT:    ret i1 [[CMP]]
+; CHECK-NEXT:    ret i1 [[CMP1]]
 ;
 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)]


        


More information about the llvm-commits mailing list