[PATCH] D27193: MachineLoopInfo: add function findInductionRegister.

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet added inline comments.


================
Comment at: include/llvm/CodeGen/MachineLoopInfo.h:174
+  ///   R = R + #bump
+  ///   if (R < #N) goto loop
+  /// IVBump is the immediate value added to R, and IVOp is the instruction
----------------
kparzysz wrote:
> qcolombet wrote:
> > Which register would you return in such case:
> >   ///   R0 = phi ..., [ R1, LatchBlock ]
> >   ///   R1 = R0 + #bump
> >   ///   if (R1 < #N) goto loop
> > 
> > Is this supposed to work only on non-SSA code?
> The induction register is the one that is being "bumped", i.e. R0 in your case.  This code only works on SSA.
Thanks Krzysztof.
Could we update the comment accordingly?


================
Comment at: include/llvm/CodeGen/MachineLoopInfo.h:176
+  /// IVBump is the immediate value added to R, and IVOp is the instruction
+  /// "R = R + #bump".
+  bool findInductionRegister(MachineLoop *L, unsigned &Reg, int64_t &IVBump,
----------------
SjoerdMeijer wrote:
> kparzysz wrote:
> > qcolombet wrote:
> > > What do we do we more general representation?
> > > E.g., R = R * 2
> > > Or with R = R + RuntimeConstant?
> > > 
> > > Basically where I am going is shouldn't we have a representation a la SCEV?
> > Runtime constants won't work, neither will *=.  This code was meant to get the typical "+= const" patterns.
> > 
> > 
> > Regarding the M-SCEV: yes, we should have it.  What's missing is the time to do it.  :)
> > 
> > It's been on my to-do-eventually list, and my idea was to use some sort of a common underlying representation of instructions (i.e. all add instructions for all targets would be represented as a generic "add" instruction). Now that we have GlobalISel, this groundwork is in place, we could develop some sort of a translation scheme from MIR to GIR(?), and then develop SCEV on top of that.
> Thanks for the reviews.  Yes, I agree, the recognised loops will be simple loops, and thus the approach is not very general, but it could serve some purposes though. Or now that GlobalIsel is being developed, is this technical debt that we don't want to develop further? I haven't followed the development of GlobalIsel close enough, but is it stable enough that we could be looking into SCEVs for machine instructions/GIR?
GlobalISel simplifies some bits by providing generic opcodes with known semantic, but, by design, GlobalISel allows to mix those with target specific opcodes, so we would still need some way to retrieve the semantic of those.

Regarding the general approach, I would say this is up to you. I think it make sense to have a SCEV like framework at the machine level, I don't see how that patch helps us toward that goal though.

I think what we lack is a way to extract semantic from the instructions. GlobalISel partially solves that. The general problem is still open.


https://reviews.llvm.org/D27193