[llvm-commits] [llvm] r125595 - in /llvm/trunk: lib/MC/MCParser/AsmParser.cpp test/MC/AsmParser/exprs.s test/MC/AsmParser/paren.s

[Chris Lattner]

On Feb 23, 2011, at 1:55 PM, Joerg Sonnenberger wrote:

> On Wed, Feb 23, 2011 at 01:30:44PM -0800, Jim Grosbach wrote:
>> I'm sorry that we've been unable to resolve this via discussion.
>> Perhaps we simply have differing enough philosophical approaches that
>> we simply weren't ever going to reach consensus.
> 
> Right, I am wrong and you are right. Please reopen the bug that this
> triggers and provide a proper solution, since you don't care about
> consistency or breaking things for other people.

Joerg and others,

It looks like there are a couple of misunderstandings going on here.  I really appreciate the work you've been doing and I agree that the patch to add [] is a good feature for the X86 backend.  Unfortunately, Jim is right that it does break perfectly legal ARM code (and Jim should add a testcase for this code that is getting broken!).

I don't think that this is a matter of "what is right or wrong" or "who" is right.  The general policy we follow in LLVM (documented here: http://llvm.org/docs/DeveloperPolicy.html#quality) is that we don't want to regress currently working stuff.  Jim's coming at this from the perspective that your patch broke something on ARM and therefore introduced a regression.  I think it's lousy that there isn't a regression test for this, but these things do happen.

In any case, instead of focusing on who is right or wrong, it is more productive to focus on how to make progress here.  I agree with Jim that reverting the patch for the time being is the right thing to do.  It would be great if you could resubmit the patch, adding a target hook at the the ARM assembler can override to disable [] parsing.  I don't think that this would be a major extension to the patch and would be happy to help with it.

Just remember, we're all friends here.  :-)

-Chris