The semantic of -mmlir are identical to -mllvm. The only notable difference is that -mmlir options should be forwarded to MLIR rather than LLVM.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
I don't know the command line library that well, so I have this curiosity: what happens if LLVM and MLIR have 2 different options with the same name? Do we get a compile time error? Or is there a risk that someone might -mllvm -XYZ and it would end up in MLIR's XYZ option instead, because we're processing the MLIR options after the LLVM ones?
flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp | ||
---|---|---|
15 | Nit: Should these come after the llvm/ headers? (So it's alphabetical except for flang, which may go first) | |
flang/test/Driver/mllvm_vs_mmlir.f90 | ||
18 | Is this the most llvm-ish option we have? I'm concerned that MLIR might also decide to add an option that sounds like this (and for sure -print-ir-before-all is mentioned in the MLIR docs). |
The llvm::cl API uses global variables. So, if a variable is defined twice, you will get a compilation error. I will advertise this before merging so that folks from LLVM and MLIR are aware of this change. I'm hoping that they won't mind having to use distinct variable names for CL options in MLIR and LLVM.
Or is there a risk that someone might -mllvm -XYZ and it would end up in MLIR's XYZ option instead, because we're processing the MLIR options after the LLVM ones?
Note that -mllvm options are saved in llvmArgs and -mmlir options are saved in mlirArgs (this is taken care of in "CompilerInvocation.cpp"). This should guarantee that the right option set is used.
flang/test/Driver/mllvm_vs_mmlir.f90 | ||
---|---|---|
18 |
Sadly, most LLVM options have rather generic names that could at some point be added in MLIR. Perhaps you'll spot something more suitable: USAGE: flang (LLVM option parsing) [options] OPTIONS: Color Options: --color - Use colors in output (default=autodetect) General options: --aarch64-neon-syntax=<value> - Choose style of NEON code to emit from AArch64 backend: =generic - Emit generic NEON assembly =apple - Emit Apple-style NEON assembly --aarch64-use-aa - Enable the use of AA during codegen. --abort-on-max-devirt-iterations-reached - Abort when the max iterations for devirtualization CGSCC repeat pass is reached --allow-ginsert-as-artifact - Allow G_INSERT to be considered an artifact. Hack around AMDGPU test infinite loops. --always-execute-loop-body - force the body of a loop to execute at least once --array-constructor-initial-buffer-size=<uint> - set the incremental array construction buffer size (default=32) --cfg-hide-cold-paths=<number> - Hide blocks with relative frequency below the given value --cfg-hide-deoptimize-paths - --cfg-hide-unreachable-paths - --cost-kind=<value> - Target cost kind =throughput - Reciprocal throughput =latency - Instruction latency =code-size - Code size =size-latency - Code size and latency --debugify-level=<value> - Kind of debug info to add =locations - Locations only =location+variables - Locations and Variables --debugify-quiet - Suppress verbose debugify output --default-kinds=<default-kind-string> - string to set default kind values --disable-i2p-p2i-opt - Disables inttoptr/ptrtoint roundtrip optimization --dot-cfg-mssa=<file name for generated dot file> - file name for generated dot file --enable-cse-in-irtranslator - Should enable CSE in irtranslator --enable-cse-in-legalizer - Should enable CSE in Legalizer --enable-gvn-memdep - --enable-load-in-loop-pre - --enable-load-pre - --enable-loop-simplifycfg-term-folding - --enable-name-compression - Enable name/filename string compression --enable-split-backedge-in-load-pre - --experimental-debug-variable-locations - Use experimental new value-tracking variable locations --fdebug-dump-pre-fir - dump the Pre-FIR tree prior to FIR generation --fs-profile-debug-bw-threshold=<uint> - Only show debug message if the source branch weight is greater than this value. --fs-profile-debug-prob-diff-threshold=<uint> - Only show debug message if the branch probility is greater than this value (in percentage). --gen-array-coor - in lowering create ArrayCoorOp instead of CoordinateOp --generate-merged-base-profiles - When generating nested context-sensitive profiles, always generate extra base profile for function with all its context profiles merged into it. --inline-all - aggressively inline everything --instcombine-code-sinking - Enable code sinking --instcombine-guard-widening-window=<uint> - How wide an instruction window to bypass looking for another guard --instcombine-max-iterations=<uint> - Limit the maximum number of instruction combining iterations --instcombine-max-num-phis=<uint> - Maximum number phis to handle in intptr/ptrint folding --instcombine-max-sink-users=<uint> - Maximum number of undroppable users for instruction sinking --instcombine-maxarray-size=<uint> - Maximum array size considered when doing a combine --instcombine-negator-enabled - Should we attempt to sink negations? --instcombine-negator-max-depth=<uint> - What is the maximal lookup depth when trying to check for viability of negation sinking. --kind-mapping=<kind-mapping-string> - kind mapping string to set kind precision --length-to-hash-string-literal=<ulong> - string literals that exceed this length will use a hash value as their symbol name --main-entry-name=<string> - override the name of the default PROGRAM entry (may be helpful for using other runtimes) --math-runtime=<value> - Select math runtime version: =fast - use pgmath fast runtime =relaxed - use pgmath relaxed runtime =precise - use pgmath precise runtime =llvm - only use LLVM intrinsics (may be incomplete) --matrix-default-layout=<value> - Sets the default matrix layout =column-major - Use column-major layout =row-major - Use row-major layout --mir-strip-debugify-only - Should mir-strip-debug only strip debug info from debugified modules by default --no-discriminators - Disable generation of discriminator information. --non-recursive-procedures - Make procedures non-recursive by default. This was the default for all Fortran standards prior to 2018. --opaque-pointers - Use opaque pointers --outline-intrinsics - Lower all intrinsic procedure implementation in their own functions --safepoint-ir-verifier-print-only - --sample-profile-check-record-coverage=<N> - Emit a warning if less than N% of records in the input profile are matched to the IR. --sample-profile-check-sample-coverage=<N> - Emit a warning if less than N% of samples in the input profile are matched to the IR. --sample-profile-max-propagate-iterations=<uint> - Maximum number of iterations to go through when propagating sample block/edge weights through the CFG. --use-alloc-runtime - Lower allocations to fortran runtime calls --use-desc-for-alloc - Always use descriptors for POINTER and ALLOCATABLE --verify-legalizer-debug-locs=<value> - Verify that debug locations are handled =none - No verification =legalizations - Verify legalizations =legalizations+artifactcombiners - Verify legalizations and artifact combines --verify-region-info - Verify region info (time consuming) Generic Options: --help - Display available options (--help-hidden for more) --help-list - Display list of available options (--help-list-hidden for more) --version - Display the version of this program | |
18 |
Doh, I updated the test before sending this, but forgot to re-run it :( Should be fixed now. |
flang/test/Driver/mllvm_vs_mmlir.f90 | ||
---|---|---|
18 | I'd be alright with a patch to MLIR that prefixed the pass manager options with mlir-. The rest of our options (that are provided by libraries) should already be prefixed with that AFAIK. |
flang/test/Driver/mllvm_vs_mmlir.f90 | ||
---|---|---|
18 | Sounds like a plan, thanks for the suggestion! |
-mmlir is fine with me, but note that MLIR has much less global options than LLVM: you will only get access to context and passmanager options and not individual passes flags. That's not a criticism :)
(that's what makes it not likely to have conflicts by the way: the set of MLIR option is well identified)
Thanks for taking a look! All this should be fine - the main rationale for this patch is to enable MLIR options in flang-new so that tests using tco or bbc (and which use MLIR options) can be ported to use flang-new. We were discussing this recently in https://reviews.llvm.org/D121171.
flang/test/Driver/mllvm_vs_mmlir.f90 | ||
---|---|---|
18 |
flang/test/Driver/mllvm_vs_mmlir.f90 | ||
---|---|---|
18 | Should we add this as a parent then? |
flang/test/Driver/mllvm_vs_mmlir.f90 | ||
---|---|---|
18 | I've just merged it, so can re-base this patch instead. |
I did a bit of digging and realised that MLIR llvm::cl options are lazily constructed on demand (see the definition of options in PassManagerOptions.cpp). This means that:
- Flang and LLVM global options are always visible and displayed when passing -mllvm -help,
- MLIR global options are only visible when explicitly initialised and displayed when passing -mmlir -help.
Since Flang and LLVM global options are always visible, llvm::cl will display them alongside MLIR options when using -mmlir -help. In other words, -mmlir -help is a superset of --mllvm -help. This is not ideal, but we'd need to refactor all option definitions in Flang and LLVM to improve this. I suggesting leaving this for later.
Rebase on top of main, re-fine the test
Make sure that the test file correctly takes into account that -mmlir -help is a superset of -mllvm -help.
flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp | ||
---|---|---|
15 | Thanks, I will fix this before merging! |
clang/include/clang/Driver/Options.td | ||
---|---|---|
3263 |
clang/include/clang/Driver/Options.td | ||
---|---|---|
3263 | Thanks, included in the final version! |