[PATCH] D70176: [Codegen][ARM] Add addressing modes from masked loads and stores
Sam Parker via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Nov 15 01:54:57 PST 2019
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
More information about the llvm-commits
mailing list