[PATCH] D76708: [ARM][LowOverheadLoops] Add horizontal reduction support

[Sam Parker via Phabricator via llvm-commits]

samparker marked 2 inline comments as done.
samparker added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp:609
   const TargetRegisterClass *QPRs = TRI.getRegClass(ARM::MQPRRegClassID);
   SetVector<MachineInstr *> Unknown;
   SmallPtrSet<MachineInstr *, 4> FalseLaneZeros;
----------------
SjoerdMeijer wrote:
> Please add a comment that Unknown is the worklist of instructions that still need to be analysed, or something along those lines.
And I'll now rename it to FalseLaneUnknown.


================
Comment at: llvm/lib/Target/ARM/ARMLowOverheadLoops.cpp:669
   }
 
   // Collect Q-regs that are live in the exit blocks. We don't collect scalars
----------------
SjoerdMeijer wrote:
> I was looking at `Unknown` again, and was wondering if it would be good to assert that Unknown should be empty at this point here. If I am not mistaken, that's what we expect here? That would require to remove items from Unknown where we insert it into to Predicated at line 667 above, but seems like a good check?
But I'm not sure what value that would add, maybe if the loop lived somewhere else, but the loop can't terminate successfully without that condition being true. I'll add a comment.


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

https://reviews.llvm.org/D76708