[PATCH] D35569: [ARM] Remove FeatureNoARM implies ModeThumb.

[Renato Golin via Phabricator via llvm-commits]

rengolin added a comment.

In https://reviews.llvm.org/D35569#816129, @fhahn wrote:

> With FeatureARM, the generic CPU would have to have this feature, but when the features of the architecture (v6m) are applied, FeatureARM cannot be removed, so the behavior will be different to the current one (Architecture features are applied after CPU features I think). We could rename FeatureARM to FeatureThumbOnly, which is a bit clearer in my opinion.


Correct. `FeatureNoARM` is specific to M-class. I don't mind how we call it as long as it's explicit the intention, and I think both `FeatureNoARM` and `FeatureThumbOnly` are clear.

About the error message, this is a similar problem than https://reviews.llvm.org/D34875: inconsistent states generate the wrong behaviour. But what can we realistically do about it depends on where we're coming from.

For example, if there's an `arm` attribute forcing a function to emit ARM in a Thumb-only core, do we do it? If another attribute forces VFP PCS, do we do it? The answer is: it depends.

If the user is trying to pass "-target armv6m -marm" then, well, the user is wrong and Clang/llc/lli should tell them that.

If the user has an attribute or inline assembly with a directive, then probably the user knows what is needed, and probably has done that on purpose, so, should we deny them of their request?

An example of that not being wrong is the unwinder. We have added `.arch` and `.fpu` directives on functions that unwind registers that are only valid on those sub-architectures, and we guarantee that they will never be called on targets that do not support it. We must have a single unwinder that works on all sub-targets, in the same way we want one kernel to run on all sub-architectures by default, etc.

So, the error message conundrum is somewhat simple to solve:

- If they come from flags, make sure they're sane and error out early if not, in the front-end
- If they come from user input (attribute/directive/inline asm), then just do whatever the user told you to do
- Everything else should be an error

In the second case above, if we generate what the user asks, and they come to use crying, well, you can easily tell them: it's your own fault.

But if we try to be smart and don't let the user to do what they explicitly requested, they'll be "smarter" and do horrible things like linker script magic or binary live patching.

The errors are hand-coded IR, other front-ends bad lowering, out-of-tree passes, bugs in the middle/back-end. Those we can emit errors for, but only if we're sure there are no attributes involved (can we?).

If we can reliably tell what's in the first and second categories, we should at least be able to create some tests and errors in the right places.

cheers,
--renato



================
Comment at: test/CodeGen/ARM/scavenging.mir:2
+# RUN: llc -o - %s -mtriple=thumb-arm-none-eabi -mcpu=cortex-m0 -run-pass scavenger-test | FileCheck %s
 ---
 # CHECK-LABEL: name: scavengebug0
----------------
I expect "arm" triple to fail here with an error message like "sorry, no ARM support".

If that's what happen now, then we should have a negative test, too.


https://reviews.llvm.org/D35569