[llvm] r194253 - [ARM] In ARMAsmParser, MatchCoprocessorOperandName() permitted p10 and p11 as operands for coprocessor instructions, resulting in encodings that clash with FP/NEON instruction encodings

James Molloy james at jamesmolloy.co.uk
Mon Dec 2 07:41:58 PST 2013


Hi Joerg,

> It breaks compatibility with existing code.

But if the existing code you're referring to is incorrect, I don't see the
issue. Incorrect code is incorrect code, and we don't keep compiler bugs
around to satisfy legacy programs. Personally I don't see this as different
to any other compiler bug.

>  I still haven't heard an argument for why it is needed.

The toolchain is not in compliance with the reference manuals for post-v6
targets, AIUI.

Basically I agree with Tim and Artyom - sorry! Do you still want to push
for reversion of this patch?

Cheers,

James


On 2 December 2013 00:06, Joerg Sonnenberger <joerg at britannica.bec.de>wrote:

> On Sun, Dec 01, 2013 at 10:39:48AM +0000, Tim Northover wrote:
> > > This commit breaks valid assembler sources that use MCR/MRC explicitly
> > > in place of the (newer) NP/NEON forms.
> >
> > I believe those sources are (strictly) invalid on v6 and above. All
> > coprocessor instructions begin with "if coproc in '101x' then SEE
> > 'floating-point instructions", terminology that's used elsewhere for
> > cases which don't permit overlap.
> >
> > > I don't see the point of this
> > > change and therefore would like to see it reverted on trunk and 3.4.
> >
> > From Artyom's point of view, it's probably making sure LLVM adheres to
> > the reference manuals. For the writers of the reference manuals, I
> > suspect the problem is running out of encoding space and having to use
> > (say) MRC-type encodings for instructions that have absolutely nothing
> > MRC-like about them -- better to completely forbid the misleading
> > forms than leave confusion.
> >
> > Personally, I don't see the argument for keeping this as an alias on
> > post-v5 architectures. I can't think of a use that wouldn't be better
> > served by writing it in terms of the real FP instructions.
>
> It breaks compatibility with existing code. As I said, I still haven't
> heard an argument for why it is needed. Deprecate them and don't add new
> aliases -- fine. But removing something that has been accepted for a
> long time?
>
> Joerg
> _______________________________________________
> 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/20131202/cf0c11f2/attachment.html>


More information about the llvm-commits mailing list