[Mlir-commits] [flang] [mlir] [mlir] Mark dump() methods with LLVM_DUMP_METHOD (PR #233398)

[Eugene Epshteyn]

================
@@ -123,7 +123,9 @@ class OpState {
   }
 
   /// Dump this operation.
-  void dump() { state->dump(); }
+#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
+  LLVM_DUMP_METHOD void dump() { state->dump(); }
----------------
eugeneepshteyn wrote:

(This comment was generated with the help of AI)

Because `LLVM_DUMP_METHOD` is `used`/`retain` outside NDEBUG, this in-class body (and `Location::dump()` in Location.h) is now emitted in every non-NDEBUG TU that includes the header, even one that never calls `dump()`, with an undefined reference to `Operation::dump()` / `Attribute::dump()`. This PR removes both of those from NDEBUG builds of MLIR, so a Debug project linking against a Release MLIR fails to link, and neither `--gc-sections` nor LTO helps (`retain` keeps the section). It is the same failure you hit with `ActionActiveStack`; defining these bodies out of line under the guard avoids it.

Minimal model of the same shape (`main.cpp` never calls `dump()`):

```c++
// lib.h
#if !defined(NDEBUG)
#define DUMP_METHOD __attribute__((noinline, used, retain))
#endif
struct Op { void dump(); };
struct OpState {
  Op *state;
#if !defined(NDEBUG)
  DUMP_METHOD void dump() { state->dump(); }
#endif
};

// lib.cpp
#include "lib.h"
#if !defined(NDEBUG)
void Op::dump() {}
#endif

// main.cpp
#include "lib.h"
int main() { OpState s{nullptr}; return s.state != nullptr; }
```

```console
$ clang++ -O2 -DNDEBUG -c lib.cpp && clang++ -O0 -c main.cpp && clang++ main.o lib.o
/usr/bin/ld: main.o: in function `OpState::dump()':
main.cpp:(.text._ZN7OpState4dumpEv[_ZN7OpState4dumpEv]+0x14): undefined reference to `Op::dump()'
```

(Same with g++.)

With the real headers, a TU that includes `BuiltinOps.h` and never calls `dump()` shows `W mlir::OpState::dump()`, `U mlir::Operation::dump()`, `W mlir::Location::dump() const`, `U mlir::Attribute::dump() const` in `nm -C`. `MemRefDependenceGraph::dump()` in Affine/Analysis/Utils.h has the same shape; there it only forces a link dependency on MLIRAffineAnalysis, but it would be good to move it too.

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