[llvm-commits] [llvm] r167758 - /llvm/trunk/lib/CodeGen/RegisterCoalescer.cpp

[Andrew Trick]

On Nov 12, 2012, at 2:25 PM, Jakob Stoklund Olesen <stoklund at 2pi.dk> wrote:

> 
> On Nov 12, 2012, at 1:42 PM, Andrew Trick <atrick at apple.com> wrote:
> 
>> +// Return true if this block should be vacated by the coalescer to eliminate
>> +// branches. The important cases to handle in the coalescer are critical edges
>> +// split during phi elimination which contain only copies. Simple blocks that
>> +// contain non-branches should also be vacated, but this can be handled by an
>> +// earlier pass similar to early if-conversion.
> 
> You probably don't want to do this for critical edges entering and exiting loops.

Ahh... I think what you're saying is that it would be nice to give the splitter a place to (re)insert copies since it doesn't try to split edges. I'll benchmark it.

Incidentally, the main problem I have with making the coalescer more aggressive is that the splitter can't seem to recover and we end up with more spills around complex control flow. I'm just guessing it's because we have less split edges but still need to verify.

>> +static bool isSplitEdge(const MachineBasicBlock *MBB) {
>> +  if (MBB->pred_size() != 1 || MBB->succ_size() != 1)
>> +    return false;
>> +
>> +  for (MachineBasicBlock::const_iterator MII = MBB->begin(), E = MBB->end();
>> +       MII != E; ++MII) {
> 
> E = MBB->getFirstTerminator() ?

I'm checking the branch too. Just not bothering to optimize the order of checking because it seems unlikely to have a ton of copies (and nothing else) followed by a non copy or conditional br.

>> +    if (MII->isCopyLike())
>> +      continue;
> 
> MII->isTransient() ?

I though about doing that and wasn't sure what made more sense. I'm currently only trying to unsplit edges that were split by the coalescer itself. Are we likely to see INSERT_SUBREG in an empty block? And do you want to optimize that case in the coalescer? In general, isTransient is not guaranteed to be removed after coalescing, right? So initially I took the conservative approach.

-Andy

>> +    if (MII->isUnconditionalBranch())
>> +      continue;
>> +    return false;
>> +  }
>> +  return true;
>> +}
> 
> /jakob
>