AARCH64_BE load/store rules fix to conform to ARM ABI

Renato Golin renato.golin at linaro.org
Tue Feb 25 14:15:18 PST 2014


On 25 February 2014 22:00, Albrecht Kadlec <akadlec at a-bix.com> wrote:
> s/accepted/demanded/

Right. It doesn't matter now, it's not a big deal, I was clearly overreacting.


> How would separating the indentation from the code changes improve the
> readability of the to-be-reviewed code ?

In this case, you don't. What put me off was the fact that the
Christian's commit was different than the reviewed one (I admit, I was
following it rather loosely), and that you sent a "cleaned up" patch
alongside the real one.


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

Phabricator / GitHub / Meld and other visual diffs are excellent tools
to ignore clutter in patches. They work out the differences in a
better way and can easily ignore silly stuff, so that you don't have
to do that yourself. Plus, we're kinda used to seeing tons of similar
changes because of 80-column violation changes, etc.

Sorry about the noise. I think I have muddled your patch review
enough. Would be great if you could start over on Phab.

cheers,
--renato



More information about the llvm-commits mailing list