[mlir] Fix a build error and a warning in mlir

[silvas]

mlir/lib/Dialect/Shape/CMakeLists.txt

1

Why is this needed?

[isuruf]

mlir/lib/Dialect/Shape/CMakeLists.txt

1

Because mlir_tablegen call in Line 3 will try to create the IR/ShapeCanonicalization.inc file without creating the IR directory.

[silvas]

@tpopp can you PTAL at this? Surely we aren't the first using DRR inside mlir core, and I didn't find evidence that this CMakeLists.txt annotation was needed for them. Wild guess, but perhaps we can put the ShapeCanonicalization.td in lib/ ?

[silvas]

mlir/Dialect/Vector/CMakeLists.txt doesn't seem to need this?

If this is critically breaking bots then we should revert the original patch. We should take some time to get to the bottom of this specific issue. I don't think that MAKE_DIRECTORY is the right solution here.

[isuruf]

@silvas, mlir_tablegen(IR/ShapeCanonicalization.inc -gen-rewriters) is the only call in the MLIR project with a folder (IR) in it. Others look like mlir_tablegen(SPIRVEnums.h.inc -gen-enum-decls)

[Kayjukh]

Maybe you could move to a slightly different directory structure and follow what other dialects are doing. Using a top-level CMakeLists.txt like

# lib/Dialect/Shape/CMakeLists.txt
add_subdirectory(IR)

and then add the dialect library generation directives in lib/Dialect/Shape/IR?
I think it would make sense to follow the same conventions as, say, the Linalg dialect.

[tpopp]

First, sorry for the breakage.

If you want to revert the changes, for now you would need to revert both https://github.com/llvm/llvm-project/commit/0a554e607ff6247b79d1c4f184999750e5ad53b9 and https://github.com/llvm/llvm-project/commit/6aab70945915ef1d565f1146734416029549a5a9

I'm not able to reproduce it myself and don't see it in the buildbots, but assuming this works, https://reviews.llvm.org/D81328 sounds like a good final end state like Kayjukh suggested, but I could do this and undo the reverts after also.

(I'm not commenting on the DRR because I'm not sure what that stands for).

[tpopp]

As more people are complaining, I'm going ahead and reverting the CLs that added and rely on this new file.

[isuruf]

Closing in favour of https://reviews.llvm.org/D81328

mlir/lib/Dialect/Shape/CMakeLists.txt

+file(MAKE_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/IR)
 set(LLVM_TARGET_DEFINITIONS IR/ShapeCanonicalization.td)
 mlir_tablegen(IR/ShapeCanonicalization.inc -gen-rewriters)
 add_public_tablegen_target(MLIRShapeCanonicalizationIncGen)
 
 add_mlir_dialect_library(MLIRShape
   IR/Shape.cpp
 
   ADDITIONAL_HEADER_DIRS

mlir/lib/Dialect/Shape/IR/Shape.cpp

LogicalResult matchAndRewrite(AssumingOp op,
 
     // Merge blocks together as there was no branching behavior from the
     // AssumingOp.
     rewriter.mergeBlocks(assumingBlock, blockBeforeAssuming);
     rewriter.mergeBlocks(blockAfterAssuming, blockBeforeAssuming);
     return success();
   }
 };
-}; // namespace
+} // namespace
 
 void AssumingOp::getCanonicalizationPatterns(OwningRewritePatternList &patterns,
                                              MLIRContext *context) {
   // If taking a passing witness, inline region
   patterns.insert<AssumingWithTrue>(context);
 }
 
 //===----------------------------------------------------------------------===//