[PATCH] D137758: [HardwareLoops] remove loops which has undef count

[ChenZheng via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/CodeGen/HardwareLoops.cpp:447
+  // dedicated exit block, so we just delete these loops.
+  if (isa<UndefValue>(Count)) {
+    LLVM_DEBUG(dbgs() << " - Bailing, loop count is undef\n");
----------------
samparker wrote:
> shchenz wrote:
> > fhahn wrote:
> > > I doubt this is enough. I think you would at least have to ensure that the branch on undef is executed unconditionally in the loop. Also, shouldn't those dead loops be cleaned up earlier?
> > > I think you would at least have to ensure that the branch on undef is executed unconditionally in the loop.
> > 
> > For hardware loops, the exiting block which contains the compare with loop count will be executed in each loop iteration.
> > 
> > > Also, shouldn't those dead loops be cleaned up earlier?
> > 
> > OPT has the pass that can clean up this kind of loop. But in LLC pipeline, there is no such pass.
> Is there a blocker on adding the pass to the llc pipeline?
hmm, to be honest, I didn't think about adding the pass to the llc pipeline. I am not sure if it is OK.

In OPT, for the case `remove-loop-count-undef-loop.ll`, the loops are optimized out by two passes, instcombine and loop deletion (Seems like loop deletion can only delete a simple loop optimized after instcombine)
- `opt -loop-deletion  -S remove-loop-count-undef-loop.ll`, the loop is kept
- `opt -instcombine -loop-deletion  -S remove-loop-count-undef-loop.ll`, the loop will be deleted.

I am thinking adding instcombine pass to llc pipeline is not a good idea and doing nested instruction combining to reduce loop complexity first in loop deletion pass also seems strange?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137758