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

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri marked 3 inline comments as done.
lebedev.ri 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());
+
----------------
craig.topper wrote:
> 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()?
Yup: https://github.com/llvm/llvm-project/blob/62fc5f16405a7d39e62044bc461752f3f31bdca0/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp#L8007-L8010


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140638