[PATCH] D76551: [Alignment][NFC] Use TargetFrameLowering::getStackAlign()
Guillaume Chatelet via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Mar 23 03:15:45 PDT 2020
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
More information about the llvm-commits
mailing list