[Mlir-commits] [mlir] [mlir][vector] Support complete folding in single pass for vector.insert/vector.extract (PR #142124)

[James Newling]

newling wrote:

I agree that this introduces inconsistency about where we test the fold methods. 

> Once such tests exist, a change like yours would ideally be validated by the absence of test diffs, which is more robust.

Can you please say what 'test diffs' are here? Is that running `--canonicalize --debug` and checking how many times the folder is called?

> At a minimum, I recommend moving the new tests to "canonicalize.mlir", so we at least maintain consistency in where we test folding behaviour.

Do you mean duplicate them, once in canonicalize.mlir and once in constant-fold.mlir? Having it only in canonicalize.mlir would not test this PR, because `--canonicalize` will call the op's folder multiple times. 

> If the goal is to verify that the fold happens in a single pass of the folder, that’s effectively testing an implementation detail or driver behaviour

Hmm I'm not sure. For a call to `createOrFold` in a pass that walk the IR just once, the difference is observable, so not something I'd call an implementation detail. 


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