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

James Molloy james.molloy at arm.com
Tue Sep 6 23:58:12 PDT 2011


Hi Owen,

 

Given the invasive nature of the patches, I didn't honestly expect them to apply
cleanly to ToT for more than a day or two after the patch was created. I was
going to deal with the merge fallout myself ideally after review, but I'll
rebase to ToT today and send updated patches for you.

 

Thanks for letting me know about the clang driver - I'll ensure that's updated
too.

 

Cheers,

 

James

 

From: resistor at me.com [mailto:resistor at me.com] 
Sent: 06 September 2011 23:28
To: James Molloy
Cc: llvm-commits at cs.uiuc.edu
Subject: Re: [llvm-commits] [PATCH] Fix cortex-m class MSR/MRS and resolve
(safe) ambiguous instruction encodings

 

Thanks James,

 

Patch 0 now applies for me, and looks fine in general, with the caveat that
there's a dependent change in the clang driver
(tools/clang/tools/driver/cc1as.cpp) that is needed as well.  I'm not totally
happy about having to expose the subtarget info to the decoder and printer, but
after looking at the reference manual I don't really see a way around it.

 

Patch 1 has some issues.  It looks like ARMDisassembler.cpp needs to be updated
to pass the subtarget info object to the individual decoder methods.

 

--Owen

On Sep 06, 2011, at 02:29 PM, James Molloy <James.Molloy at arm.com> wrote:

Hi Owen,

Sorry for taking so long getting back to you on this - make check has crashed my
laptop twice now with what appears to be an errant llc process. I'm looking into
that now (doesn't happen when running lit manually...)

Anyway, the patch attached applies to ToT now and has no conflicts. All targets
build correctly and the regression tests pass.

Cheers,

James
________________________________________
From: Owen Anderson [resistor at me.com]
Sent: 06 September 2011 19:43
To: James Molloy
Cc: llvm-commits at cs.uiuc.edu
Subject: Re: [llvm-commits] [PATCH] Fix cortex-m class MSR/MRS and resolve
(safe) ambiguous instruction encodings

James,

Patch 0 seems to be missing some necessary changes to non-ARM targets to make
LLVM build with it applied. I see a lot of errors of the form:

MipsMCTargetDesc.cpp:109:41: error: cannot initialize a parameter of type
'Target::MCInstPrinterCtorTy' (aka 'llvm::MCInstPrinter *(*)(const llvm::Target
&, unsigned int, const llvm::MCAsmInfo &, const llvm::MCSubtargetInfo &)') with
an lvalue of type 'llvm::MCInstPrinter *(const llvm::Target &,
unsigned int, const llvm::MCAsmInfo &)'
createMipsMCInstPrinter);
^~~~~~~~~~~~~~~~~~~~~~~

--Owen

On Sep 2, 2011, at 8:38 AM, James Molloy wrote:

> 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]
>
<patch0-add-sti-to-instprinter.patch><patch1-modify-fixedlendecoderemitter.patch
><patch2-fix-msr-mrs.patch>_______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits




-- IMPORTANT NOTICE: The contents of this email and any attachments are
confidential and may also be privileged. If you are not the intended recipient,
please notify the sender immediately and do not disclose the contents to any
other person, use it for any purpose, or store or copy the information in any
medium. Thank you.

_______________________________________________
llvm-commits mailing list
llvm-commits at cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20110907/c15aea6b/attachment.html>


More information about the llvm-commits mailing list