[llvm-commits] [llvm] r47046 - in /llvm/trunk: include/llvm/CodeGen/LiveIntervalAnalysis.h lib/CodeGen/SimpleRegisterCoalescing.cpp lib/CodeGen/SimpleRegisterCoalescing.h

[Evan Cheng]

On Feb 13, 2008, at 9:31 AM, Chris Lattner wrote:

> On Feb 13, 2008, at 1:47 AM, Evan Cheng wrote:
>>> Very nice, does it also help shootout/fib?
>>
>> bh speedup doesn't seem real. It doesn't help fib.
>
> Ok, once things have settled, please take a look at the comments in
> the bugzilla to see if there are other cases being missed.
>
>>> This idiom happens a lot in the preexisting code.  Please use:
>>>
>>> VNInfo *AValNo = IntA.FindLiveRangeContaining(CopyIdx-1)->valno;
>>
>> ALR is also used below.
>
> Ok!
>
>>>> +  for (unsigned i = 0, e = DefMI->getNumOperands(); i != e; ++i) {
>>>
>>> This loop needs a comment. What are you trying to find with this
>>> loop?  Maybe it should be moved out to its own function which just
>>> returns idx?  This would force you to name it, which would describe
>>> what it does :)
>>
>> There is a reason I said "initial" in the commit message. This is not
>> done. I need a target hook to tell me what is the new destination
>> register after commuting. Right now this is basically assuming x86
>> commutable instructions: A = A + B
>
> Ok.
>
>>> Another random question unrelated to this code: now that we have
>>> efficient RAUW, can we eliminate the 'rep'  mapping stuff and just
>>> RAUW vregs as they are coallesced?
>>
>> Well, actually it's related. I think it's required for correctness.
>> See below.
>
> Ah, I see.  Well this seems like an independently useful change that
> will both speed up and simplify coalescing.  Are you interested in
> tackling it as part of this project?

That's the plan. Without RAUW change, I can't enable this optimization.

It's *shouldn't* take long.

Evan

>
>
>>> Yay for use_iterators :)
>>
>> Except I don't think this is quite right. Iterating through uses of
>> IntA.reg means we end up not updating other uses that have already
>> been coalesced to it.
>
> Yeah, that's bad.
>
>>> Where the use iterator could point to the first r2, and incrementing
>>> it could point to the next r2.  This could be avoided by not zapping
>>> instructions in this loop: move the copy zapping to a walk over a
>>> smallset or something.
>>
>> Nothing is being zapped in the loop. Copies that would be zapped are
>> put into a set JoinedCopies. In fact, all uses have to be updated to
>> the new register.
>
> Ok.
>
>>> It isn't clear to me what this code is doing.  It looks like it is
>>> looking for the copy that has now becomes an identity copy.  Is  
>>> there
>>> some way to factor this with other code that is coallescing copies
>>> away?  It seems duplicated from somewhere else.  It would be nicer  
>>> to
>>
>> Not really. A number of things are happening here. We are trying to
>> determine which copies are now identity copies and make sure their
>> valno# will be eliminated. We also need to transfer the live ranges
>> defined by the (now dead) copies to the new val# defined by the
>> commuted definition MI. It's also tracking kill information.
>
> Ok, please comment it a bit better.  Is there any way to avoid this
> work or factor this out into functions that can be shared with other
> code?
>
>>> just add all copies to a small vector and then zap them later or
>>> something, to keep the logic simple.
>>
>> That's essentially what's happening. This has gone through a number  
>> of
>> iterations already. Apart from some cosmetic changes, it's not going
>> to get much simpler than this.
>
> ok.
>
>>> Does this need to be an ivar?  Can it just be passed by-ref between
>>> the methods that need it?
>>
>> This will be cleaned up later as part of RAUW change.
>
> Ok.  Thanks Evan!
>
> -Chris
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits