[mlir] NFC: Fix layering check / parse headers violations

[googlewalt]

Those tools check strict dependency and standalone headers in Google, but some internal build optimizations caused some violations not to be detected. This change adds a missing dependency, and includes some types that are needed for template instantiation.

[llvmbot]

@llvm/pr-subscribers-mlir-linalg

@llvm/pr-subscribers-mlir

Author: Walter Lee (googlewalt)

Changes

Those tools check strict dependency and standalone headers in Google, but some internal build optimizations caused some violations not to be detected. This change adds a missing dependency, and includes some types that are needed for template instantiation.

---

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

2 Files Affected:

• (modified) mlir/include/mlir/Dialect/Linalg/TransformOps/LinalgTransformOps.h (+1-4)
• (modified) utils/bazel/llvm-project-overlay/mlir/BUILD.bazel (+1)

diff --git a/mlir/include/mlir/Dialect/Linalg/TransformOps/LinalgTransformOps.h b/mlir/include/mlir/Dialect/Linalg/TransformOps/LinalgTransformOps.h
index db25c9b241734f..9ede21e87cf530 100644
--- a/mlir/include/mlir/Dialect/Linalg/TransformOps/LinalgTransformOps.h
+++ b/mlir/include/mlir/Dialect/Linalg/TransformOps/LinalgTransformOps.h
@@ -14,6 +14,7 @@
 #include "mlir/Dialect/Linalg/IR/Linalg.h"
 #include "mlir/Dialect/Transform/IR/TransformAttrs.h"
 #include "mlir/Dialect/Transform/IR/TransformDialect.h"
+#include "mlir/Dialect/Transform/IR/TransformTypes.h"
 #include "mlir/Dialect/Transform/Interfaces/TransformInterfaces.h"
 #include "mlir/Dialect/Utils/StructuredOpsUtils.h"
 #include "mlir/IR/OpImplementation.h"
@@ -42,10 +43,6 @@ class UnPackOp;
 } // namespace tensor
 
 namespace transform {
-class AnyOpType;
-class AnyValueType;
-class OperationType;
-class TransformHandleTypeInterface;
 // Types needed for builders.
 struct TileSizesSpec {};
 struct NumThreadsSpec {};
diff --git a/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel b/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
index f5437245e8e135..a8b74b2376e71d 100644
--- a/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
@@ -12370,6 +12370,7 @@ cc_library(
     hdrs = glob(["include/mlir/Dialect/Transform/IR/*.h"]),
     deps = [
         ":Analysis",
+        ":BytecodeOpInterface",
         ":CallOpInterfaces",
         ":CastInterfaces",
         ":ControlFlowInterfaces",

[jyknight]

Looks fine to me. But what was the actual error message from parsing LinalgTransformOps.h without #include "mlir/Dialect/Transform/IR/TransformTypes.h"?

[googlewalt]

Here is one example:

In file included from third_party/llvm/llvm-project/mlir/include/mlir/Dialect/Linalg/TransformOps/LinalgTransformOps.h:12:
In file included from third_party/llvm/llvm-project/mlir/include/mlir/Dialect/Bufferization/IR/Bufferization.h:12:
In file included from third_party/llvm/llvm-project/mlir/include/mlir/Bytecode/BytecodeOpInterface.h:17:
In file included from third_party/llvm/llvm-project/mlir/include/mlir/Bytecode/BytecodeImplementation.h:17:
In file included from third_party/llvm/llvm-project/mlir/include/mlir/IR/Attributes.h:12:
In file included from third_party/llvm/llvm-project/mlir/include/mlir/IR/AttributeSupport.h:18:
third_party/llvm/llvm-project/mlir/include/mlir/IR/Types.h:420:14: error: incomplete type 'mlir::transform::AnyValueType' named in nested name specifier
  420 |       return To::classof(ty);
      |              ^~~~
third_party/llvm/llvm-project/llvm/include/llvm/Support/Casting.h:549:36: note: in instantiation of member function 'llvm::CastInfo<mlir::transform::AnyValueType, const mlir::Type>::isPossible' requested here
  549 |   return CastInfo<To, const From>::isPossible(Val);
      |                                    ^
third_party/llvm/llvm-project/mlir/include/mlir/IR/Value.h:447:51: note: in instantiation of function template specialization 'llvm::isa<mlir::transform::AnyValueType, mlir::Type>' requested here
  447 |   static bool classof(Value value) { return llvm::isa<Ty>(value.getType()); }
      |                                                   ^
third_party/llvm/llvm-project/mlir/include/mlir/IR/Value.h:616:18: note: in instantiation of member function 'mlir::detail::TypedValue<mlir::transform::AnyValueType>::classof' requested here
  616 |       return To::classof(ty);
      |                  ^
third_party/llvm/llvm-project/llvm/include/llvm/Support/Casting.h:549:36: note: in instantiation of member function 'llvm::CastInfo<mlir::detail::TypedValue<mlir::transform::AnyValueType>, const mlir::OpResult>::isPossible' requested here
  549 |   return CastInfo<To, const From>::isPossible(Val);
      |                                    ^
third_party/llvm/llvm-project/llvm/include/llvm/Support/Casting.h:566:10: note: in instantiation of function template specialization 'llvm::isa<mlir::detail::TypedValue<mlir::transform::AnyValueType>, mlir::OpResult>' requested here
  566 |   assert(isa<To>(Val) && "cast<Ty>() argument of incompatible type!");
      |          ^
third_party/llvm/llvm-project/mlir/include/mlir/Dialect/Linalg/TransformOps/LinalgTransformOps.h:47:7: note: forward declaration of 'mlir::transform::AnyValueType'
   47 | class AnyValueType;
      |       ^

[joker-eph]

I don't understand why the change in the source file is needed? Why would the build be successful with CMake but not with Bazel?

[jyknight]

The error reported was from the bazel "parse_headers" validation feature, which tries to ensure that headers can be parsed by themselves, as a standalone compile step (via clang -fsyntax-only).

In all the current uses of this header, the source-file already also includes the header defining the required types, so the actual compilation steps succeed just fine (in bazel, too).

[joker-eph]

which tries to ensure that headers can be parsed by themselves

The header is included as the first include here:

llvm-project/mlir/lib/Dialect/Linalg/TransformOps/LinalgTransformOps.cpp

Line 9 in 3f8380f

#include "mlir/Dialect/Linalg/TransformOps/LinalgTransformOps.h"

; why wouldn't this trigger the same bug?

[googlewalt]

Because template instantiation occurs after all the headers have been parsed, and a header that follows supplys the requried definitions.

Without this change, LinalgTransformOps.h is not self contained. You can verify that by trying to compile a cpp file that only has that include in -- it will give the the same kind of errors that I had pasted earlier.