On Sun, Jul 22, 2012 at 2:35 AM, Duncan Sands <span dir="ltr"><<a href="mailto:baldrick@free.fr" target="_blank" class="cremed">baldrick@free.fr</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Chandler,<br>
<div class="im"><br>
> Based on a few reports, I've been tracking down some extremely slow compiles of<br>
> small, reasonable code snippets, and it turns out that most of them look exactly<br>
> like PR13392. Not only does creating i1024 and i2048 variables everywhere in<br>
> SROA confuse the daylights out of the ARM codegen, it also makes lots of the IR<br>
> and DAG optimizers slower because it slows down ComputeDemandedBits and other<br>
> APInt operations on these values.<br>
</div>...<br>
<br>
> --- a/lib/Transforms/Scalar/ScalarReplAggregates.cpp<br>
> +++ b/lib/Transforms/Scalar/ScalarReplAggregates.cpp<br>
> @@ -63,15 +63,15 @@ namespace {<br>
>      SROA(int T, bool hasDT, char &ID, int ST, int AT, int SLT)<br>
>        : FunctionPass(ID), HasDomTree(hasDT) {<br>
>        if (T == -1)<br>
> -        SRThreshold = 128;<br>
> +        SRThreshold = -1;<br>
>        else<br>
>          SRThreshold = T;<br>
>        if (ST == -1)<br>
> -        StructMemberThreshold = 32;<br>
> +        StructMemberThreshold = -1;<br>
>        else<br>
>          StructMemberThreshold = ST;<br>
>        if (AT == -1)<br>
> -        ArrayElementThreshold = 8;<br>
> +        ArrayElementThreshold = -1;<br>
>        else<br>
>          ArrayElementThreshold = AT;<br>
>        if (SLT == -1)<br>
<br>
I think you should just get rid of the thresholds altogether.  That way, if the<br>
default logic is not always adequate then people will complain and you will hear<br>
about their problematic testcases.  If you leave the thresholds then most likely<br>
people with problems will just use them to work around issues and you will never<br>
know that there are issues.<br></blockquote><div><br></div><div>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. =]</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> @@ -1867,9 +1866,20 @@ void SROA::isSafeMemAccess(uint64_t Offset, uint64_t MemSize,<br>
>        return;<br>
>      }<br>
>    }<br>
> +<br>
>    // Check if the offset/size correspond to a component within the alloca type.<br>
>    Type *T = Info.AI->getAllocatedType();<br>
> -  if (TypeHasComponent(T, Offset, MemSize)) {<br>
> +  if (Type *EltTy = getComponentType(T, Offset, MemSize)) {<br>
> +    // If access is untyped (memcpy, etc), and the element being accessed is<br>
> +    // an array, disable SROA to prevent exploding the memory intrinsic into<br>
> +    // element-wise accesses to the array, unless the total size is within the<br>
> +    // scalar load threshold where we can turn it into an integer memory<br>
> +    // access.<br>
> +    if (!MemOpType && EltTy->isArrayTy() &&<br>
<br>
Is this really specific to arrays?  Couldn't you get the same thing for structs?<br></blockquote><div><br></div><div>We can perform actual loads and stores of structs as a single load or store instruction, but we can't do that for arrays.</div>
<div><br></div><div>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...</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<br>
> +        ((MemSize * 8) > ScalarLoadThreshold ||<br>
> +         !TD->fitsInLegalInteger(MemSize * 8)))<br>
> +      return MarkUnsafe(Info, TheAccess);<br>
> +<br>
>      Info.hasSubelementAccess = true;<br>
>      return;<br>
>    }<br>
<br>
Ciao, Duncan.<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" class="cremed">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank" class="cremed">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>