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

[Sam Parker via Phabricator via llvm-commits]

samparker added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/TargetLowering.h:1177
+    unsigned Ty = (unsigned)VT.SimpleTy;
+    return (LegalizeAction)((IndexedModeActions[Ty][IdxMode] & 0xf000) >> 12);
+  }
----------------
dmgreen wrote:
> SjoerdMeijer wrote:
> > dmgreen wrote:
> > > samparker wrote:
> > > > Is there a way that we can avoid these magic hex values?
> > > I'm not sure. We are trying to pull 4 bits out of an 16bit value, so the hex seems to fit perfectly!
> > Agreed, but how about we do that inside a few simple getter/setter helper functions? I guess that would improve readability here a bit.
> I think this is the getter helper function? I would prefer to keep it consistent with everything else in this file, which seem to just use shifts and Ands.
> 
> I could change them to `(LegalizeAction)((IndexedModeActions[Ty][IdxMode] >> 12) & 0xf);` if that would be better?
I don't mind adds and shifts, but what is twelve and why four bits? Can't we at least some enums for readability?


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

https://reviews.llvm.org/D70176