[PATCH] D77292: [Alignment][NFC] Convert MachineIRBuilder::buildDynStackAlloc to Align

Fri Apr 3 01:34:54 PDT 2020

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

================
Comment at: llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp:1895

-  MF->getFrameInfo().CreateVariableSizedObject(Align ? Align : 1, &AI);
+  MF->getFrameInfo().CreateVariableSizedObject(Alignment, &AI);
   assert(MF->getFrameInfo().hasVarSizedObjects());
----------------
aemerson wrote:
> gchatelet wrote:
> > gchatelet wrote:
> > > courbet wrote:
> > > > I'd like to get a knowledgeable pair of eye to look at this. We used to align anything smaller than the stack size to 1, now we align to max(AI.align, preftypealign). This sounds OK, but let's be on the safe side.
> > > @aemerson what do you think?
> > AFAICT it is not clear what it means use G_DYN_STACKALLOC with an alignment of `0`
> > https://llvm.org/docs/GlobalISel/GenericOpcode.html#g-dyn-stackalloc
> > 
> > @aemerson can you help me understand what's going on here? Or add relevant people to the thread?
> An alignment of 0 I believe is used to mean there is no alignment requirement at all.
Ok I dug into it and `0` or `1` have the same meaning.
https://reviews.llvm.org/D77372

For code simplicity, I'll use `1` here.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77292