[PATCH] D76551: [Alignment][NFC] Use TargetFrameLowering::getStackAlign()

[Guillaume Chatelet via Phabricator via llvm-commits]

gchatelet marked 9 inline comments as done.
gchatelet added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:4031
-      DAG.getSubtarget().getFrameLowering()->getStackAlignment();
-  if (Align <= StackAlign)
-    Align = 0;
----------------
courbet wrote:
> I think it was clearer with this conditional here rather than inline later.
Unfortunately if I do so I have to use a `MaybeAlign` instead of `Align` to express the `0` case.
Is this better?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:4037
+      DAG.getNode(ISD::ADD, dl, AllocSize.getValueType(), AllocSize,
+                  DAG.getConstant(StackAlign.value() - 1, dl, IntPtr), Flags);
 
----------------
courbet wrote:
> extract `StackAlign.value() - 1` to a `StackAlignMask` variable for semantics ? Or maybe even add a `getMask()` method to `llvm::Align` given that this is a very common operation ? 
I plan to do so as a separate step, maybe introducing a Mask type. There are less cases than it seems though so I'm still weighing the costs / benefits.


================
Comment at: llvm/lib/Target/PowerPC/PPCRegisterInfo.cpp:548
       BuildMI(MBB, II, dl, TII.get(PPC::LI8), NegSizeReg)
-        .addImm(~(MaxAlign-1));
+          .addImm(~(MaxAlign.value() - 1));
 
----------------
courbet wrote:
> would be useful here too.
Acknowledged


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76551