[PATCH] D77523: Add CanonicalizeFreezeInLoops pass

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

In D77523#2014624 <https://reviews.llvm.org/D77523#2014624>, @aqjune wrote:

> Hi,
>
> In D77523#2012741 <https://reviews.llvm.org/D77523#2012741>, @jdoerfert wrote:
>
> > I'm asking myself if it wouldn't be better to start with the freeze instructions and work our way up through the operands. At the end of the day we do a lot of work just to figure out there is no freeze or the instruction chains we build do not end in one. Am I missing something?
>
>
> It's true, but we should iterate over instructions inside the loop to find freeze instructions.
>  Indexing freezes that are uses of induction variables will be helpful for exiting early; right now, DivRemPair is the place where it introduces freezes, but it does not do relevant conversion in x86-64, so the code will have almost zero freeze instructions.
>  Would it make sense if IVUsers maintains the info (existence of freeze)? I'm not familiar with the class, but may have a look.


It might but that means maintaining state, which is hard and error prone.

I was thinking more along the lines of this:

  for (auto *I : L->instruction())
    if (auto *Fr = dyn_cast<Freeze>(I))
      handleFreeze(Fr);
  
  handleFreeze(L, Fr) { 
  Worklist = {Fr};
  while (!Worklist.empty()) {
    Inst =  dyn_cast<Instruction>(Worklist.pop())
    if (!Inst || !L->contains(Inst) || !Visited.insert(V))
      continue;
    if (isSpecialPHI(V))
      // found special PHI that ends in a freeze, do record it
    for (auto &Op : Inst->operands())
      Worklist.push_back(Op);
  }
  }

The main difference is that we are performing a single traversal of the loop + work per freeze instruction that is bounded by the def-use chain ending in the freeze and contained in the loop.

>> Also, could this drop flags of instructions not on the path between a phi and a freeze? So do we keep the `nsw` on the sub and mul in the following example:
>> 
>>   %iv = phi [0, ...] [%step, ...]
>>   %step = add nsw %iv, 1
>>   %f = freeze %step
>>   %not_step1 = sub nsw %iv, 1
>>   %not_step2 = mul nsw %step, 2
> 
> The nsw flags of sub and mul will be preserved. I'll add this as a test.

Thx!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77523