DavidSpickett added a comment. > If multiple instances of the -arm-implicit-it option is passed to > the backend, it errors out.
Does it make sense to fix that side too? Seems like we have a few options to handle this on clang's side: 1. Last of `-mimplicit-it` and `-Wa,-mimplicit-it` wins 2. If they're the same, accept it, if they mismatch, error 3. Take the (last) one that matches the current input type There is some precedent for number 3 in 1d51c699b9e2ebc5bcfdbe85c74cc871426333d4 <https://reviews.llvm.org/rG1d51c699b9e2ebc5bcfdbe85c74cc871426333d4>. Quoting from the commit msg: Now the same rules will apply to -Wa,-march/-mcpu as would if you just passed them to the compiler: * -Wa/-Xassembler options only apply to assembly files. * Architecture derived from mcpu beats any march options. * When there are multiple mcpu or multiple march, the last one wins. * If there is a compiler option and an assembler option of the same type, we prefer the one that fits the input type. * If there is an applicable mcpu option but it is overruled by an march, the cpu value is still used for the "-target-cpu" cc1 option. I'm not up on all the background to this change so perhaps doing number 3 is going to break existing use cases. ================ Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2584 } + if (!ImplicitItCompiler.empty() && !ImplicitItAsm.empty()) { + // Set both via compiler flag and asm flag; ok if they match. ---------------- ``` if (ImplicitItCompiler.size() && ImplicitItAsm.size()) { ``` ================ Comment at: clang/test/Driver/arm-target-as-mimplicit-it.s:39 // ALWAYS: "-mllvm" "-arm-implicit-it=always" +// ALWAYS-NOT: "-arm-implicit-it={{.*}}" // NEVER: "-mllvm" "-arm-implicit-it=never" ---------------- mstorsjo wrote: > This pattern wouldn't detct if there's e.g. `-arm-implicit-it=never > -arm-implicit-it=always`, but I added test cases that pass > `-Wa,-mimplicit-it=always` twice, where this check should be able to verify > that we only output it once. Could you do: ``` // ALWAYS-NOT: "-mllvm" "-arm-implicit-it=never" // ALWAYS: "-mllvm" "-arm-implicit-it=always" ``` Not sure if doing the NOT moves the check forward to beyond where the always would be. ================ Comment at: clang/test/Driver/arm-target-as-mimplicit-it.s:44 // THUMB: "-mllvm" "-arm-implicit-it=thumb" -// NEVER_ALWAYS: "-mllvm" "-arm-implicit-it=never" "-mllvm" "-arm-implicit-it=always" -// ALWAYS_NEVER: "-mllvm" "-arm-implicit-it=always" "-mllvm" "-arm-implicit-it=never" +// MISMATCH: error: unsupported argument '{{.*}}' to option '-mimplicit-it' // INVALID: error: unsupported argument '-mimplicit-it=foo' to option 'Wa,' ---------------- Can you do this check without the regex? Reading it as is this doesn't look like a very helpful error. I'd expect something like: ``` error: mismatched values for implicit-it "thumb" and "arm" ``` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D102812/new/ https://reviews.llvm.org/D102812 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
