[PATCH] D70176: [Codegen][ARM] Add addressing modes from masked loads and stores

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

Alright, Awesome.

I had written something similar, but different in what parts were altered. I didn't like it in the end though, I felt like it ended up being harder to read and maintain because you just end up spreading the information that you need all over the file. I may not be the best judge of such things though, I often favour a declarative style over one that is more elegant, and am aware that that opinion isn't always popular. It's worth remembering that the aim is for simplicity and maintainability over something that is prettier (although they are often related).

But if you are having trouble reading this in its current form, I will try and make some modifications to it. Watch this space.

I will say that there is lots of other code in this patch that could do with extra eyes though! You've already found one issue, and it's worth making sure we are not focusing on the wrong thing, missing the forest for the trees. There's lots of other code in here that could do with a bit of extra scrutiny.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70176/new/

https://reviews.llvm.org/D70176