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

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri marked an inline comment as done.
lebedev.ri added a comment.

In D140638#4024588 <https://reviews.llvm.org/D140638#4024588>, @efriedma wrote:

> I won't insist on implementing everything in one patch.  I was going to say we need a bailout for targets that don't have unaligned loads... but looking at llvm/test/CodeGen/RISCV/shifts.ll, I guess the previous codegen is terrible enough that it's an improvement even if we expand unaligned loads to byte loads.  So I guess that's fine.

That was precisely my point. It's already not great, and this does not seem to regress it.



================
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:
> 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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140638