[TargetLowering] remove unused virtual from ComputeConstraintToUse

[nickdesaulniers]

There are no overrides in tree.

Fix up resulting linkage failures in unittests and sort the dependency
lists.

[llvmbot]

@llvm/pr-subscribers-llvm-analysis

Changes

There are no overrides in tree.

Fix up resulting linkage failures in unittests and sort the dependency
lists.

---

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

3 Files Affected:

• (modified) llvm/include/llvm/CodeGen/TargetLowering.h (+2-3)
• (modified) llvm/unittests/Analysis/CMakeLists.txt (+2-1)
• (modified) llvm/unittests/Passes/Plugins/CMakeLists.txt (+1-1)

diff --git a/llvm/include/llvm/CodeGen/TargetLowering.h b/llvm/include/llvm/CodeGen/TargetLowering.h
index c46d78e460b3250..0b4222ba2feb707 100644
--- a/llvm/include/llvm/CodeGen/TargetLowering.h
+++ b/llvm/include/llvm/CodeGen/TargetLowering.h
@@ -4836,9 +4836,8 @@ class TargetLowering : public TargetLoweringBase {
   /// AsmOperandInfo, setting OpInfo.ConstraintCode and OpInfo.ConstraintType.
   /// If the actual operand being passed in is available, it can be passed in as
   /// Op, otherwise an empty SDValue can be passed.
-  virtual void ComputeConstraintToUse(AsmOperandInfo &OpInfo,
-                                      SDValue Op,
-                                      SelectionDAG *DAG = nullptr) const;
+  void ComputeConstraintToUse(AsmOperandInfo &OpInfo, SDValue Op,
+                              SelectionDAG *DAG = nullptr) const;
 
   /// Given a constraint, return the type of constraint it is for this target.
   virtual ConstraintType getConstraintType(StringRef Constraint) const;
diff --git a/llvm/unittests/Analysis/CMakeLists.txt b/llvm/unittests/Analysis/CMakeLists.txt
index 847430bf17697ab..6e5bb703dacf188 100644
--- a/llvm/unittests/Analysis/CMakeLists.txt
+++ b/llvm/unittests/Analysis/CMakeLists.txt
@@ -2,11 +2,12 @@ set(LLVM_LINK_COMPONENTS
   Analysis
   AsmParser
   Core
+  IPO
   Passes
+  SelectionDAG
   Support
   TargetParser
   TransformUtils
-  IPO
   )
 
 set(ANALYSIS_TEST_SOURCES
diff --git a/llvm/unittests/Passes/Plugins/CMakeLists.txt b/llvm/unittests/Passes/Plugins/CMakeLists.txt
index e90cae167bc2223..986ce1a55bf4f5d 100644
--- a/llvm/unittests/Passes/Plugins/CMakeLists.txt
+++ b/llvm/unittests/Passes/Plugins/CMakeLists.txt
@@ -3,7 +3,7 @@
 # work with DLLs on Windows (where a shared library can't have undefined
 # references), so just skip this testcase on Windows.
 if (NOT WIN32 AND NOT CYGWIN)
-  set(LLVM_LINK_COMPONENTS Support Passes Core AsmParser)
+  set(LLVM_LINK_COMPONENTS Support Passes Core AsmParser SelectionDAG)
   add_llvm_unittest(PluginsTests
     PluginsTest.cpp
     )

[nickdesaulniers]

IIUC, looks like I need to fix the cmake rules for llvm-extract as well.

8367/9684] Linking CXX executable bin\llvm-extract.exe
--
  | FAILED: bin/llvm-extract.exe

[nickdesaulniers]

are there gn or bazel files that need to updated, too?

[nickdesaulniers]

polly\unittests\ needs this as well...

[nickdesaulniers]

@petrhosek do you think there's a better way than adding these linkage dependencies? Perhaps some definitions need to be moved from somewhere to somewhere else? Perhaps that's a bigger yak shave?

[petrhosek]

@petrhosek do you think there's a better way than adding these linkage dependencies? Perhaps some definitions need to be moved from somewhere to somewhere else? Perhaps that's a bigger yak shave?

Do you know where is the reference to ComputeConstraintToUse coming from?

[nickdesaulniers]

@rnk found https://reviews.llvm.org/D140893 which may have inadvertently exposed a layering violation between Passes and Codegen.

cc @aeubanks @sparker-arm

[rnk]

I'd say that we have an unwanted dependency between Passes and CodeGen as a result of the codegen pass NPM work. IR-only tools like llvm-extract now depend on CodeGen. Fixing that would be nice conceptually, but it might not be high priority, and it's not creating fragility.

The layering issue is that CodeGen has a header dependency on SelectionDAG without having a library link dependency on it. CodeGenPrepare calls ComputeConstraintToUse, which is defined in SelectionDAG:
https://github.com/llvm/llvm-project/blob/main/llvm/lib/CodeGen/CodeGenPrepare.cpp#L5024

If you make the virtual call direct, it results in a linker error. This code can't execute at runtime if you don't link in any targets that implement TargetLowering anyway, llvm-extract just depends on it transitively.

I'm not sure what the next step is, I think I touched this before and looked the other way.

[nickdesaulniers]

I don't have time to follow up on this. Closing.