[PATCH] Fix instruction scheduling live register tracking

Andrew Trick atrick at apple.com
Fri May 29 00:48:37 PDT 2015


I worked through some examples on paper. I haven't debugged real examples, so could be wrong about something...

In the normal two-address case with only a single physical register, LiveRegDef and LiveRegGen should always be non-null when a node that defines the same register is unscheduled. So I understand why you didn't hit any assert and why we don't have any unit tests (which would need to be realizable from valid IR with a real target).

A comment above the assert should be sufficient, as long as this
passes all the testing that Hal mentioned and doesn't break any
bots. You could say something like:

  With interference on only a single physical register (flags), a node
  that defines the register will not be unscheduled unless it is a
  two-address instruction. In that case, LiveRegDef will currently be
  set to the def that reaches this instruciton, and LiveRegGen will
  already be set to the last user of this instruction.

I might even convert the assert into report_fatal_error since the condition is theoretically possible and could result in a miscompile if true.

Now, to answer your question, here's a hypothetical case the motivates
the current code:

Say Rx and Ry are a physical registers and we have this DAG:
------------------------------------------------------------

Ry = I0
Rx = I1(I0:Ry)
I2(I1:Rx)
I3(I1:Rx)
Rx = I4(I0)

I5(I4:Rx)
---------

We first schedule this sequence bottom-up:

Rx = I1(I0:Ry)
I2(I1:Rx)
I3(I1:Rx)

Giving us:
Def[Rx]=null, Def[Ry]=I0
Gen[Rx]=null, Gen[Ry]=I1

Since I4 has not been scheduled, we cannot schedule I0 and need to backtrack.

Unscheduling I1, we see two successors: I2 and I3. I3 has the lower height, so we should end up with:
Def[Rx]=I1, Def[Ry]=null
Gen[Rx]=I3, Gen[Ry]=null

If we mistakenly picked Gen[Rx]=I2, then we would not backtrack far enough and would miscompile this example!

So, if fixing the code to check scheduled order instead of DAG height fixes your problem, that would be the best solution.


REPOSITORY
  rL LLVM

http://reviews.llvm.org/D9993

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list