|30 ms||linux > MLIR.Dialect/LLVMIR::roundtrip.mlir|
Script: -- : 'RUN: at line 1'; /mnt/disks/ssd0/agent/llvm-project/build/bin/mlir-opt /mnt/disks/ssd0/agent/llvm-project/mlir/test/Dialect/LLVMIR/roundtrip.mlir -verify-roundtrip | /mnt/disks/ssd0/agent/llvm-project/build/bin/FileCheck /mnt/disks/ssd0/agent/llvm-project/mlir/test/Dialect/LLVMIR/roundtrip.mlir
|50 ms||windows > MLIR.Dialect/LLVMIR::roundtrip.mlir|
Script: -- : 'RUN: at line 1'; c:\ws\w16n2-1\llvm-project\premerge-checks\build\bin\mlir-opt.exe C:\ws\w16n2-1\llvm-project\premerge-checks\mlir\test\Dialect\LLVMIR\roundtrip.mlir -verify-roundtrip | c:\ws\w16n2-1\llvm-project\premerge-checks\build\bin\filecheck.exe C:\ws\w16n2-1\llvm-project\premerge-checks\mlir\test\Dialect\LLVMIR\roundtrip.mlir
IMO, it depends on the user running mlir-opt. If I'm iterating locally, I generally wouldn't want these additional cost/complexity of running this. For tests that run automatically, that default is probably fine. I would strongly prefer that we keep mlir-opt defaults useful for users iterating locally and generally using mlir-opt as a development tool. For testing, could we just have another frontend(alias of mlir-opt?) that has better defaults for testing use cases? We've had conversations of sane defaults in the past, and it seems like the truly contentious ones always have this divide between local development and testing.
I'd also be pro not making it default from orthogonality point of view. Having all tests verifying printing/parsing vs having dedicated tests seems a wrong tradeoff. Yes having it default would catch cases not covered, but the fact that they aren't covered by unit tests is the problematic part that has to be addressed IMHO.
I don't know at which point we refactor this into a options struct, but 6 bools in a row is quite easy to mess up.
Nit: this need not verify custom, depending on other flags passed (unless you set generic form to false on first print I believe the command line option for whether set or not takes preference) this just verifies printing and parsing.
Mmm, we now have 3 bools in a row here, thats a bit error prone
Or potentially rename to alsoVerifyRoundTrip to avoid confusion?