Fix SROA for intrinsics

[Mandeep Singh Grang]

Hi,

 

Can the owner for SROA please take a look into the attached patch?

 

Also if you are fine with the patch could you please commit it since I do not have commit rights?

 

Thanks,

Mandeep

From: Owen Anderson [mailto:resistor at mac.com] 
Sent: Wednesday, January 21, 2015 8:18 PM
To: Mandeep Singh Grang
Cc: llvm-commits; apazos at codeaurora.org <mailto:apazos at codeaurora.org> 
Subject: Re: Question about commit d4748bbd497b550a4e5db246c6708fcd6de542da

 

Ok, patches welcome. :-)

 

—Owen

 

On Jan 21, 2015, at 8:00 PM, Mandeep Singh Grang < <mailto:mgrang at codeaurora.org> mgrang at codeaurora.org> wrote:

 

Yes, in the scenario where we have an intrinsic and the following condition is true:

else if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(U->getUser()))

 

But the following condition in false:

if (II->getIntrinsicID() != Intrinsic::lifetime_start &

    II->getIntrinsicID() != Intrinsic::lifetime_end)

 

In this case, all other subsequent else-if’s are skipped and the logic falls through to the “return true” and we see a lot of “ins” instructions being generated.

But I think it should evaluate other else if’s since some of them may meet the condition and we may return false.

 

We saw degradation in AArch64 cortex-a57 code due to this.

 

We modified the code on our end to the following:

 

IntrinsicInst *II = dyn_cast<IntrinsicInst>(U->getUser());

 

<start of if else clauses>

…

else if (II && II->getIntrinsicID() != Intrinsic::lifetime_start &

           II->getIntrinsicID() != Intrinsic::lifetime_end)

 

<other else if’s>

 

Making this change get us back the lost performance.

 

 

Thanks,

Mandeep

From: Owen Anderson [ <mailto:resistor at mac.com> mailto:resistor at mac.com] 
Sent: Wednesday, January 21, 2015 6:30 PM
To: Mandeep Singh Grang
Cc: llvm-commits;  <mailto:apazos at codeaurora.org> apazos at codeaurora.org
Subject: Re: Question about commit d4748bbd497b550a4e5db246c6708fcd6de542da

 

Yes, it is intentional. Do you have a counter example where it results in undesirable behavior?

 

-Owen


On Jan 21, 2015, at 6:04 PM, Mandeep Singh Grang < <mailto:mgrang at codeaurora.org> mgrang at codeaurora.org> wrote:

Hi Owen,

 

In the following commit, we observed that with your else-if condition all the subsequent else-if’s are skipped.

Could you please confirm whether this was your intent?

 

commit d4748bbd497b550a4e5db246c6708fcd6de542da

Author: Owen Anderson < <mailto:resistor at mac.com> resistor at mac.com>

Date:   Thu Aug 7 21:07:35 2014 +0000

 

    Fix a case in SROA where lifetime intrinsics could inhibit alloca promotion.  In

    this case, the code path dealing with vector promotion was missing the explicit

    checks for lifetime intrinsics that were present on the corresponding integer

    promotion path.

 

+++ b/lib/Transforms/Scalar/SROA.cpp

@@ -1659,6 +1659,10 @@ static bool isVectorPromotionViableForSlice(

       return false;

     if (!I->isSplittable())

       return false; // Skip any unsplittable intrinsics.

+  } else if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(U->getUser())) {

+    if (II->getIntrinsicID() != Intrinsic::lifetime_start &&

+        II->getIntrinsicID() != Intrinsic::lifetime_end)

+      return false;

   } else if (U->get()->getType()->getPointerElementType()->isStructTy()) {

     // Disable vector promotion when there are loads or stores of an FCA.

     return false;

 

 

Thanks,

Mandeep

 

<Fix_SROA_for_intrinsics.patch>

 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150125/113e5d30/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Fix_SROA_for_intrinsics.patch
Type: application/octet-stream
Size: 1117 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150125/113e5d30/attachment.obj>