[PATCH] D32869: [globalisel][tablegen] Require that all registers between instructions of a match are virtual.

[Daniel Sanders via Phabricator via llvm-commits]

dsanders added a comment.

In https://reviews.llvm.org/D32869#752657, @qcolombet wrote:

> Hi Daniel,
>
> We are talking pass each other :).


:-) For my part, I was trying to solve a problem that originates from tablegen matching the shape before testing the predicates.

For example, suppose you have a rule that matches (G_FADD (G_FMUL x, y), z). If the tablegen code is presented with:

  %R0 = COPY %0
  %R0 = COPY %1
  %0 = COPY %R0  // <-- MI

it will capture the instructions for that rule with code to the effect of:

  MI0 = MI;
  MI1 = getVRegDef(MI0.getOperand(1).getReg());

triggering the assertion in getVRegDef() when multiple defs are detected.

I believe we can fix this for SelectionDAG-imported rules by checking the opcode as we go, but I think the problem will re-occur once we support GlobalISel specific rules. Particularly if targets introduce target-instructions before ISel. Forbidding physical registers covers both situations.

While I'm on the subject of issues involving getVRegDef(). The use of getVRegDef() is a bit questionable for another reason too. Nothing ensures that the def it finds occurs before the use. That's fine for vregs but that assumption isn't valid when you have something like:

  liveins: %r0
  %0 = COPY %r0 // getVRegDef(%r0) will return the instruction below
  %r0 = COPY %0

which can occur during ISel.

> I think I miss the use case you want to solve.
> 
> On my side, I thought you wanted to fallback on cases where we have a physical register use or def like in ADJUSTSTACKxxx.
> 
>   let Defs = [SP], Uses = [SP], [...] // <-- I am talking about that
>   [...]
>   def ADJCALLSTACKDOWN : [...]
>   [...]
> 
> 
> Whereas I was saying we want to handle such case like this:
>  SP = vregIn
>  SP = ADJCALL SP
>  vregOut = SP
> 
> instead of falling back, given it will happen.
> 
> Cheers,
> -Quentin

I don't think we have the same problem on implicit uses/defs. Attempting to follow the def use chain with getVRegDef() will have the same problems as above, but tablegen will never try to do this because it only follows the chains for explicit uses.


https://reviews.llvm.org/D32869