[PATCH] D95586: [ARM] permit PC as destination of MOVS

Sat Jan 30 04:39:37 PST 2021

rengolin added a comment.

In D95586#2531012 <https://reviews.llvm.org/D95586#2531012>, @nickdesaulniers wrote:

> Then why does llvm/test/MC/Disassembler/ARM/unpredictable-LSL-regform.txt test `LSL`, and not `ARS`, `LSR`, or `ROR`?  (There are other tests llvm/test/MC/Disassembler/ARM/unpredictable-, but there don't seem to be tests for those).

Who knows why! :)

But we shouldn't rely on tests to tell us what the architecture does.

> Ah, looks like out of `movs, lsl, lsr, asr, ror, and rrx`, the kernel is only using `movs` with `pc` destination, in ARM mode (non-Thumb).  I can restrict the patch to `movs`, though the test case in llvm/test/MC/Disassembler/ARM/unpredictable-LSL-regform.txt would still fail.

I figured as much. Hand-written ARM assembly is usually in ARM mode, especially kernel stuff. So much so that GNU inline assembly assumes ARM mode.

> And it still begs the question, why is it ok for the immediate operand versions of these to permit `pc` destination?

That's a hardware design decision, documented in the manual, which compilers have to follow, so we need to implement it as is.

> I guess I don't really follow whether the recommendation is to do something else in LLVM, or remove the kernel code?

The recommendation will always be to "do the right thing in LLVM and change user code to match". We have fixed a staggering amount of bugs and broken code in the kernel and chromium with that attitude.

However, in this case in particular, it seems the kernel code isn't wrong (ie. it uses the allowed exceptions in MOVs only), so just adding the exception for MOVs would probably fix the kernel problem without making LLVM generate code that is unpredictable.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95586/new/

https://reviews.llvm.org/D95586