[PATCH] D42088: [x86] shrink 'and' immediate values by setting the high bits (PR35907)

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: lib/Target/X86/X86ISelDAGToDAG.cpp:2471
+
+  SDLoc DL(And);
+  SDValue NewMask = CurDAG->getConstant(NegVal, DL, VT);
----------------
spatel wrote:
> craig.topper wrote:
> > spatel wrote:
> > > craig.topper wrote:
> > > > One of these days we should really decide between 'DL' and 'dl'...
> > > I thought 'dl' was a holdover from the days when variables were lower-case? I'd be happy to reformat the whole file. :)
> > It feels like everytime I touch DAGCombiner or X86ISelLowering I have to figure out which name is already used in the function I'm editing.
> > 
> > And to really make it confusing, there are a couple places in X86ISelLowering that have DataLayout in DL and an SDLoc in dl.
> I didn't make the connection to DataLayout. What's also weird is that we name these 'DL' at all...because an SDLoc isn't a DebugLoc...it contains a DL. No respect for the other member of the class!
> 
> To skirt controversy, I just removed the local variable here. :)
You said you were happy to reformat the whole file to 'DL' but there are places in X86ISelLowering where the same function simultaneously uses the variable name dl for an SDLoc and DL for a DataLayout reference. So a blind rename would fail to build. Anyway this isn't relevant to this review.


https://reviews.llvm.org/D42088