[llvm] r186425 - When the inliner merges allocas, it must keep the larger alignment

[Hal Finkel]

----- Original Message -----
> Hi Hal,
> 
>  > When the inliner merges allocas, it must keep the larger alignment
> >
> > For safety, the inliner cannot decrease the allignment on an alloca
> > when
> > merging it with another.
> >
> > I've included two variants of the test case for this: one with
> > DataLayout
> > available, and one without. When DataLayout is not available, if
> > only one of
> > the allocas uses the default alignment (getAlignment() == 0), then
> > they cannot
> > be safely merged.
> 
> > --- llvm/trunk/lib/Transforms/IPO/Inliner.cpp (original)
> > +++ llvm/trunk/lib/Transforms/IPO/Inliner.cpp Tue Jul 16 12:10:55
> > 2013
> > @@ -189,6 +190,14 @@ static bool InlineCallIfPossible(CallSit
> >       bool MergedAwayAlloca = false;
> >       for (unsigned i = 0, e = AllocasForType.size(); i != e; ++i)
> >       {
> >         AllocaInst *AvailableAlloca = AllocasForType[i];
> > +
> > +      unsigned Align1 = AI->getAlignment(),
> > +               Align2 = AvailableAlloca->getAlignment();
> > +      // If we don't have data layout information, and only one
> > alloca is using
> > +      // the target default, then we can't safely merge them
> > because we can't
> > +      // pick the greater alignment.
> > +      if (!TD && (!Align1 || !Align2) && Align1 != Align2)
> > +        continue;
> >
> >         // The available alloca has to be in the right function,
> >         not in some other
> >         // function in this SCC.
> > @@ -206,6 +215,11 @@ static bool InlineCallIfPossible(CallSit
> >                      << *AvailableAlloca << '\n');
> >
> >         AI->replaceAllUsesWith(AvailableAlloca);
> > +
> > +      if (Align1 > Align2 || (!Align1 && TD &&
> > +          TD->getABITypeAlignment(AI->getAllocatedType()) >
> > Align2))
> > +        AvailableAlloca->setAlignment(Align1);
> 
> If Align1 is 1 and Align2 is zero then you set the alignment to
> Align1 but
> Align2 might represent a larger alignment.  How about something like
> this:
> 
>    if (Align1 != Align2) {
>       if (!Align1 || !Align2) {
>         assert(TD && "Should have bailed out earlier!");
>         unsigned TypeAlign =
>         TD->getABITypeAlignment(AI->getAllocatedType());
>         if (!Align1)
>           Align1 = TypeAlign;
>         if (!Align2)
>           Align2 = TypeAlign;
>       }
> 
>       if (Align1 > Align2)
>         AvailableAlloca->setAlignment(Align1);
>    }

r186510; thanks! I modified this slightly so that we'd preserve the 'Alignment == 0' if that's larger; otherwise I just used this suggestion (and wrote a test case).

 -Hal

> 
> Ciao, Duncan.
> 
> > +
> >         AI->eraseFromParent();
> >         MergedAwayAlloca = true;
> >         ++NumMergedAllocas;
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory