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

[NFC][Utils] Extract CloneFunctionBodyInto from CloneFunctionInto #118624

Merged
merged 1 commit into from
Dec 16, 2024

Conversation

@llvmbot
Copy link
Member

llvmbot commented Dec 4, 2024

@llvm/pr-subscribers-llvm-transforms

Author: Artem Pianykh (artempyanykh)

Changes

[NFC][Utils] Extract CloneFunctionBodyInto from CloneFunctionInto

Summary:
This and previously extracted CloneFunction*Into functions will be used in later diffs.

Test Plan:
ninja check-llvm-unit check-llvm


Full diff: https://github.com/llvm/llvm-project/pull/118624.diff

2 Files Affected:

  • (modified) llvm/include/llvm/Transforms/Utils/Cloning.h (+21-13)
  • (modified) llvm/lib/Transforms/Utils/CloneFunction.cpp (+55-41)
diff --git a/llvm/include/llvm/Transforms/Utils/Cloning.h b/llvm/include/llvm/Transforms/Utils/Cloning.h
index 698d773525e80b..47b75853ce9482 100644
--- a/llvm/include/llvm/Transforms/Utils/Cloning.h
+++ b/llvm/include/llvm/Transforms/Utils/Cloning.h
@@ -194,6 +194,15 @@ void CloneFunctionMetadataInto(Function *NewFunc, const Function *OldFunc,
                                ValueMapTypeRemapper *TypeMapper = nullptr,
                                ValueMaterializer *Materializer = nullptr);
 
