[PATCH] D93734: [LoopDeletion] Insert an early exit from dead path in loop

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

In D93734#2469162 <https://reviews.llvm.org/D93734#2469162>, @jonpa wrote:

> In D93734#2469137 <https://reviews.llvm.org/D93734#2469137>, @jdoerfert wrote:
>
>> The idea is that we have
>>
>>   H:
>>     %c = ... ; invariant wrt H and L
>>     br %c, L, B
>>   B: 
>>     side_effects
>>     br L
>>   L:
>>     br %x, H, Exit
>>   
>>   Exit:
>>     ...
>>
>> right?
>
> Yes, exactly (could be more blocks than 3, though, of course).
>
>> One of my problems is that this is too tied to the syntax.
>
> How do you mean, exactly?



  H:
    %c1 = ... ; invariant wrt H, LazyEval, and L
    br %c1, LazyEval, B
  LazyEval:  
    %c2 = ... ; invariant wrt H, LazyEval, and L
    br %c2, L, B
  B: 
    side_effects
    br L
  L:
    br %x, H, Exit
  Exit:
     ...
   

I mean the above breaks the matching even though the same logic applies. I would argue that is not what we want.
(And given that %c1 and %c2 can have loads, we might not be able to undo short circuit evaluation here to reduce
 it to your initial pattern.)

>> Isn't this more related to LoopUnswitch?
>> We want to "unswitch the paths that are side-effect free", or something like that.
>
> I thought that if we cannot delete the whole loop we can effectively delete a part of it with an early exit.

We delete parts, agreed, but not a loop. Unsure if this is the right place conceptually.

> LoopUnswitching only works if the condition is loop invariant currently as far as I can see, so I am not sure how simple that would be...

With that argumentation you could say LoopDeletion doesn't do this either (right now) ;)
I mean, you can simply copy this code there and it should work, right?

>> We could also make it generic by collecting the blocks that are allowed on such a path
>> in order to allow different CFGs. But that can be done later as well.
>
> Might be worth a try...

FWIW, I my goal is to make it something driven by a generic analysis, not another pattern we match.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93734/new/

https://reviews.llvm.org/D93734