[llvm] [llvm-mca] Abort on parse error without -skip-unsupported-instructions (PR #90474)

Peter Waller via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 30 01:44:33 PDT 2024


peterwaller-arm wrote:

I accept your points. The root of my thinking was to preserve backwards compatibility, but I think there are potential reasons to want this.

> To ignore parse errors is to accept programs that are not well formed. Do we really want to support this? Is it useful to have this behavior? What use cases does this have? You wouldn't give a CPU a program with instructions it couldn't support and expect it to work correctly. The compiler wouldn't generate that code for that CPU either.

* llvm-mca today accepts bad input and prints analysis outputs.
* It seems likely that was unintentional, but if anyone is relying on this, accidentally or otherwise, it would be nice to 'give them an out' if they need it.
* tomorrow's new instructions are today's malformed inputs. Yes, one hopes LLVM will gain support for those new instructions, and that a user will be able to upgrade their llvm-mca to gain support of those, but I still think it is useful to have an alternative route if either of these conditions are false for some time. 
* Imagine a case where someone is trying to bisect a behaviour change in llvm-mca, or study mca's output as a function of time for a given (large) set of inputs. They could find themselves running an old llvm-mca against new code. If that use case involved throwing 'lots of code at llvm-mca', it may be useful to ignore instructions which don't parse at older revisions of llvm.

MCA and LLVM's MC parser seems to already handle bad input quite well, and assembler syntax lends itself is simple enough for this. There aren't lots of complex ambiguities that are introduced by ignoring a line, that I can think of, correct me if I'm wrong.

The analysis outputs may be compromised to some degree for bad inputs, but so long as the user is being explicit about allowing it and being warned that the analysis will be less accurate, I don't see the harm in supporting this case.

> the test case should use a different instruction sequence that both CPU can support, or the test case should be split into two cases, one for each CPU.

When I first saw this test case was running with errors behind the scenes I thought this was unfortunate, but later I thought the [test case](
https://github.com/llvm/llvm-project/blob/6b57520107755e8721b61cd4fcd99461be29f5c9/llvm/test/tools/llvm-mca/AArch64/Exynos/float-divide-multiply.s) as it stands is pretty neat for what it's worth. It's autogenerated and has all the CPU generations in one file, and the generation lacking support for the instruction neatly omits the unsupported instructions from the output.

I don't feel the case above is very strong, but I'd like to pursue this unless strong objections are raised.

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


More information about the llvm-commits mailing list