AARCH64_BE load/store rules fix to conform to ARM ABI

Albrecht Kadlec akadlec at a-bix.com
Tue Feb 25 14:00:04 PST 2014


On 2014-02-25 22:28, Adrian Prantl wrote:
> On Feb 25, 2014, at 9:10, Renato Golin <renato.golin at linaro.org> wrote:
> 
>> On 25 February 2014 16:59, Albrecht Kadlec <akadlec at a-bix.com> wrote:
>>> there're a few misunderstandings:
>> 
>> I apologise.
>> 
>> 
>>> First, I'm not Christian (and he had a reviewer explicitly asking for 
>>> those
>>> _related_ indentations, which brings up the question about global 
>>> policy.)
>> 
>> This is a global policy, not sure where it is, though, or why people
>> accepted white space changes alongside functionality changes.

s/accepted/demanded/

> 
> It’s over here:
> 
> http://llvm.org/docs/CodingStandards.html#golden-rule

thanks - that's what we knew from hearsay & expected anyway: "follow the 
coding style around you".

>> Our long term goal is for the entire codebase to follow the 
>> convention, but we explicitly do not want patches that do large-scale 
>> reformating of existing code. On the other hand, it is reasonable to 
>> rename the methods of a class if you’re about to change it in some 
>> other way. Just do the reformating as a separate commit from the 
>> functionality change.
>> 
> 
> Submitting two patches, one that does the necessary cleanups, and one
> that has the actual changes should be fine.


hmm, there are no cleanups in otherwise untouched code in this patch.

"let predicates = [IsLE] in {" was needed to disable a set of matcher 
rules harmful to BigEndian code.
That's the target-description equivalent of an if-statement in C and the 
body should be indented for readability - as was done in the rest of the 
file.
How would separating the indentation from the code changes improve the 
readability of the to-be-reviewed code ?
Normal diff highlights the unchanged rules, diff -w keeps the focus on 
the added conditions and shows that the rules are indeed unchanged.

Maybe I'm using the wrong tools - will look at Phabricator (I want my 
Critique back!)

> -- adrian




More information about the llvm-commits mailing list