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

Tim Northover t.p.northover at gmail.com
Fri Mar 28 08:26:13 PDT 2014


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.
+ (ARMAsmBackend.cpp:719): Identifiers beginning with an underscore
and a capital letter are reserved in C++. Using them is undefined
behaviour.
+ "(ARMAsmBackend.cpp:790): Thumble" looks rather like an actual word,
and "Thumbbe" like a 17th century typo. I'd probably capitalise the
BE/LE.

Cheers.

Tim.



More information about the llvm-commits mailing list