[llvm] 88024a7 - [RS4GC] Use one DVCache for both inlineGetBaseAndOffset() and insertParsePoints()

Yevgeny Rouban via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 12 04:13:40 PDT 2021


Author: Yevgeny Rouban
Date: 2021-07-12T18:13:00+07:00
New Revision: 88024a724c3bac4aa902dc85d16d3285caac4377

URL: https://github.com/llvm/llvm-project/commit/88024a724c3bac4aa902dc85d16d3285caac4377
DIFF: https://github.com/llvm/llvm-project/commit/88024a724c3bac4aa902dc85d16d3285caac4377.diff

LOG: [RS4GC] Use one DVCache for both inlineGetBaseAndOffset() and insertParsePoints()

This new test demonstrates a case where a base ptr is generated
twice for the same value: the first one is generated while
the gc.get.pointer.base() is inlined, the second is generated
for the statepoint. This happens because the methods
inlineGetBaseAndOffset() and insertParsePoints() do not share
their defining value cache used by the findBasePointer() method.

Reviewed By: reames
Differential Revision: https://reviews.llvm.org/D103240

Added: 
    

Modified: 
    llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
    llvm/test/Transforms/RewriteStatepointsForGC/intrinsics-bare.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp b/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
index 53cd90b407e6f..bc0fecc972fc1 100644
--- a/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
+++ b/llvm/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
@@ -2400,8 +2400,8 @@ static void rematerializeLiveValues(CallBase *Call,
 }
 
 static bool inlineGetBaseAndOffset(Function &F,
-                                   SmallVectorImpl<CallInst *> &Intrinsics) {
-  DefiningValueMapTy DVCache;
+                                   SmallVectorImpl<CallInst *> &Intrinsics,
+                                   DefiningValueMapTy &DVCache) {
   auto &Context = F.getContext();
   auto &DL = F.getParent()->getDataLayout();
   bool Changed = false;
@@ -2451,7 +2451,8 @@ static bool inlineGetBaseAndOffset(Function &F,
 
 static bool insertParsePoints(Function &F, DominatorTree &DT,
                               TargetTransformInfo &TTI,
-                              SmallVectorImpl<CallBase *> &ToUpdate) {
+                              SmallVectorImpl<CallBase *> &ToUpdate,
+                              DefiningValueMapTy &DVCache) {
 #ifndef NDEBUG
   // sanity check the input
   std::set<CallBase *> Uniqued;
@@ -2502,16 +2503,10 @@ static bool insertParsePoints(Function &F, DominatorTree &DT,
   findLiveReferences(F, DT, ToUpdate, Records);
 
   // B) Find the base pointers for each live pointer
-  /* scope for caching */ {
-    // Cache the 'defining value' relation used in the computation and
-    // insertion of base phis and selects.  This ensures that we don't insert
-    // large numbers of duplicate base_phis.
-    DefiningValueMapTy DVCache;
-    for (size_t i = 0; i < Records.size(); i++) {
-      PartiallyConstructedSafepointRecord &info = Records[i];
-      findBasePointers(DT, DVCache, ToUpdate[i], info);
-    }
-  } // end of cache scope
+  for (size_t i = 0; i < Records.size(); i++) {
+    PartiallyConstructedSafepointRecord &info = Records[i];
+    findBasePointers(DT, DVCache, ToUpdate[i], info);
+  }
 
   // The base phi insertion logic (for any safepoint) may have inserted new
   // instructions which are now live at some safepoint.  The simplest such
@@ -2937,13 +2932,20 @@ bool RewriteStatepointsForGC::runOnFunction(Function &F, DominatorTree &DT,
     }
   }
 
+  // Cache the 'defining value' relation used in the computation and
+  // insertion of base phis and selects.  This ensures that we don't insert
+  // large numbers of duplicate base_phis. Use one cache for both
+  // inlineGetBaseAndOffset() and insertParsePoints().
+  DefiningValueMapTy DVCache;
+
   if (!Intrinsics.empty())
     // Inline @gc.get.pointer.base() and @gc.get.pointer.offset() before finding
     // live references.
-    MadeChange |= inlineGetBaseAndOffset(F, Intrinsics);
+    MadeChange |= inlineGetBaseAndOffset(F, Intrinsics, DVCache);
 
   if (!ParsePointNeeded.empty())
-    MadeChange |= insertParsePoints(F, DT, TTI, ParsePointNeeded);
+    MadeChange |= insertParsePoints(F, DT, TTI, ParsePointNeeded, DVCache);
+
   return MadeChange;
 }
 

diff  --git a/llvm/test/Transforms/RewriteStatepointsForGC/intrinsics-bare.ll b/llvm/test/Transforms/RewriteStatepointsForGC/intrinsics-bare.ll
index cded22163d792..0760fd5dd3e79 100644
--- a/llvm/test/Transforms/RewriteStatepointsForGC/intrinsics-bare.ll
+++ b/llvm/test/Transforms/RewriteStatepointsForGC/intrinsics-bare.ll
@@ -10,11 +10,10 @@ define i8 addrspace(1)* @test_duplicate_base_generation(i8 addrspace(1)* %obj1,
 ; CHECK-NEXT:    [[OBJ1_12:%.*]] = getelementptr inbounds i8, i8 addrspace(1)* [[OBJ1:%.*]], i64 12
 ; CHECK-NEXT:    [[OBJ2_16:%.*]] = getelementptr inbounds i8, i8 addrspace(1)* [[OBJ2:%.*]], i64 16
 ; CHECK-NEXT:    [[SELECTED_BASE:%.*]] = select i1 [[C:%.*]], i8 addrspace(1)* [[OBJ2]], i8 addrspace(1)* [[OBJ1]], !is_base_value !0
-; CHECK-NEXT:    [[SELECTED_BASE1:%.*]] = select i1 [[C]], i8 addrspace(1)* [[OBJ2]], i8 addrspace(1)* [[OBJ1]], !is_base_value !0
 ; CHECK-NEXT:    [[SELECTED:%.*]] = select i1 [[C]], i8 addrspace(1)* [[OBJ2_16]], i8 addrspace(1)* [[OBJ1_12]]
-; CHECK-NEXT:    [[STATEPOINT_TOKEN:%.*]] = call token (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 2882400000, i32 0, void ()* @foo, i32 0, i32 0, i32 0, i32 0) [ "gc-live"(i8 addrspace(1)* [[SELECTED]], i8 addrspace(1)* [[SELECTED_BASE1]]) ]
+; CHECK-NEXT:    [[STATEPOINT_TOKEN:%.*]] = call token (i64, i32, void ()*, i32, i32, ...) @llvm.experimental.gc.statepoint.p0f_isVoidf(i64 2882400000, i32 0, void ()* @foo, i32 0, i32 0, i32 0, i32 0) [ "gc-live"(i8 addrspace(1)* [[SELECTED]], i8 addrspace(1)* [[SELECTED_BASE]]) ]
 ; CHECK-NEXT:    [[SELECTED_RELOCATED:%.*]] = call coldcc i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token [[STATEPOINT_TOKEN]], i32 1, i32 0)
-; CHECK-NEXT:    [[SELECTED_BASE1_RELOCATED:%.*]] = call coldcc i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token [[STATEPOINT_TOKEN]], i32 1, i32 1)
+; CHECK-NEXT:    [[SELECTED_BASE_RELOCATED:%.*]] = call coldcc i8 addrspace(1)* @llvm.experimental.gc.relocate.p1i8(token [[STATEPOINT_TOKEN]], i32 1, i32 1)
 ; CHECK-NEXT:    ret i8 addrspace(1)* [[SELECTED_RELOCATED]]
 ;
 entry:


        


More information about the llvm-commits mailing list