[PATCH] D19002: [LazyValueInfo] Fix for a nasty compile-time problem with questions

[Gerolf Hoflehner via llvm-commits]

Philip,
can you share data about your compile-time improvements? Platform, benchmarks, options? Did you measure performance impact also?

Thanks
Gerolf 
> On Apr 26, 2016, at 7:08 PM, Gerolf Hoflehner <ghoflehner at apple.com> wrote:
> 
> Yes, I had already kicked off a build with your patches. Unfortunately it does not (at least no significantly) fix the issue and the test case is still running (for about 30 min now). My fix brings CT down to 2 min on an RA compiler.
> 
>> On Apr 26, 2016, at 5:53 PM, Philip Reames <listmail at philipreames.com> wrote:
>> 
>> reames added a comment.
>> 
>> I'm 99% sure this patch is no longer needed after "267642: [LVI] Reduce compile time by lazily scanning blocks if needed.".  Can you test and confirm?
>> 
>> 
>> ================
>> Comment at: lib/Analysis/LazyValueInfo.cpp:65
>> @@ +64,3 @@
>> +/// constantrange applies only to integers. constant/notconstant applies to
>> +/// non-integers, too.
>> +
>> ----------------
>> I landed a change which clarifies this.  Please rebase.
> Ok, sure.
>> 
>> ================
>> Comment at: lib/Analysis/LazyValueInfo.cpp:246
>> @@ -241,2 +245,3 @@
>>    if (isNotConstant()) {
>>      if (RHS.isConstant()) {
>> +        // A value is different on two paths -> mark as bottom (overdefined)
>> ----------------
>> I re-read my previous comment and realized I was wrong.  This code path is in the conservative meet (i.e. merge two paths at a phi).  Conflicting facts here merely imply we can't reason precisely about the values flowing through the merge.  It does not imply unreachable code.  Sorry for the confusion.  
> Do you suggest improving the comment? It still looks OK to me.
>> 
>> 
>> http://reviews.llvm.org/D19002
>> 
>> 
>> 
>