Fix SROA for intrinsics

[Chandler Carruth]

This looks generally like a good improvement, but it has no test cases or a
clear explanation of the problem it is fixing. It needs those before it can
go into the tree.

On Sun, Jan 25, 2015 at 6:48 PM, Mandeep Singh Grang <mgrang at codeaurora.org>
wrote:

> 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 <resistor at mac.com>]
> *Sent:* Wednesday, January 21, 2015 8:18 PM
> *To:* Mandeep Singh Grang
> *Cc:* llvm-commits; 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 <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 <resistor at mac.com>]
> *Sent:* Wednesday, January 21, 2015 6:30 PM
> *To:* Mandeep Singh Grang
> *Cc:* llvm-commits; 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 <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 <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>
>
>
>
> _______________________________________________
> 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/20150125/77fb4fa4/attachment.html>