[llvm-dev] Early CSE clobbering llvm.assume

[Daniel Berlin via llvm-dev]

Maybe.  It may not fix it directly because you never use %1 or %2 again.
I haven't looked to see how good the lookup is.

On Fri, Jun 10, 2016, 3:45 PM Josh Klontz <josh.klontz at gmail.com> wrote:

> Thanks Daniel, with that knowledge I think I can at least work around the
> issue in my frontend.
>
> Ignoring GVN for a second though, and just looking at Early CSE, it seems
> to me that at least in this pass that there is the potential for an easy
> fix. Here the issue appears to be that we hit
>
>   if (Value *V = SimplifyInstruction(Inst, DL, &TLI, &DT, &AC))
>
> immediately replacing %1 with 3 before we even reach %3. If we were to
> record this replacement in EarlyCSE::AvailableValues, wouldn't that address
> the issue? I'll try this out and see.
>
> v/r,
> Josh
>
> On Fri, Jun 10, 2016 at 4:23 PM, Daniel Berlin <dberlin at dberlin.org>
> wrote:
>
>> Yeah, that change is completely unrelated, that is about correctness,
>> this is about optimization.
>> I'm working on a proposal to just fix assume at some point to deal with
>> the former issue.
>>
>> The problem with this testcase is that all the ways assume is propagate
>> expect the variable in the assume to later be used.
>>
>>
>> <This is the main way assume constants are propagated>
>> bool GVN::processAssumeIntrinsic(IntrinsicInst *Inst) {
>> ...
>>
>>    // We can replace assume value with true, which covers cases like this:
>>    // call void @llvm.assume(i1 %cmp)
>>    // br i1 %cmp, label %bb1, label %bb2 ; will change %cmp to true
>>    ReplaceWithConstMap[V] = True;
>> ...
>> }
>>
>> ...
>> bool GVN::processBlock(BasicBlock *BB) {
>> ...
>>    // Clearing map before every BB because it can be used only for single
>> BB.
>>    ReplaceWithConstMap.clear();
>> ....
>>
>> }
>>
>>
>> So it's going to go through the rest of the bb, see nothing with %2, do
>> nothing, and then next iteration, clear the constant map.  It's not valid
>> to avoid clearing the constant map, and in fact,  what is happening here is
>> a pretty complicated to help with.
>> There is no easy way to see that the load  at %3 is affected at all by
>> the assume.
>>
>> It's possible to make this work using the predication/value inference
>> algorithms in the paper newgvn is based on, but it's not even implemented
>> there.
>>
>> Short answer, without special casing this in magic ways, i wouldn't
>> expect this to get fixed anytime soon.
>>
>> If we fixed assume in one of the ways i thought about, like bodik's
>> extended ssa:
>>
>> http://homepages.dcc.ufmg.br/~fernando/classes/dcc888/ementa/slides/RangeAnalysis.pdf
>>
>> You would at least see that the load result is used by an assume, and
>> could go look at that assume and so something with it.  Currently, it's a
>> few steps away.
>>
>> In the current scheme, assumes just float in air, and so it can be hard
>> to see what their effects touch
>> :)
>>
>>
>> On Fri, Jun 10, 2016 at 2:00 PM, Josh Klontz via llvm-dev <
>> llvm-dev at lists.llvm.org> wrote:
>>
>>> Thanks for the lead Kevin. Unfortunately when I updated to ToT the
>>> problem persists. Will put together a minimum reproducing example.
>>>
>>> On Fri, Jun 10, 2016 at 12:26 PM, Smith, Kevin B <
>>> kevin.b.smith at intel.com> wrote:
>>>
>>>> You might look at this commit to fix the problem: r270823
>>>> "MemorySSA: Revert r269678 and r268068; replace with special casing in
>>>> MemorySSA."
>>>>
>>>> I think that might fix the issue for you.
>>>>
>>>> Kevin Smith
>>>>
>>>>
>>>
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> llvm-dev at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160610/360457b3/attachment.html>