Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename WeakVH to WeakTrackingVH; NFC #6663

Merged
merged 2 commits into from
Jun 18, 2024

Conversation

lizhengxing
Copy link
Collaborator

This PR pulls the upstream change, Rename WeakVH to WeakTrackingVH; NFC (llvm/llvm-project@e6bca0e), into DXC.

Here's the summary of the change:

I plan to use WeakVH to mean "nulls itself out on deletion, but does not track RAUW" in a subsequent commit.

Reviewers: dblaikie, davide

Reviewed By: davide

Subscribers: arsenm, mehdi_amini, mcrosier, mzolotukhin, jfb, llvm-commits, nhaehnle

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

This is part 3 of the fix for #6659.

@lizhengxing lizhengxing requested a review from a team as a code owner May 31, 2024 17:09
Copy link
Contributor

github-actions bot commented May 31, 2024

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff a44c88e2b8032463001c2f379e535a5e16f5807b a776a7714facc73dfdf8f8eed15e08dc9e3f2ad7 -- include/llvm/Analysis/AssumptionCache.h include/llvm/Analysis/CallGraph.h include/llvm/Analysis/DxilValueCache.h include/llvm/Analysis/IVUsers.h include/llvm/Analysis/MemoryBuiltins.h include/llvm/Analysis/ScalarEvolutionExpander.h include/llvm/IR/ValueHandle.h include/llvm/Transforms/Utils/Cloning.h include/llvm/Transforms/Utils/SimplifyIndVar.h include/llvm/Transforms/Utils/ValueMapper.h lib/Analysis/IPA/CallGraphSCCPass.cpp lib/Analysis/ScalarEvolutionExpander.cpp lib/Bitcode/Reader/BitcodeReader.cpp lib/CodeGen/CodeGenPrepare.cpp lib/HLSL/DxilPreparePasses.cpp lib/IR/Value.cpp lib/Transforms/IPO/GlobalOpt.cpp lib/Transforms/IPO/MergeFunctions.cpp lib/Transforms/InstCombine/InstructionCombining.cpp lib/Transforms/Scalar/DeadStoreElimination.cpp lib/Transforms/Scalar/IndVarSimplify.cpp lib/Transforms/Scalar/LoopIdiomRecognize.cpp lib/Transforms/Scalar/LoopStrengthReduce.cpp lib/Transforms/Scalar/LoopUnswitch.cpp lib/Transforms/Scalar/Reassociate.cpp lib/Transforms/Utils/BasicBlockUtils.cpp lib/Transforms/Utils/CloneFunction.cpp lib/Transforms/Utils/InlineFunction.cpp lib/Transforms/Utils/Local.cpp lib/Transforms/Utils/LoopUnroll.cpp lib/Transforms/Utils/SimplifyIndVar.cpp lib/Transforms/Vectorize/SLPVectorizer.cpp tools/clang/lib/CodeGen/CGDeclCXX.cpp tools/clang/lib/CodeGen/CodeGenFunction.h tools/clang/lib/CodeGen/CodeGenModule.cpp tools/clang/lib/CodeGen/CodeGenModule.h unittests/IR/ValueHandleTest.cpp
View the diff from clang-format here.
diff --git a/lib/Analysis/IPA/CallGraphSCCPass.cpp b/lib/Analysis/IPA/CallGraphSCCPass.cpp
index 610f01a2..a9fd9930 100644
--- a/lib/Analysis/IPA/CallGraphSCCPass.cpp
+++ b/lib/Analysis/IPA/CallGraphSCCPass.cpp
@@ -219,7 +219,7 @@ bool CGPassManager::RefreshCallGraph(CallGraphSCC &CurSCC,
     // Get the set of call sites currently in the function.
     for (CallGraphNode::iterator I = CGN->begin(), E = CGN->end(); I != E; ) {
       // If this call site is null, then the function pass deleted the call
-      // entirely and the WeakTrackingVH nulled it out.  
+      // entirely and the WeakTrackingVH nulled it out.
       if (!I->first ||
           // If we've already seen this call site, then the FunctionPass RAUW'd
           // one call with another, which resulted in two "uses" in the edge
@@ -360,7 +360,7 @@ bool CGPassManager::RefreshCallGraph(CallGraphSCC &CurSCC,
     if (NumIndirectRemoved > NumIndirectAdded &&
         NumDirectRemoved < NumDirectAdded)
       DevirtualizedCall = true;
-    
+
     // After scanning this function, if we still have entries in callsites, then
     // they are dangling pointers.  WeakTrackingVH should save us for this, so
     // abort if
diff --git a/lib/CodeGen/CodeGenPrepare.cpp b/lib/CodeGen/CodeGenPrepare.cpp
index 9b358de6..0a9fbf33 100644
--- a/lib/CodeGen/CodeGenPrepare.cpp
+++ b/lib/CodeGen/CodeGenPrepare.cpp
@@ -1370,8 +1370,8 @@ bool CodeGenPrepare::OptimizeCallInst(CallInst *CI, bool& ModifiedDT) {
       Constant *RetVal = ConstantInt::get(ReturnTy, Min ? 0 : -1ULL);
 
       // Substituting this can cause recursive simplifications, which can
-      // invalidate our iterator.  Use WeakTrackingVH to hold onto it in case this
-      // happens.
+      // invalidate our iterator.  Use WeakTrackingVH to hold onto it in case
+      // this happens.
       WeakTrackingVH IterHandle(CurInstIterator);
 
       replaceAndRecursivelySimplify(CI, RetVal,
diff --git a/lib/Transforms/IPO/MergeFunctions.cpp b/lib/Transforms/IPO/MergeFunctions.cpp
index 355165c1..4bdef889 100644
--- a/lib/Transforms/IPO/MergeFunctions.cpp
+++ b/lib/Transforms/IPO/MergeFunctions.cpp
@@ -1248,7 +1248,8 @@ bool MergeFunctions::runOnModule(Module &M) {
     // Insert only strong functions and merge them. Strong function merging
     // always deletes one of them.
     for (std::vector<WeakTrackingVH>::iterator I = Worklist.begin(),
-           E = Worklist.end(); I != E; ++I) {
+                                               E = Worklist.end();
+         I != E; ++I) {
       if (!*I) continue;
       Function *F = cast<Function>(*I);
       if (!F->isDeclaration() && !F->hasAvailableExternallyLinkage() &&
@@ -1262,7 +1263,8 @@ bool MergeFunctions::runOnModule(Module &M) {
     // functions are identical, we create a new strong function with two weak
     // weak thunks to it which are identical but not mergable.
     for (std::vector<WeakTrackingVH>::iterator I = Worklist.begin(),
-           E = Worklist.end(); I != E; ++I) {
+                                               E = Worklist.end();
+         I != E; ++I) {
       if (!*I) continue;
       Function *F = cast<Function>(*I);
       if (!F->isDeclaration() && !F->hasAvailableExternallyLinkage() &&
diff --git a/lib/Transforms/InstCombine/InstructionCombining.cpp b/lib/Transforms/InstCombine/InstructionCombining.cpp
index d4e6ed38..6bc322fa 100644
--- a/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -1843,7 +1843,7 @@ Instruction *InstCombiner::visitGetElementPtrInst(GetElementPtrInst &GEP) {
 
 static bool isAllocSiteRemovable(Instruction *AI,
                                  SmallVectorImpl<WeakTrackingVH> &Users,
-                     const TargetLibraryInfo *TLI) {
+                                 const TargetLibraryInfo *TLI) {
   SmallVector<Instruction*, 4> Worklist;
   Worklist.push_back(AI);
 
diff --git a/lib/Transforms/Scalar/IndVarSimplify.cpp b/lib/Transforms/Scalar/IndVarSimplify.cpp
index 31961b4d..d120d465 100644
--- a/lib/Transforms/Scalar/IndVarSimplify.cpp
+++ b/lib/Transforms/Scalar/IndVarSimplify.cpp
@@ -903,20 +903,13 @@ class WidenIV {
   SmallVector<NarrowIVDefUse, 8> NarrowIVUsers;
 
 public:
-  WidenIV(const WideIVInfo &WI, LoopInfo *LInfo,
-          ScalarEvolution *SEv, DominatorTree *DTree, SmallVectorImpl<WeakTrackingVH> &DI)
-      :
-    OrigPhi(WI.NarrowIV),
-    WideType(WI.WidestNativeType),
-    IsSigned(WI.IsSigned),
-    LI(LInfo),
-    L(LI->getLoopFor(OrigPhi->getParent())),
-    SE(SEv),
-    DT(DTree),
-    WidePhi(nullptr),
-    WideInc(nullptr),
-    WideIncExpr(nullptr),
-    DeadInsts(DI) {
+  WidenIV(const WideIVInfo &WI, LoopInfo *LInfo, ScalarEvolution *SEv,
+          DominatorTree *DTree, SmallVectorImpl<WeakTrackingVH> &DI)
+      : OrigPhi(WI.NarrowIV), WideType(WI.WidestNativeType),
+        IsSigned(WI.IsSigned), LI(LInfo),
+        L(LI->getLoopFor(OrigPhi->getParent())), SE(SEv), DT(DTree),
+        WidePhi(nullptr), WideInc(nullptr), WideIncExpr(nullptr),
+        DeadInsts(DI) {
     assert(L->getHeader() == OrigPhi->getParent() && "Phi must be an IV");
   }
 
diff --git a/lib/Transforms/Scalar/LoopStrengthReduce.cpp b/lib/Transforms/Scalar/LoopStrengthReduce.cpp
index e5c60a43..3ab9367a 100644
--- a/lib/Transforms/Scalar/LoopStrengthReduce.cpp
+++ b/lib/Transforms/Scalar/LoopStrengthReduce.cpp
@@ -1766,21 +1766,14 @@ class LSRInstance {
                                   const LSRUse &LU,
                                   SCEVExpander &Rewriter) const;
 
-  Value *Expand(const LSRFixup &LF,
-                const Formula &F,
-                BasicBlock::iterator IP,
+  Value *Expand(const LSRFixup &LF, const Formula &F, BasicBlock::iterator IP,
                 SCEVExpander &Rewriter,
                 SmallVectorImpl<WeakTrackingVH> &DeadInsts) const;
-  void RewriteForPHI(PHINode *PN, const LSRFixup &LF,
-                     const Formula &F,
+  void RewriteForPHI(PHINode *PN, const LSRFixup &LF, const Formula &F,
                      SCEVExpander &Rewriter,
-                     SmallVectorImpl<WeakTrackingVH> &DeadInsts,
-                     Pass *P) const;
-  void Rewrite(const LSRFixup &LF,
-               const Formula &F,
-               SCEVExpander &Rewriter,
-               SmallVectorImpl<WeakTrackingVH> &DeadInsts,
-               Pass *P) const;
+                     SmallVectorImpl<WeakTrackingVH> &DeadInsts, Pass *P) const;
+  void Rewrite(const LSRFixup &LF, const Formula &F, SCEVExpander &Rewriter,
+               SmallVectorImpl<WeakTrackingVH> &DeadInsts, Pass *P) const;
   void ImplementSolution(const SmallVectorImpl<const Formula *> &Solution,
                          Pass *P);
 
@@ -4449,10 +4442,8 @@ LSRInstance::AdjustInsertPositionForExpand(BasicBlock::iterator LowestIP,
 
 /// Expand - Emit instructions for the leading candidate expression for this
 /// LSRUse (this is called "expanding").
-Value *LSRInstance::Expand(const LSRFixup &LF,
-                           const Formula &F,
-                           BasicBlock::iterator IP,
-                           SCEVExpander &Rewriter,
+Value *LSRInstance::Expand(const LSRFixup &LF, const Formula &F,
+                           BasicBlock::iterator IP, SCEVExpander &Rewriter,
                            SmallVectorImpl<WeakTrackingVH> &DeadInsts) const {
   const LSRUse &LU = Uses[LF.LUIdx];
   if (LU.RigidFormula)
@@ -4634,10 +4625,8 @@ Value *LSRInstance::Expand(const LSRFixup &LF,
 /// RewriteForPHI - Helper for Rewrite. PHI nodes are special because the use
 /// of their operands effectively happens in their predecessor blocks, so the
 /// expression may need to be expanded in multiple places.
-void LSRInstance::RewriteForPHI(PHINode *PN,
-                                const LSRFixup &LF,
-                                const Formula &F,
-                                SCEVExpander &Rewriter,
+void LSRInstance::RewriteForPHI(PHINode *PN, const LSRFixup &LF,
+                                const Formula &F, SCEVExpander &Rewriter,
                                 SmallVectorImpl<WeakTrackingVH> &DeadInsts,
                                 Pass *P) const {
   DenseMap<BasicBlock *, Value *> Inserted;
@@ -4710,8 +4699,7 @@ void LSRInstance::RewriteForPHI(PHINode *PN,
 /// Rewrite - Emit instructions for the leading candidate expression for this
 /// LSRUse (this is called "expanding"), and update the UserInst to reference
 /// the newly expanded value.
-void LSRInstance::Rewrite(const LSRFixup &LF,
-                          const Formula &F,
+void LSRInstance::Rewrite(const LSRFixup &LF, const Formula &F,
                           SCEVExpander &Rewriter,
                           SmallVectorImpl<WeakTrackingVH> &DeadInsts,
                           Pass *P) const {
diff --git a/lib/Transforms/Scalar/LoopUnswitch.cpp b/lib/Transforms/Scalar/LoopUnswitch.cpp
index 00a1594e..bf2aa49d 100644
--- a/lib/Transforms/Scalar/LoopUnswitch.cpp
+++ b/lib/Transforms/Scalar/LoopUnswitch.cpp
@@ -980,10 +980,10 @@ void LoopUnswitch::UnswitchNontrivialCondition(Value *LIC, Constant *Val,
   LoopProcessWorklist.push_back(NewLoop);
   redoLoop = true;
 
-  // Keep a WeakTrackingVH holding onto LIC.  If the first call to RewriteLoopBody
-  // deletes the instruction (for example by simplifying a PHI that feeds into
-  // the condition that we're unswitching on), we don't rewrite the second
-  // iteration.
+  // Keep a WeakTrackingVH holding onto LIC.  If the first call to
+  // RewriteLoopBody deletes the instruction (for example by simplifying a PHI
+  // that feeds into the condition that we're unswitching on), we don't rewrite
+  // the second iteration.
   WeakTrackingVH LICHandle(LIC);
 
   // Now we rewrite the original code to know that the condition is true and the
diff --git a/lib/Transforms/Utils/InlineFunction.cpp b/lib/Transforms/Utils/InlineFunction.cpp
index ff4b29dd..f6a255a0 100644
--- a/lib/Transforms/Utils/InlineFunction.cpp
+++ b/lib/Transforms/Utils/InlineFunction.cpp
@@ -217,7 +217,7 @@ static void HandleCallsInBlockInlinedThroughInvoke(BasicBlock *BB,
     II->setDebugLoc(CI->getDebugLoc());
     II->setCallingConv(CI->getCallingConv());
     II->setAttributes(CI->getAttributes());
-    
+
     // Make sure that anything using the call now uses the invoke!  This also
     // updates the CallGraph if present, because it uses a WeakTrackingVH.
     CI->replaceAllUsesWith(II);
diff --git a/lib/Transforms/Vectorize/SLPVectorizer.cpp b/lib/Transforms/Vectorize/SLPVectorizer.cpp
index a12404a2..b4b4c1c4 100644
--- a/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -3195,9 +3195,11 @@ private:
 /// \brief Check that the Values in the slice in VL array are still existent in
 /// the WeakTrackingVH array.
 /// Vectorization of part of the VL array may cause later values in the VL array
-/// to become invalid. We track when this has happened in the WeakTrackingVH array.
-static bool hasValueBeenRAUWed(ArrayRef<Value *> VL, ArrayRef<WeakTrackingVH> VH,
-                               unsigned SliceBegin, unsigned SliceSize) {
+/// to become invalid. We track when this has happened in the WeakTrackingVH
+/// array.
+static bool hasValueBeenRAUWed(ArrayRef<Value *> VL,
+                               ArrayRef<WeakTrackingVH> VH, unsigned SliceBegin,
+                               unsigned SliceSize) {
   VL = VL.slice(SliceBegin, SliceSize);
   VH = VH.slice(SliceBegin, SliceSize);
   return !std::equal(VL.begin(), VL.end(), VH.begin());
diff --git a/tools/clang/lib/CodeGen/CGDeclCXX.cpp b/tools/clang/lib/CodeGen/CGDeclCXX.cpp
index 699c0f70..1a4b81f6 100644
--- a/tools/clang/lib/CodeGen/CGDeclCXX.cpp
+++ b/tools/clang/lib/CodeGen/CGDeclCXX.cpp
@@ -556,9 +556,10 @@ CodeGenFunction::GenerateCXXGlobalInitFunc(llvm::Function *Fn,
   FinishFunction();
 }
 
-void CodeGenFunction::GenerateCXXGlobalDtorsFunc(llvm::Function *Fn,
+void CodeGenFunction::GenerateCXXGlobalDtorsFunc(
+    llvm::Function *Fn,
     const std::vector<std::pair<llvm::WeakTrackingVH, llvm::Constant *>>
-                                                &DtorsAndObjects) {
+        &DtorsAndObjects) {
   {
     auto NL = ApplyDebugLocation::CreateEmpty(*this);
     StartFunction(GlobalDecl(), getContext().VoidTy, Fn,
diff --git a/tools/clang/lib/CodeGen/CodeGenFunction.h b/tools/clang/lib/CodeGen/CodeGenFunction.h
index 7c7ff628..96137c66 100644
--- a/tools/clang/lib/CodeGen/CodeGenFunction.h
+++ b/tools/clang/lib/CodeGen/CodeGenFunction.h
@@ -2848,9 +2848,10 @@ public:
 
   /// GenerateCXXGlobalDtorsFunc - Generates code for destroying global
   /// variables.
-  void GenerateCXXGlobalDtorsFunc(llvm::Function *Fn,
-      const std::vector<std::pair<llvm::WeakTrackingVH,
-                                  llvm::Constant*> > &DtorsAndObjects);
+  void GenerateCXXGlobalDtorsFunc(
+      llvm::Function *Fn,
+      const std::vector<std::pair<llvm::WeakTrackingVH, llvm::Constant *>>
+          &DtorsAndObjects);
 
   void GenerateCXXGlobalVarDeclInitFunc(llvm::Function *Fn,
                                         const VarDecl *D,
diff --git a/unittests/IR/ValueHandleTest.cpp b/unittests/IR/ValueHandleTest.cpp
index b167b04f..77daab77 100644
--- a/unittests/IR/ValueHandleTest.cpp
+++ b/unittests/IR/ValueHandleTest.cpp
@@ -100,7 +100,6 @@ TEST_F(ValueHandle, WeakTrackingVH_NullOnDeletion) {
   EXPECT_EQ(null_value, WVH_Recreated);
 }
 
-
 TEST_F(ValueHandle, AssertingVH_BasicOperation) {
   AssertingVH<CastInst> AVH(BitcastV.get());
   CastInst *implicit_to_exact_type = AVH;
  • Check this box to apply formatting changes to this branch.

@lizhengxing lizhengxing force-pushed the user/zhengxing/Emulate-TrackingVH-upstream branch 4 times, most recently from a2573e8 to 37995a6 Compare June 6, 2024 08:13
@lizhengxing lizhengxing changed the base branch from user/zhengxing/Emulate-TrackingVH-upstream to main June 12, 2024 00:56
@lizhengxing lizhengxing force-pushed the user/zhengxing/Rename-WeakVH-upstream branch from e13c18b to 0f29828 Compare June 12, 2024 02:04
This PR pulls the upstream change, Rename WeakVH to WeakTrackingVH; NFC (llvm/llvm-project@e6bca0e), into DXC.

Here's the summary of the change:

>  I plan to use WeakVH to mean "nulls itself out on deletion, but does not track RAUW" in a subsequent commit.
>
>  Reviewers: dblaikie, davide
>
>  Reviewed By: davide
>
>  Subscribers: arsenm, mehdi_amini, mcrosier, mzolotukhin, jfb, llvm-commits, nhaehnle
>
>  Differential Revision: https://reviews.llvm.org/D32266

This is part 3 of the fix for #6659.
@lizhengxing lizhengxing force-pushed the user/zhengxing/Rename-WeakVH-upstream branch from cca3bbf to a776a77 Compare June 12, 2024 03:15
@adam-yang adam-yang self-assigned this Jun 18, 2024
unittests/IR/ValueHandleTest.cpp Show resolved Hide resolved
@adam-yang adam-yang merged commit 45018c7 into main Jun 18, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants