[LLVMdev] ScheduleDAG Question

[David Greene]

I am tracking down a tricky bug involving select/CMOV and scheduling.
In my test, I have a float select that has to be implemented with a
diamond CFG by the scheduler.  The high level 
ScheduleDAGSDNodes::EmitSchedule does this:


  for (unsigned i = 0, e = Sequence.size(); i != e; i++) {
    [...]
    Emitter.EmitNode(SU->getNode(), SU->OrigNode != SU, SU->isCloned,
                     VRBaseMap);
    [...]
  }

TheInstrEmitter::EmitMachineNode does this:

  if (II.usesCustomInsertionHook()) {
    // Insert this instruction into the basic block using a target
    // specific inserter which may returns a new basic block.
    bool AtEnd = InsertPos == MBB->end();
    MachineBasicBlock *NewMBB = TLI->EmitInstrWithCustomInserter(MI, MBB);
    if (NewMBB != MBB) {
      if (AtEnd)
        InsertPos = NewMBB->end();
      MBB = NewMBB;
    }
    return;
  }

So now the InstrEmitter's insert point has been updated to point to the BOTTOM
MachineBasicBlock of the diamond, but ScheduleDAGSDNodes' insertion point
is still pointing to the TOP block of the diamond!  Isn't that wrong?

What I'm seeing is the SDUnit following the diamond being emitted at the 
diamond top block AFTER the terminating jump by this code in  
ScheduleDAGSDNodes::EmitSchedule:

    // For pre-regalloc scheduling, create instructions corresponding to the
    // SDNode and any flagged SDNodes and append them to the block.
    if (!SU->getNode()) {
      // Emit a copy.
      EmitPhysRegCopy(SU, CopyVRBaseMap);
      continue;
    }

EmitPhysRegCopy doesn't use the InstrEmitter so it is using the stale emit 
point.  Later instructions use the InstrEmitter and so get put in the right 
place.

I think EmitPhysRegCopy should use the InstrEmitter too.  Is there something 
I'm missing?  Why does ScheduleDAGSDNodes even keep an insert point?
It is bound to get out of sync with InstrEmitter.

I will try to get a reduced testcase and file a bug.

Thanks!

                                   -Dave