[PATCH] D140638: [Codegen][LegalizeIntegerTypes] New legalization strategy for scalar shifts: shift through stack

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:4160
+  // And spill it into the stack slot.
+  Ch = DAG.getStore(Ch, dl, Init, StackPtr, MachinePointerInfo());
+
----------------
efriedma wrote:
> lebedev.ri wrote:
> > efriedma wrote:
> > > MachinePointerInfo::getUnknownStack instead of MachinePointerInfo()?  Or actually, DAGTypeLegalizer::SplitVecRes_INSERT_SUBVECTOR does the following:
> > > 
> > > ```
> > >   auto FrameIndex = cast<FrameIndexSDNode>(StackPtr.getNode())->getIndex();
> > >   auto PtrInfo = MachinePointerInfo::getFixedStack(MF, FrameIndex);
> > > ```
> > As per @craig.topper, and looking at `-debug`,
> > we get right `MachinePointerInfo` for stack-based
> > memory operations automatically. Not doing that manually
> > frees us from having to track alignment, too.
> I guess optimizations kick in, sure, so you don't need to compute the exact pointer info.
> 
> But please at least use getUnknownStack(); a default-constructed MachinePointerInfo implicitly points to address-space zero.
Isn't there a check for null pointer info in getLoad/getStore to infer the pointer info? Will that still trigger with getUnknownStack()?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140638