[llvm-commits] PATCH: Fix or PR13392 and many other slow-opt-compile bugs in LLVM

[Chandler Carruth]

On Sun, Jul 22, 2012 at 2:35 AM, Duncan Sands <baldrick at free.fr> wrote:

> Hi Chandler,
>
> > Based on a few reports, I've been tracking down some extremely slow
> compiles of
> > small, reasonable code snippets, and it turns out that most of them look
> exactly
> > like PR13392. Not only does creating i1024 and i2048 variables
> everywhere in
> > SROA confuse the daylights out of the ARM codegen, it also makes lots of
> the IR
> > and DAG optimizers slower because it slows down ComputeDemandedBits and
> other
> > APInt operations on these values.
> ...
>
> > --- a/lib/Transforms/Scalar/ScalarReplAggregates.cpp
> > +++ b/lib/Transforms/Scalar/ScalarReplAggregates.cpp
> > @@ -63,15 +63,15 @@ namespace {
> >      SROA(int T, bool hasDT, char &ID, int ST, int AT, int SLT)
> >        : FunctionPass(ID), HasDomTree(hasDT) {
> >        if (T == -1)
> > -        SRThreshold = 128;
> > +        SRThreshold = -1;
> >        else
> >          SRThreshold = T;
> >        if (ST == -1)
> > -        StructMemberThreshold = 32;
> > +        StructMemberThreshold = -1;
> >        else
> >          StructMemberThreshold = ST;
> >        if (AT == -1)
> > -        ArrayElementThreshold = 8;
> > +        ArrayElementThreshold = -1;
> >        else
> >          ArrayElementThreshold = AT;
> >        if (SLT == -1)
>
> I think you should just get rid of the thresholds altogether.  That way,
> if the
> default logic is not always adequate then people will complain and you
> will hear
> about their problematic testcases.  If you leave the thresholds then most
> likely
> people with problems will just use them to work around issues and you will
> never
> know that there are issues.
>

I don't really disagree with you, but I think it should be a separate patch
at least. I have some sympathy to the people who are actively using these
thresholds for something, and would prefer to give them a chance to
complain. =]


>
> > @@ -1867,9 +1866,20 @@ void SROA::isSafeMemAccess(uint64_t Offset,
> uint64_t MemSize,
> >        return;
> >      }
> >    }
> > +
> >    // Check if the offset/size correspond to a component within the
> alloca type.
> >    Type *T = Info.AI->getAllocatedType();
> > -  if (TypeHasComponent(T, Offset, MemSize)) {
> > +  if (Type *EltTy = getComponentType(T, Offset, MemSize)) {
> > +    // If access is untyped (memcpy, etc), and the element being
> accessed is
> > +    // an array, disable SROA to prevent exploding the memory intrinsic
> into
> > +    // element-wise accesses to the array, unless the total size is
> within the
> > +    // scalar load threshold where we can turn it into an integer memory
> > +    // access.
> > +    if (!MemOpType && EltTy->isArrayTy() &&
>
> Is this really specific to arrays?  Couldn't you get the same thing for
> structs?
>

We can perform actual loads and stores of structs as a single load or store
instruction, but we can't do that for arrays.

Perhaps we should have support for array types in the IR, and use those
instead... That's a discussion for another time and/or place though...


>
> > +        ((MemSize * 8) > ScalarLoadThreshold ||
> > +         !TD->fitsInLegalInteger(MemSize * 8)))
> > +      return MarkUnsafe(Info, TheAccess);
> > +
> >      Info.hasSubelementAccess = true;
> >      return;
> >    }
>
> Ciao, Duncan.
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120722/9268070c/attachment.html>