[Mlir-commits] [mlir] [MLIR] Add options to generate-runtime-verification to enable faster pass running (PR #160331)

[Hanchenng Wu]

HanchengWu wrote:

> Can you apply this patch:
> 
> ```
> diff --git a/mlir/lib/Transforms/GenerateRuntimeVerification.cpp b/mlir/lib/Transforms/GenerateRuntimeVerification.cpp
> index cfe531385fef..9a25b7087d4d 100644
> --- a/mlir/lib/Transforms/GenerateRuntimeVerification.cpp
> +++ b/mlir/lib/Transforms/GenerateRuntimeVerification.cpp
> @@ -6,6 +6,7 @@
>  //
>  //===----------------------------------------------------------------------===//
>  
> +#include "mlir/IR/AsmState.h"
>  #include "mlir/Transforms/Passes.h"
>  
>  #include "mlir/IR/Builders.h"
> @@ -43,23 +44,22 @@ void GenerateRuntimeVerificationPass::runOnOperation() {
>    getOperation()->walk([&](RuntimeVerifiableOpInterface verifiableOp) {
>      ops.push_back(verifiableOp);
>    });
> -
> +  OpPrintingFlags flags;
> +  // We may generate a lot of error messages and so we need to ensure the
> +  // printing is fast.
> +  flags.elideLargeElementsAttrs();
> +  flags.skipRegions();
> +  flags.useLocalScope();
> +  AsmState state(getOperation(), flags);
>    // Create error message generator based on verboseLevel
> -  auto errorMsgGenerator = [vLevel = verboseLevel.getValue()](
> +  auto errorMsgGenerator = [vLevel = verboseLevel.getValue(), &state](
>                                 Operation *op, StringRef msg) -> std::string {
>      std::string buffer;
>      llvm::raw_string_ostream stream(buffer);
> -    OpPrintingFlags flags;
> -    // We may generate a lot of error messages and so we need to ensure the
> -    // printing is fast.
> -    flags.elideLargeElementsAttrs();
> -    flags.printGenericOpForm();
> -    flags.skipRegions();
> -    flags.useLocalScope();
>      stream << "ERROR: Runtime op verification failed\n";
>      if (vLevel == 2) {
>        // print full op including operand names, very expensive
> -      op->print(stream, flags);
> +      op->print(stream, state);
>        stream << "\n " << msg;
>      } else if (vLevel == 1) {
>        // print op name and operand types
> ```
> 
> And then redo the benchmarks and the SSA value comparisons and let me know how does this look now?

Hi, 
The patch actually works - constructing an AsmState at the very beginning and use it throughout the pass.

With this patch, I observed the following.

1. Verbose 2 now all finished very quickly, within 0.3 second, similar to verbose 0 and 1.
2. For verbose 2, the string of the op that we inserted in the error message is the same string that is used for the op in the input MLIR file.

This seems to be the most ideal case where in error message we refer to op strings from the input mlir.

I actually saw this version of "op->print(stream, asmState)" before, but since the definition of AsmState mentions that "The IR should not be mutated in-between invocations using this state", I didn't try this method. But it looks working as I tried now. Probably it's because we are only inserting new nodes into the IR, and never deletes anything with the pass, so whatever the old ArmState stored is still valid to old IR nodes. I guess something would break, if we try to print the new node that we inserted during the pass run using this old AsmState.

If you think using this is the safe, then I propose that we move forward and remove verbose 1 option. So the new options would be:

verbose 0: print location in input mlir only.
verbose 1: print op string and location in input mlir. 

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