[llvm] r179957 - SimplifyCFG: If convert single conditional stores

[Arnold Schwaighofer]

On Apr 20, 2013, at 5:44 PM, Arnold <aschwaighofer at apple.com> wrote:

> Hi Chandler,
> 
> I totally agree with you that if we can model the effects (issue slots) and predict the performance we should do it in machine if-conversation. 

I meant Early-If conversion.

> In this case - I am not sure we can.
> 
> Assume we are in a loop and assume we have two issue slots for memory ops- but it really does not matter how many we choose - we going to end up deciding not to do it -  because we are going to take up two cycles of the two load/store unit instead of one([first store/may alias load][second speculated store]). But you may want to still do it - heuristically speaking because of the cost of the mid predict so I am not sure a "better" decision could be made. (On your typical architecture)

I stand corrected on this paragraph. I was assuming that we don't accept any extension of the critical path. The code in Early-If conversion actually allows this (I should better listen to Jakob when he explains the pass in meetings - I only remembered it being conservative):

  // Set a somewhat arbitrary limit on the critical path extension we accept.
  unsigned CritLimit = SchedModel->MispredictPenalty/2;

Given this we actually always would if convert a store (if we implement it in early-ifconv) on any sane architecture that we support (i.e. this is a "should if-convert store or not question" which we could also predicate on TTI). So we might as well do it early in SimplifyCFG given the additional benefits?


Best,
Arnold

> Given that there are potential positive secondary effects of having longer basic blocks I believe the overall effect to be positive.
> 
> Also I believe in the short term the machine if conversation pass is not in the shape to be turned on and this opt temporarily improves things until it can (possibly) be superceeded by machine if conversation. 
> 
> I just don't want to look bad on phoronix if only temporary.  ;)
> 
> 
> Best,
> Arnold
> (Having said all that I will happily revert the patch if folks disagree I would have just liked to see it go through some bots)
> 
> Sent from my iPhone
> 
>  On Apr 20, 2013, at 4:56 PM, Chandler Carruth <chandlerc at google.com> wrote:
> 
>> On Sat, Apr 20, 2013 at 10:42 PM, Arnold Schwaighofer <aschwaighofer at apple.com> wrote:
>> SimplifyCFG: If convert single conditional stores
>> 
>> Last time I touched if-conversion in simplifycfg, Evan, Jakob, and Andy had serious objections to it, and insisted that it should instead be done by enabling a (sufficiently conservative) version of the early if conversion in the backend.
>> 
>> I'm actually sympathetic to the desire to do this (i hit similar performance problems as the one you're seeing, specifically in zlib in my case), but I really see their point. Either way, it seems quite strange to reverse direction without some commentary about previous attempts, alternatives, and what the plan is for getting the x86 backend to handle if-conversion in a sane way[1].
>> 
>> 
>> [1]: And unless there is any confusion, I think we all agreed that the sane form of if-conversion on x86 is *insanely* conservative due to the relative efficiency of predicted branches and cost of speculation. It's conversions much like the one you're targeting where the store itself is the only speculation and it is already in cache are the best kinds.