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

[Hanchenng Wu]

================
@@ -36,10 +44,46 @@ void GenerateRuntimeVerificationPass::runOnOperation() {
     ops.push_back(verifiableOp);
   });
 
+  // Create error message generator based on verboseLevel
----------------
HanchengWu wrote:

Hi, Mehdi

I agree with your observation on the current behavior. See my below answers.

-isn't it an issue of the current pass that we're reconstructing the SSA numbering between each application of the instrumentation?

I think my previous understanding of how AsmState works is wrong. Currently, each op-print() will create a new AsmState for each op's assert/error message, and I think the SSA name created by this AsmState will be stored to the op and used by the emitter later for the op. But it seems that this is not the case. In the dumped mlir file after the pass, I see the operand names dumped in the assert/error message does seem to be some names that only exist in the middle of the application of the pass. 

-I would think the user would likely want to match the message to the IR as printed before the pass instead of some state that only exists in the middle of the application of the pass?

Similar to above - the operand names dumped in the assert/error message do seem to be some names that only exist in the middle of the application of the pass. However, the location that gets printed as part of the error message does point to the original location. That said, if the input is an text mlir file, the error message will be able to direct the user to the original line of instruction for which the error is thrown.

-That said we can definitely land this PR as-is, to not affect the behavior too much, and then try to land an update where we construct an AsmState and cache it for the duration of the pass?

Thanks for your thoughts on this. I have submitted a new version that's rebased to tip of llvm. Can you help move this forward?

In addition, I took a look at AsmState definition. It says "The IR should not be mutated in-between invocations using this state, and the IR being printed must not be an parent of the IR originally used to initialize this state. " Since we keep addding new code for verification purposes throughtout the pass, reusing the same AsmState for the duration of the pass seems not feasible. I will look into this and update with you later on this issue.




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