[PATCH] Add ARM big endian Target (armeb, thumbeb)

[Christian Pirker]

Hi Tim,

I am sorry that I committed so fast before I got a definitive OK.

Please see my comments without reply below.
What do you think, should I start a new patch or revert the whole commit?

Thanks,
Christian

On 03/28/14 16:26, Tim Northover wrote:
> Hi Christian,
>
>>    I committed this patch as r205007.
> Please don't do that. As far as I can see no-one has said they look
> good to commit. The general policy is that if you thought a patch was
> worth reviewing, that's not going to change over time so you should
> wait until someone says it's OK.
>
> And in fact you've not addressed my comments (even by telling me why
> you're right and I'm wrong). They're on the Phabricator issue, but:
>
> + (ARMTargetMachine.h:170): Are these extra classes needed for some
> weird magic hidden elsewhere? Because they look fairly cluttered for
> the sake of one parameter in a constructor.
This classes are used by the RegisterTargetMachine.
Therefore I needed them for little and big endian.
The base class (ThumbTargetMachine) has the additional parameter (isLittle).
It is the same as for ARMTargetMachine.
> + (ARMAsmBackend.cpp:719): Identifiers beginning with an underscore
> and a capital letter are reserved in C++. Using them is undefined
> behaviour.
You are right, I should use it without underscore.
But I used it similar to the _OSABI parameter.
> + "(ARMAsmBackend.cpp:790): Thumble" looks rather like an actual word,
> and "Thumbbe" like a 17th century typo. I'd probably capitalise the
> BE/LE.
You are right, I should capitalise the BE/LE.
I think it should be changed also for ARMbe/ARMle, but it will be a long 
sequence of uppercase characters.
> Cheers.
>
> Tim.