llvm dev - Nov 2010 - [LLVMdev] ScheduleDAG Question

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

David Greene <dag at cray.com> writes:
> I'm missing?  Why does ScheduleDAGSDNodes even keep an insert point?
> It is bound to get out of sync with InstrEmitter.
Ok, I figured out what ScheduleDAG has an insert point.  Apparently
it's used for other types of schedulers.

I fixed the problem by changing the ScheduleDAGEmit.cpp code to
query the InstrEmitter for its insert point.  Unfortunately a
testcase is harder to produce because I ran into this problem
compiling for AVX and the current llvm trunk doesn't support
enough AVX to be able to compile the testcase.

I'm working on that.  :)

                       -Dave