[llvm-commits] [PATCH] Fix cortex-m class MSR/MRS and resolve (safe) ambiguous instruction encodings

James Molloy james.molloy at arm.com
Fri Sep 2 08:38:21 PDT 2011


Hi,

The attached three patches fix Kurt Lidl's problem of MRS/MSR on Cortex-M series
not allowing the correct mask names.

The patch becomes difficult because:
  * There is no way to determine in the InstPrinter any subtarget specific
features (such as "operating on an m-class core?").
  * There is currently no subtarget feature for "M-class core?"; the nearest is
IsThumb2 && !HasARM.
  * The encoding for M-class (v{6,7}m) MSR/MRS differs from v{6,7}ar, but not in
a way that makes them deterministically separable. This causes non-conflicting
ambiguity between the M and AR class insns in the FixedLenDecoderEmitter where
both code paths are emitted but only one will ever be hit.

The solution is:
  * Add subtarget info to InstPrinter so it can determine what to do with the
mask immediate.
  * Add a new SubtargetFeature "IsMClass" which is true on v{6,7}m. The inverse
IsARClass == !IsMClass. This feature only has semantic sense on v6+
architectures.
  * The conflicts occur because the FixedLenDecoderEmitter does not honour the
Predicates field of instructions in the tablegen description. If the island
checking also checked the AssemblerPredicates field (if defined), ambiguous
instruction encodings that are disambiguated by predicates would (do) work fine.

The first patch (#0) adds a MCSubtargetInfo to MCInstPrinter and MCDisassembler.
They can use this (in patch #2) to determine what mask names to accept (primask?
or cpsr_zxvf?)

The second enhances FixedLenPredicateEncoder to emit predicate checks for
instructions before it accepts an encoding - this allows ambiguous instructions
to be disambiguated by the Predicates field in TableGen. This required changing
llvm-mc to accept -mattr, and tests to set the correct features they rely upon.

**As part of this, a bug in the MC was found in that ARM-mode STC2's were being
generated in Thumb2 mode (not the T2 encoding) and the test was checking for
this. The test has been disabled for the moment until a patch to add T2 STC/STC2
is created.**

The third patch adds a new predicate "IsMClass" along with its counterpart
"IsARClass = !IsMClass". The T2 MRS instruction is duplicated; one
Requires<[IsMClass]> the other Requires<[IsARClass]> with their differing
encodings respectively. It also fixes mask printing for MRS/MSR in the
ARMMCInstPrinter and ARMMCAsmParser.

Testcases added with the final patch.

Comments? Is it OK?

Cheers,

James

[N.B.: Patch was against ToT~3 days, so I'll deal with any merge fallout myself
after review]
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch0-add-sti-to-instprinter.patch
Type: application/octet-stream
Size: 24579 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20110902/872fddd5/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch1-modify-fixedlendecoderemitter.patch
Type: application/octet-stream
Size: 20782 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20110902/872fddd5/attachment-0001.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch2-fix-msr-mrs.patch
Type: application/octet-stream
Size: 13451 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20110902/872fddd5/attachment-0002.obj>


More information about the llvm-commits mailing list