+/// Clone OldFunc's body into NewFunc.
+void CloneFunctionBodyInto(Function *NewFunc, const Function *OldFunc,
+                           ValueToValueMapTy &VMap, RemapFlags RemapFlag,
+                           SmallVectorImpl<ReturnInst *> &Returns,
+                           const char *NameSuffix = "",
+                           ClonedCodeInfo *CodeInfo = nullptr,
+                           ValueMapTypeRemapper *TypeMapper = nullptr,
+                           ValueMaterializer *Materializer = nullptr);
+
 void CloneAndPruneIntoFromInst(Function *NewFunc, const Function *OldFunc,
                                const Instruction *StartingInst,
                                ValueToValueMapTy &VMap, bool ModuleLevelChanges,
@@ -214,7 +223,7 @@ void CloneAndPruneIntoFromInst(Function *NewFunc, const Function *OldFunc,
 ///
 void CloneAndPruneFunctionInto(Function *NewFunc, const Function *OldFunc,
                                ValueToValueMapTy &VMap, bool ModuleLevelChanges,
-                               SmallVectorImpl<ReturnInst*> &Returns,
+                               SmallVectorImpl<ReturnInst *> &Returns,
                                const char *NameSuffix = "",
                                ClonedCodeInfo *CodeInfo = nullptr);
 
@@ -360,32 +369,31 @@ void updateProfileCallee(
 /// Find the 'llvm.experimental.noalias.scope.decl' intrinsics in the specified
 /// basic blocks and extract their scope. These are candidates for duplication
 /// when cloning.
-void identifyNoAliasScopesToClone(
-    ArrayRef<BasicBlock *> BBs, SmallVectorImpl<MDNode *> &NoAliasDeclScopes);
+void identifyNoAliasScopesToClone(ArrayRef<BasicBlock *> BBs,
+                                  SmallVectorImpl<MDNode *> &NoAliasDeclScopes);
 
 /// Find the 'llvm.experimental.noalias.scope.decl' intrinsics in the specified
 /// instruction range and extract their scope. These are candidates for
 /// duplication when cloning.
-void identifyNoAliasScopesToClone(
-    BasicBlock::iterator Start, BasicBlock::iterator End,
-    SmallVectorImpl<MDNode *> &NoAliasDeclScopes);
+void identifyNoAliasScopesToClone(BasicBlock::iterator Start,
+                                  BasicBlock::iterator End,
+                                  SmallVectorImpl<MDNode *> &NoAliasDeclScopes);
 
 /// Duplicate the specified list of noalias decl scopes.
 /// The 'Ext' string is added as an extension to the name.
 /// Afterwards, the ClonedScopes contains the mapping of the original scope
 /// MDNode onto the cloned scope.
 /// Be aware that the cloned scopes are still part of the original scope domain.
-void cloneNoAliasScopes(
-    ArrayRef<MDNode *> NoAliasDeclScopes,
-    DenseMap<MDNode *, MDNode *> &ClonedScopes,
-    StringRef Ext, LLVMContext &Context);
+void cloneNoAliasScopes(ArrayRef<MDNode *> NoAliasDeclScopes,
+                        DenseMap<MDNode *, MDNode *> &ClonedScopes,
+                        StringRef Ext, LLVMContext &Context);
 
 /// Adapt the metadata for the specified instruction according to the
 /// provided mapping. This is normally used after cloning an instruction, when
 /// some noalias scopes needed to be cloned.
-void adaptNoAliasScopes(
-    llvm::Instruction *I, const DenseMap<MDNode *, MDNode *> &ClonedScopes,
-    LLVMContext &Context);
+void adaptNoAliasScopes(llvm::Instruction *I,
+                        const DenseMap<MDNode *, MDNode *> &ClonedScopes,
+                        LLVMContext &Context);
 
 /// Clone the specified noalias decl scopes. Then adapt all instructions in the
 /// NewBlocks basicblocks to the cloned versions.
diff --git a/llvm/lib/Transforms/Utils/CloneFunction.cpp b/llvm/lib/Transforms/Utils/CloneFunction.cpp
index 4306e5d2abdd18..1f4cdec333a2c2 100644
--- a/llvm/lib/Transforms/Utils/CloneFunction.cpp
+++ b/llvm/lib/Transforms/Utils/CloneFunction.cpp
@@ -215,6 +215,59 @@ void llvm::CloneFunctionMetadataInto(Function *NewFunc, const Function *OldFunc,
   }
 }
 
+void llvm::CloneFunctionBodyInto(Function *NewFunc, const Function *OldFunc,
+                                 ValueToValueMapTy &VMap, RemapFlags RemapFlag,
+                                 SmallVectorImpl<ReturnInst *> &Returns,
+                                 const char *NameSuffix,
+                                 ClonedCodeInfo *CodeInfo,
+                                 ValueMapTypeRemapper *TypeMapper,
+                                 ValueMaterializer *Materializer) {
+  if (OldFunc->isDeclaration())
+    return;
+
+  // Loop over all of the basic blocks in the function, cloning them as
+  // appropriate.  Note that we save BE this way in order to handle cloning of
+  // recursive functions into themselves.
+  for (const BasicBlock &BB : *OldFunc) {
+
+    // Create a new basic block and copy instructions into it!
+    BasicBlock *CBB = CloneBasicBlock(&BB, VMap, NameSuffix, NewFunc, CodeInfo);
+
+    // Add basic block mapping.
+    VMap[&BB] = CBB;
+
+    // It is only legal to clone a function if a block address within that
+    // function is never referenced outside of the function.  Given that, we
+    // want to map block addresses from the old function to block addresses in
+    // the clone. (This is different from the generic ValueMapper
+    // implementation, which generates an invalid blockaddress when
+    // cloning a function.)
+    if (BB.hasAddressTaken()) {
+      Constant *OldBBAddr = BlockAddress::get(const_cast<Function *>(OldFunc),
+                                              const_cast<BasicBlock *>(&BB));
+      VMap[OldBBAddr] = BlockAddress::get(NewFunc, CBB);
+    }
+
+    // Note return instructions for the caller.
+    if (ReturnInst *RI = dyn_cast<ReturnInst>(CBB->getTerminator()))
+      Returns.push_back(RI);
+  }
+
+  // Loop over all of the instructions in the new function, fixing up operand
+  // references as we go. This uses VMap to do all the hard work.
+  for (Function::iterator
+           BB = cast<BasicBlock>(VMap[&OldFunc->front()])->getIterator(),
+           BE = NewFunc->end();
+       BB != BE; ++BB)
+    // Loop over all instructions, fixing each one as we find it, and any
+    // attached debug-info records.
+    for (Instruction &II : *BB) {
+      RemapInstruction(&II, VMap, RemapFlag, TypeMapper, Materializer);
+      RemapDbgRecordRange(II.getModule(), II.getDbgRecordRange(), VMap,
+                          RemapFlag, TypeMapper, Materializer);
+    }
+}
+
 // Clone OldFunc into NewFunc, transforming the old arguments into references to
 // VMap values.
 void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc,
@@ -281,47 +334,8 @@ void llvm::CloneFunctionInto(Function *NewFunc, const Function *OldFunc,
   CloneFunctionMetadataInto(NewFunc, OldFunc, VMap, RemapFlag, TypeMapper,
                             Materializer);
 
-  // Loop over all of the basic blocks in the function, cloning them as
-  // appropriate.  Note that we save BE this way in order to handle cloning of
-  // recursive functions into themselves.
-  for (const BasicBlock &BB : *OldFunc) {
-
-    // Create a new basic block and copy instructions into it!
-    BasicBlock *CBB = CloneBasicBlock(&BB, VMap, NameSuffix, NewFunc, CodeInfo);
-
-    // Add basic block mapping.
-    VMap[&BB] = CBB;
-
-    // It is only legal to clone a function if a block address within that
-    // function is never referenced outside of the function.  Given that, we
-    // want to map block addresses from the old function to block addresses in
-    // the clone. (This is different from the generic ValueMapper
-    // implementation, which generates an invalid blockaddress when
-    // cloning a function.)
-    if (BB.hasAddressTaken()) {
-      Constant *OldBBAddr = BlockAddress::get(const_cast<Function *>(OldFunc),
-                                              const_cast<BasicBlock *>(&BB));
-      VMap[OldBBAddr] = BlockAddress::get(NewFunc, CBB);
-    }
-
-    // Note return instructions for the caller.
-    if (ReturnInst *RI = dyn_cast<ReturnInst>(CBB->getTerminator()))
-      Returns.push_back(RI);
-  }
-
-  // Loop over all of the instructions in the new function, fixing up operand
-  // references as we go. This uses VMap to do all the hard work.
-  for (Function::iterator
-           BB = cast<BasicBlock>(VMap[&OldFunc->front()])->getIterator(),
-           BE = NewFunc->end();
-       BB != BE; ++BB)
-    // Loop over all instructions, fixing each one as we find it, and any
-    // attached debug-info records.
-    for (Instruction &II : *BB) {
-      RemapInstruction(&II, VMap, RemapFlag, TypeMapper, Materializer);
-      RemapDbgRecordRange(II.getModule(), II.getDbgRecordRange(), VMap,
-                          RemapFlag, TypeMapper, Materializer);
-    }
+  CloneFunctionBodyInto(NewFunc, OldFunc, VMap, RemapFlag, Returns, NameSuffix,
+                        CodeInfo, TypeMapper, Materializer);
 
   // Only update !llvm.dbg.cu for DifferentModule (not CloneModule). In the
   // same module, the compile unit will already be listed (or not). When

@artempyanykh artempyanykh force-pushed the users/artempyanykh/fast-coro-upstream/5 branch from 4bda093 to 44ae17c Compare December 4, 2024 13:16
@artempyanykh artempyanykh force-pushed the users/artempyanykh/fast-coro-upstream/4 branch from 4c89772 to 36b917e Compare December 4, 2024 13:16
jollaitbot pushed a commit to sailfishos-mirror/llvm-project that referenced this pull request Dec 4, 2024
Summary:
This and previously extracted `CloneFunction*Into` functions will be used in later diffs.

Test Plan:
ninja check-llvm-unit check-llvm

stack-info: PR: llvm/llvm-project#118624, branch: users/artempyanykh/fast-coro-upstream/5
@artempyanykh artempyanykh force-pushed the users/artempyanykh/fast-coro-upstream/4 branch from 36b917e to 8540f25 Compare December 6, 2024 12:42
artempyanykh added a commit that referenced this pull request Dec 6, 2024
Summary:
This and previously extracted `CloneFunction*Into` functions will be used in later diffs.

Test Plan:
ninja check-llvm-unit check-llvm

stack-info: PR: #118624, branch: users/artempyanykh/fast-coro-upstream/5
@artempyanykh artempyanykh force-pushed the users/artempyanykh/fast-coro-upstream/5 branch from 44ae17c to 39cccf5 Compare December 6, 2024 12:42
@artempyanykh artempyanykh changed the base branch from users/artempyanykh/fast-coro-upstream/4 to main December 6, 2024 14:03
@artempyanykh artempyanykh force-pushed the users/artempyanykh/fast-coro-upstream/5 branch from 39cccf5 to 4d03e3f Compare December 6, 2024 14:04
@artempyanykh artempyanykh changed the base branch from main to users/artempyanykh/fast-coro-upstream/4 December 6, 2024 14:04
jollaitbot pushed a commit to sailfishos-mirror/llvm-project that referenced this pull request Dec 6, 2024
Summary:
This and previously extracted `CloneFunction*Into` functions will be used in later diffs.

Test Plan:
ninja check-llvm-unit check-llvm

stack-info: PR: llvm/llvm-project#118624, branch: users/artempyanykh/fast-coro-upstream/5
@artempyanykh artempyanykh force-pushed the users/artempyanykh/fast-coro-upstream/4 branch from ca3fb5e to 8ce25ca Compare December 9, 2024 12:40
artempyanykh added a commit that referenced this pull request Dec 9, 2024
Summary:
This and previously extracted `CloneFunction*Into` functions will be used in later diffs.

Test Plan:
ninja check-llvm-unit check-llvm

stack-info: PR: #118624, branch: users/artempyanykh/fast-coro-upstream/5
@artempyanykh artempyanykh force-pushed the users/artempyanykh/fast-coro-upstream/5 branch from 4d03e3f to d5ed405 Compare December 9, 2024 12:40
@artempyanykh artempyanykh changed the base branch from users/artempyanykh/fast-coro-upstream/4 to main December 9, 2024 16:57
@artempyanykh artempyanykh force-pushed the users/artempyanykh/fast-coro-upstream/5 branch from d5ed405 to 6845960 Compare December 9, 2024 16:57
@artempyanykh artempyanykh changed the base branch from main to users/artempyanykh/fast-coro-upstream/4 December 9, 2024 16:57
artempyanykh added a commit to artempyanykh/llvm-project that referenced this pull request Dec 9, 2024
Summary:
This and previously extracted `CloneFunction*Into` functions will be used in later diffs.

Test Plan:
ninja check-llvm-unit check-llvm

stack-info: PR: llvm#118624, branch: users/artempyanykh/fast-coro-upstream/5
@@ -194,6 +194,15 @@ void CloneFunctionMetadataInto(Function *NewFunc, const Function *OldFunc,
ValueMapTypeRemapper *TypeMapper = nullptr,
ValueMaterializer *Materializer = nullptr);

/// Clone OldFunc's body into NewFunc.
void CloneFunctionBodyInto(Function *NewFunc, const Function *OldFunc,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can NewFunc / OldFunc be nullptr? If not, they should be references

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Updated them to be references.


/// Adapt the metadata for the specified instruction according to the
/// provided mapping. This is normally used after cloning an instruction, when
/// some noalias scopes needed to be cloned.
void adaptNoAliasScopes(
llvm::Instruction *I, const DenseMap<MDNode *, MDNode *> &ClonedScopes,
LLVMContext &Context);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are any of these changes part of the PR? It not, they should not be part of the diff.

If you run into this as a result of running clang-format in the entire patch, a good way to avoid this is by running clang-format on the diff only. For example, (assuming all the changes are unstaged) git add the changes you want to stage, and then git clang-format --staged

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, sorry, it was an accidental whole-file format. I backed out those formatting changes.

@artempyanykh artempyanykh force-pushed the users/artempyanykh/fast-coro-upstream/4 branch from 9dcfc56 to cec0213 Compare December 10, 2024 08:10
artempyanykh added a commit that referenced this pull request Dec 10, 2024
Summary:
This and previously extracted `CloneFunction*Into` functions will be used in later diffs.

Test Plan:
ninja check-llvm-unit check-llvm

stack-info: PR: #118624, branch: users/artempyanykh/fast-coro-upstream/5
@artempyanykh artempyanykh force-pushed the users/artempyanykh/fast-coro-upstream/5 branch from 6845960 to 77bb594 Compare December 10, 2024 08:10
Summary:
This and previously extracted `CloneFunction*Into` functions will be used in later diffs.

Test Plan:
ninja check-llvm-unit check-llvm

stack-info: PR: #118624, branch: users/artempyanykh/fast-coro-upstream/5
@artempyanykh artempyanykh changed the base branch from users/artempyanykh/fast-coro-upstream/4 to main December 11, 2024 07:48
@artempyanykh artempyanykh force-pushed the users/artempyanykh/fast-coro-upstream/5 branch from 77bb594 to 1560bb0 Compare December 11, 2024 07:48
@artempyanykh artempyanykh changed the base branch from main to users/artempyanykh/fast-coro-upstream/4 December 11, 2024 07:48
@artempyanykh artempyanykh changed the base branch from users/artempyanykh/fast-coro-upstream/4 to main December 11, 2024 09:32
@artempyanykh artempyanykh changed the base branch from main to users/artempyanykh/fast-coro-upstream/4 December 11, 2024 09:32
jollaitbot pushed a commit to sailfishos-mirror/llvm-project that referenced this pull request Dec 11, 2024
Summary:
This and previously extracted `CloneFunction*Into` functions will be used in later diffs.

Test Plan:
ninja check-llvm-unit check-llvm

stack-info: PR: llvm/llvm-project#118624, branch: users/artempyanykh/fast-coro-upstream/5
Base automatically changed from users/artempyanykh/fast-coro-upstream/4 to main December 16, 2024 20:50
@artempyanykh artempyanykh force-pushed the users/artempyanykh/fast-coro-upstream/5 branch from 1560bb0 to b679aaa Compare December 16, 2024 20:50
@artempyanykh artempyanykh merged commit 8402a0f into main Dec 16, 2024
8 checks passed
@artempyanykh artempyanykh deleted the users/artempyanykh/fast-coro-upstream/5 branch December 16, 2024 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants