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

[Duncan Sands]

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.

> @@ -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?

> +        ((MemSize * 8) > ScalarLoadThreshold ||
> +         !TD->fitsInLegalInteger(MemSize * 8)))
> +      return MarkUnsafe(Info, TheAccess);
> +
>      Info.hasSubelementAccess = true;
>      return;
>    }

Ciao, Duncan.