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

[Arnold Schwaighofer]

Let me expand on this a little by claiming that there can be two scopes relevant to the decision of determining whether a select should be formed:

* a purely local one (like i would argue - in my case) - essentially a boolean value
* a trace based one - where you want to take an as precise trace of instructions into account (the trace matters to the decision).

I am justifying my patch by saying we can and should do select formation for cases of the first scope as early as possible. For cases of the latter scope we will do it in early-if conversion.

Best,
Arnold

On Apr 20, 2013, at 8:47 PM, Arnold Schwaighofer <aschwaighofer at apple.com> wrote:

> I don't push for general if conversation but just if-conversion of single stores. This is always going to be deemed (or not - but always the same answer) beneficial by the existing early if conversion pass (see my previous email):
> 
> Basically we say if-convert a store: if critical-path + cost of store < CONST
> 
> (Jakob correct me if I am wrong)
> 
> This is a simple boolean value (obtainable via TTI if need to be).
> 
> So even if the early-if conversion pass implemented conversion of stores (which it currently does not - but it could be made to do so) and were enabled for x86 the decision would be the same. Lest the benefit of doing it earlier, and we do some select formation (I don't think we want to totally get rid of it - I might be wrong of course).
> 
> There is also the argument about canonicalization. Maybe we only want to do it in a target lowering phase run of SimplifyCFG - but we currently don't have this distinction. 
> 
> I do think in our existing pass structure we want to straighten basic blocks when we can which is why we have some select formation: InstCombine, Vectorization, ISel all benefit from it.
> 
> My judgement call was that this is a case where we want do it early, which is why I pushed it. Again, others might disagree with this judgement and I am happy to revert it if this is the case.
> 
> I do understand the concerns about guidance what and what not should be done. I agree discussion should be had. I defer to higher powers on this and try to make my point.
> 
> Best,
> Arnold
> 
> On Apr 20, 2013, at 8:19 PM, Chandler Carruth <chandlerc at google.com> wrote:
> 
>> 
>> On Sat, Apr 20, 2013 at 11:44 PM, Arnold <aschwaighofer at apple.com> wrote:
>> 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. 
>> 
>> These two arguments were *exactly* the ones I used, and they were very loudly objected to by everyone I mentioned. I largely agreed with you, but also saw the concerns put forward by others. They were very firm and demanded the patch be reverted. I don't even know *exactly* how similar the patches are, but at the very least I think there should be a discussion and explanation of what has changed since then. Otherwise, we have inconsistent and confusing direction on how to develop any and all of these passes.
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits