[PATCH] D78206: [Target][ARM] Make Low Overhead Loops coexist with VPT blocks

[Pierre van Houtryve via Phabricator via llvm-commits]

Pierre-vh added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp:230
+      MachineInstr *Op2Def = RDA.getMIOperand(MI, MI->getOperand(2));
+      if(Op1Def == nullptr && Op2Def == nullptr)
+        return false;
----------------
samparker wrote:
> Pierre-vh wrote:
> > samparker wrote:
> > > I'm still pondering about this... I think this should be checking that both operands are either predicated q-regs, scalar or are loop invariant, that way should ensure that the result will be the same after the transform. This method also restricts us to instructions that are directly predicated, so it would be good to add a TODO here as well.
> > > I'm still pondering about this... I think this should be checking that both operands are either predicated q-regs, scalar or are loop invariant, that way should ensure that the result will be the same after the transform.
> > 
> > If I understand correctly, this function currently only handles the first case (predicated q-regs), but it should be expanded to allow any r-reg and loop invariants (registers that aren't defined inside any of the loop's basic blocks) ?
> > 
> > > This method also restricts us to instructions that are directly predicated, so it would be good to add a TODO here as well.
> > 
> > What do you mean by "directly predicated"? Can you please give me an example of an indirectly predicated instruction in this context?
> By directly predicated, I mean in a VPT block. This pass also tries to understand which instructions are indirectly predicated too, by looking at their use-def chain in LowOverheadLoop::ValidateLiveOuts. Just checking q-regs is fine at the moment, but is it really acceptable is only one operand is predicated?
Right, I see, I'll change this so it requires that both operands are either a predicated q-reg, a r-reg, or a loop invariant.
Can I accept every r-reg, or only the loop invariants ones? Can I accept loop-invariant non-predicated q-regs as well?


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

https://reviews.llvm.org/D78206