[llvm] [GVN] Enforce MemDep/MemorySSA mutual exclusion for cl::opt overrides (PR #217896)

[via llvm-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-transforms

Author: Madhur Amilkanthwar (madhur13490)

<details>
<summary>Changes</summary>

parseGVNOptions() already keeps the gvn<...> pass parameters mutually exclusive, but the -enable-gvn-{memdep,memoryssa} cl::opt overrides default independently. Two fixes for that path:

- Enabling MemorySSA now implies MemDep is off, so -enable-gvn-memoryssa=true on its own no longer trips the on-demand MemorySSA assertion.
- Explicitly enabling both is a contradiction and is now rejected with a diagnostic instead of resolving it arbitrarily.

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


2 Files Affected:

- (modified) llvm/lib/Transforms/Scalar/GVN.cpp (+15) 
- (added) llvm/test/Transforms/GVN/memoryssa-implies-no-memdep.ll (+18) 


``````````diff
diff --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp
index 64fd14fcba1c5..96b6797605ec1 100644
--- a/llvm/lib/Transforms/Scalar/GVN.cpp
+++ b/llvm/lib/Transforms/Scalar/GVN.cpp
@@ -68,6 +68,7 @@
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/Debug.h"
+#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Transforms/Utils/AssumeBundleBuilder.h"
 #include "llvm/Transforms/Utils/BasicBlockUtils.h"
@@ -869,6 +870,11 @@ bool GVNPass::isLoadPRESplitBackedgeEnabled() const {
 }
 
 bool GVNPass::isMemDepEnabled() const {
+  // MemDep and MemorySSA are mutually exclusive. parseGVNOptions() enforces
+  // this for pass parameters, but the -enable-gvn-{memdep,memoryssa} cl::opt
+  // overrides default independently, so honor MemorySSA winning here too.
+  if (isMemorySSAEnabled())
+    return Options.AllowMemDep.value_or(false);
   return Options.AllowMemDep.value_or(GVNEnableMemDep);
 }
 
@@ -3456,6 +3462,15 @@ bool GVNPass::runImpl(Function &F, AssumptionCache &RunAC, DominatorTree &RunDT,
                       const TargetLibraryInfo &RunTLI, AAResults &RunAA,
                       MemoryDependenceResults *RunMD, LoopInfo &LI,
                       OptimizationRemarkEmitter *RunORE, MemorySSA *MSSA) {
+  // MemDep and MemorySSA are mutually exclusive. isMemDepEnabled() silently
+  // lets MemorySSA win for the common single-flag case, but an explicit
+  // request for both via -enable-gvn-{memdep,memoryssa} is a contradiction we
+  // reject rather than resolve arbitrarily.
+  if (GVNEnableMemDep.getNumOccurrences() && GVNEnableMemDep &&
+      GVNEnableMemorySSA.getNumOccurrences() && GVNEnableMemorySSA)
+    report_fatal_error("GVN: -enable-gvn-memdep and -enable-gvn-memoryssa are "
+                       "mutually exclusive",
+                       /*gen_crash_diag=*/false);
   AC = &RunAC;
   DT = &RunDT;
   VN.setDomTree(DT);
diff --git a/llvm/test/Transforms/GVN/memoryssa-implies-no-memdep.ll b/llvm/test/Transforms/GVN/memoryssa-implies-no-memdep.ll
new file mode 100644
index 0000000000000..def9d5e7ea773
--- /dev/null
+++ b/llvm/test/Transforms/GVN/memoryssa-implies-no-memdep.ll
@@ -0,0 +1,18 @@
+; RUN: opt -passes=gvn -enable-gvn-memoryssa=true -S < %s | FileCheck %s
+;
+; Explicitly requesting both engines is a contradiction and is rejected.
+; RUN: not opt -passes=gvn -enable-gvn-memdep=true -enable-gvn-memoryssa=true \
+; RUN:   -S < %s 2>&1 | FileCheck %s --check-prefix=CONFLICT
+; CONFLICT: -enable-gvn-memdep and -enable-gvn-memoryssa are mutually exclusive
+
+define i32 @redundant_load(ptr %p) {
+; CHECK-LABEL: @redundant_load(
+; CHECK:         %a = load i32, ptr %p
+; CHECK-NOT:     load i32
+; CHECK:         %c = add i32 %a, %a
+; CHECK:         ret i32 %c
+  %a = load i32, ptr %p
+  %b = load i32, ptr %p
+  %c = add i32 %a, %b
+  ret i32 %c
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/217896