[GVNHoist] Replace combineKnownMetadata with combineMetadataForCSE

[hiraditya]

There is no reason to call combineMetadata directly with a list of MD_ nodes. The combineMetadataForCSE function handles all the metadata correctly

Partially fixes: #30866

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: AdityaK (hiraditya)

Changes

There is no reason to call combineMetadata directly with a list of MD_ nodes. The combineMetadataForCSE function handles all the metadata correctly

Partially fixes: #30866

---

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

1 Files Affected:

• (modified) llvm/lib/Transforms/Scalar/GVNHoist.cpp (+1-13)

diff --git a/llvm/lib/Transforms/Scalar/GVNHoist.cpp b/llvm/lib/Transforms/Scalar/GVNHoist.cpp
index 261c1259c9c96..b5333c532280c 100644
--- a/llvm/lib/Transforms/Scalar/GVNHoist.cpp
+++ b/llvm/lib/Transforms/Scalar/GVNHoist.cpp
@@ -238,18 +238,6 @@ class CallInfo {
   const VNtoInsns &getStoreVNTable() const { return VNtoCallsStores; }
 };
 
-static void combineKnownMetadata(Instruction *ReplInst, Instruction *I) {
-  static const unsigned KnownIDs[] = {LLVMContext::MD_tbaa,
-                                      LLVMContext::MD_alias_scope,
-                                      LLVMContext::MD_noalias,
-                                      LLVMContext::MD_range,
-                                      LLVMContext::MD_fpmath,
-                                      LLVMContext::MD_invariant_load,
-                                      LLVMContext::MD_invariant_group,
-                                      LLVMContext::MD_access_group};
-  combineMetadata(ReplInst, I, KnownIDs, true);
-}
-
 // This pass hoists common computations across branches sharing common
 // dominator. The primary goal is to reduce the code size, and in some
 // cases reduce critical path (by exposing more ILP).
@@ -996,8 +984,8 @@ unsigned GVNHoist::rauw(const SmallVecInsn &Candidates, Instruction *Repl,
         MSSAUpdater->removeMemoryAccess(OldMA);
       }
 
+      combineMetadataForCSE(Repl, I, true);
       Repl->andIRFlags(I);
-      combineKnownMetadata(Repl, I);
       I->replaceAllUsesWith(Repl);
       // Also invalidate the Alias Analysis cache.
       MD->removeInstruction(I);

[nikic]

LGTM

[hiraditya]

Moving combineMetadataForCSE above andIRFlags is intentional for two reasons.

1. In future if andIRFlags does something to manipulate metadata(unlikely), calling it later makes this pass more resilient to change.
2. GVNSink takes a similar approach. https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/Scalar/GVNSink.cpp#L920