CriticalAntiDepBreaker rewrites a register operand of a call instruction

I am running into a problem when I turn on post-RA scheduler with mode “ANTIDEP_CRITICAL” for mips.
I’d appreciate if someone could explain what is going wrong here.

This is the basic block before post RA scheduling (at PostRASchedulerList.cpp:322):

(gdb)
#3 0x0000000000ed3d26 in runOnMachineFunction (this=0x20aa470, Fn=…)
at lib/CodeGen/PostRASchedulerList.cpp:322
322 Scheduler.Observe(MI, CurrentCount);

(gdb) p (*MBB).dump()
BB#218: derived from LLVM BB %if.then1289
Live Ins: %A2 %S0_64 %S1_64 %S2_64 %S4 %S5_64 %T0 %T1 %T2 %T3_64 %T6 %T8 %T9
Predecessors according to CFG: BB#217

%V0 = ADDu %ZERO, %T9
%T9_64 = LD_P8 %T3_64, ga:intrapred[TF=3]; mem:LD8[GOT]

JALR64 %T9_64, %A0_64, %A1, %A2, %A3, %T0, , %SP, %V0

BNE %V0, %V1, <BB#372>

MI is JALR64 which is a call (jump-and-link-register). T9_64 is the destination register and A0 - A3 and T0 are the argument registers.
These registers are not callee-saved.

(gdb) p MI->dump()
JALR64 %T9_64, %A0_64, %A1, %A2, %A3, %T0, , %SP, %V0

After the region above the JALR64 instruction is scheduled, the basic block looks like this (at PostRASchedulerList.cpp:341):

(gdb)
341 Scheduler.FixupKills(MBB);
(gdb) p (*MBB).dump()
BB#218: derived from LLVM BB %if.then1289
Live Ins: %A2 %S0_64 %S1_64 %S2_64 %S4 %S5_64 %T0 %T1 %T2 %T3_64 %T6 %T8 %T9
Predecessors according to CFG: BB#217
%S6_64 = LD_P8 %T3_64, ga:intrapred[TF=3]; mem:LD8[GOT]
%V0 = ADDu %ZERO, %T9

JALR64 %S6_64, %A0_64, %A1, %A2, %A3, %T0, , %SP, %V0

$110 = void

CriticalAntiDepBreaker has replaced T9_64 with S6_64 to break an anti-dependence edge.
This code is incorrect since the first operand of JALR64 has to be T9_64 in PIC mode.

After further investigation, I found that the flag for T9_64 of CriticalAntiDepBreaker::KeepRegs that had been set in PrescanInstruction was reset in ScanInstruction, which I think is causing the problem:

CriticalAntiDepBreaker.cpp:
00154 PrescanInstruction(MI);
00155 ScanInstruction(MI, Count);

T9_64 is cleared in CriticalAntiDepBreaker.cpp:264:

Breakpoint 8, llvm::CriticalAntiDepBreaker::ScanInstruction (this=0x262d290,
MI=0x22b2168, Count=15)
at lib/CodeGen/CriticalAntiDepBreaker.cpp:264
264 KeepRegs.reset(i);
(gdb) p i
$179 = 145
(gdb) p /x KeepRegs.test(145)
$180 = 0x1

145 is the value of T9_64.

In CriticalAntiDepBreaker.cpp, there is a comment " Also assume all registers used in a call must not be changed (ABI).", but T9_64 is changed here. Also it says “registers that are defed but not used in this instruction are now dead”, but I don’t see the code that checks whether T9_64 is used.

Hi Akira,

I am running into a problem when I turn on post-RA scheduler with mode
"ANTIDEP_CRITICAL" for mips.
I'd appreciate if someone could explain what is going wrong here.

All these passes are pretty sensitive to correct register liveness
information. As a first step I'd check whether machine verifier
reports no errors here.

Hi Anton,

Thanks for the suggestions.

I compiled the .ll file with llc with command line options
-verify-dom-info, -verify-regalloc and -verify-loop-info.
I didn't see any diagnostic messages.

When I add -verify-machineinstrs, it complains that there are instructions
after terminator instructions.
It seems that these error messages are printed because the verifier does
not understand that mips has delay slots, not because it has detected any
true violations.

$ llc macroblock.llvm.mips64el.ll -mcpu=mips64r2 -O3 -o macroblock.s
-mattr=n64 -verify-machineinstrs

# After PreEmit passes
# Machine code for function start_macroblock: Post SSA

BB#0: derived from LLVM BB %entry
    Live Ins: %A0_64 %T9_64 %RA_64 %S3_64 %S2_64 %S1_64 %S0_64
    BEQ %A0<kill>, %ZERO, <BB#2>
    NOP
    Successors according to CFG: BB#2 BB#1

# End machine code for function start_macroblock.

*** Bad machine code: MBB exits via unconditional fall-through but doesn't
have exactly one CFG successor! ***
- function: start_macroblock
- basic block: entry 0x3ce4700 (BB#0)

*** Bad machine code: Non-terminator instruction after the first terminator

Hi Akira,

When I add -verify-machineinstrs, it complains that there are instructions
after terminator instructions.

Yes, -verify-machineinstrs and -verify-coalescing are your friends here :slight_smile:

Hi Anton,

I ran llc with -verify-coalescing. There were no error messages.
Then I added code in MipsPassConfig::addPreEmitPass() to prevent machine
verifier from running post delay -slot-filler, and ran llc again. Again,
there were no error messages.

This is the list of passes run after post-RA scheduling. machine verifier
is run twice after post RA scheduler (and CriticalAntiDepBreaker) is run.

      Post RA top-down list latency scheduler
      Verify generated machine code
      Analyze Machine Code For Garbage Collection
      Machine Block Frequency Analysis
      Branch Probability Basic Block Placement
      Verify generated machine code
      Mips Delay Slot Filler
      MachineDominator Tree Construction
      Machine Natural Loop Construction
      Mips Assembly Printer
      Delete Garbage Collector Information

Sorry, I meant to say,

I added code to prevent llc from running machine verifier after delay slots
are filled.

MipsInstrInfo::AnalyzeBranch generates incorrect results after delay slots
are filled. Also, it seems that code in MachineVerifier.cpp wasn't written
with architectures that have delay slots in mind.

I filed a PR.

Bug 12829