[PATCH] D43005: [ARM] Error out on .arm assembler directives on windows

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

Apologies to take so long to get back to you. I've had a chance to take a look. While I've not got a magic solution I do have some questions/suggestions.

> We already set the NoARM variable in ARMSubtarget but we don't clear the FeatureNoARM feature bit. Clearing it from there doesn't seem to be enough when assembling .s files from clang though.

If I set the noarm feature string directly for example --triple=armv7-none-eabi -mattr=+noarm then I get the desired behaviour on your test case. It might be that setting the feature bit directly is insufficient as it may get recalculated later from the triple and mattr, but if you can add +noarm to the mattr feature string this should keep the noarm feature set.
The NoARM and corresponding hasARMOps() in its current form looks like it could and probably should be removed.

- NoARM is only set when isTargetWindows(), so hasARMOps() is effectively !isTargetWindows(), to me this makes the name hasARMOps() misleading. If it is truly hasARMOps() then it should be calculated from the feature bits.
- hasARMOps() is only used in a few places and could be easily replaced by !isTargetWindows() if that is the intention.

To summarise:

- It should be possible to accomplish what you want by using just the featurebits. Can you elaborate on why this doesn't work for you?
- I suggest renaming or removing hasARMOps(), have I misunderstood what it is used for?


Repository:
  rL LLVM

https://reviews.llvm.org/D43005