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

[Nikita Popov via Phabricator via llvm-commits]

nikic 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");
----------------
shchenz wrote:
> 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?
Generally speaking, if opt optimizes something, then llc should **not** optimize it, unless the optimization is only possible as a result of legalization. It doesn't sound like that's the case here?

The input to llc is assumed to be optimized IR -- if it isn't, it's sufficient to not crash or miscompile it, but there is no need to produce good code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137758