Another problem with "Recommit r265547, and r265610, r265639, r265657"

[Wei Mi via llvm-commits]

> Ok, I tried to make LiveVariable to run again after RegisterCoalescer but
> hit a fatal error in LiveVariables since we're not on SSA anymore:
>
>   if (!MRI->isSSA())
>     report_fatal_error("regalloc=... not currently supported with -O0");
>
> However, I managed to bring out the big hammer and "repair" LIS at the end
> of RegisterCoalescer, and then the code compiles succesfully!

That is an interesting result!

> However, I'm still curious... do you say that RegisterCoalescer should work
> and keep LiveVariables properly updated even without kill/dead-flags? Or is
> already the input to the coalescer broken since kill flags are missing?
>
> Is it a bug that LiveVariables removes the kill-flags when it's run
> somewhere prior to RegisterCoalescer or is this ok?

Before Live Interval Analysis
624B            %vreg7<def> = COPY %vreg29<kill>; aN32_rN_Pairs:%vreg7,%vreg29

After Live Interval Analysis
624B            %vreg7<def> = COPY %vreg29; aN32_rN_Pairs:%vreg7,%vreg29

I managed to reproduce the same behavior using foo.red.ll locally so
now I can see how the kill flag is removed by Live Interval Analysis.

It is done deliberately by LiveInterval Analysis in
LiveRangeCalc::extendToUses. Here is the related comment:
  // Clear all kill flags. They will be reinserted after register allocation
  // by LiveIntervalAnalysis::addKillFlags().

This looks reasonable because after LiveInterval is setup, following
passes mostly depends on LiveInterval instead of dead/kill flags.

>
> So when kill and dead flags should be present and who should update them is
> still quite unclear for me.

Operand kill and dead flags are initially set by LiveVariables
Analysis. Seems only TwoAddressInstructionPass and Register Scavenger
requires that. For the other regalloc related passes including
RegisterCoalescer, they get dead and kill information directly using
LiveInterval, but they may update Operand kill and dead flags. I guess
at two points kill and dead flags should be properly set -- before
TwoAddressInstructionPass and after regalloc - for scavenger. Other
points, it is not mandatory.

>
> And if the register allocator has assumptions about how the input code
> should look, shouldn't there be some checks that those assumptions are
> fulfilled rather than ending up with invalid live ranges after regalloc that
> the verifier shouts about?

I agree with you. Ideally verifier should catch the non-meaningful
live interval about %vreg29 early -- after RegisterCoalescing when we
turn on -verify-coalescing. Existing verifier doesn't have it maybe
because that means to recompute and verify if current LiveInterval
needs to be updated, and it is costly.


A still important question is: what is wrong with the LiveInterval
update in RegisterCoalescer. Although I can reproduce to know how the
kill flag is cleaned in LiveInterval Analysis, I havn't reproduced the
LiveInterval update problem in RegisterCoalescer successfully yet.

Thanks,
Wei.