This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: combine/dce patch for PR36003, PR42575


On 06/29/2010 07:38 PM, Eric Botcazou wrote:
>> This produces better results in a number of cases, but it's not a clear
>> win.  In smallish testcases it often produces extra instructions due to
>> the general problem IRA currently has with tracking lifetimes of DImode
>> subwords.  If that gets fixed we might consider the patch, but right now
>> I don't think we can.
>>
>>         smull   r4, r5, r2, ip
>> +       mov     ip, r4
>>         ldr     r6, [r0, r3]
>> -       mov     ip, r4, lsr #31
>> +       mov     r4, r5
>> +       mov     ip, ip, lsr #31
> 
> The original testcase is smallish too so it would be interesting to understand 
> why it helps for certain smallish testcases and pessimizes for others.

As I said, it's the known IRA deficiency.  Your patch creates more
instances of

 (set (reg:DI x) (something))
 (set (reg:SI a) (subreg x 0))
 (set (reg:SI b) (subreg x 1))

where IRA treats x as conflicting with a.  If we can fix this in IRA, we
should retest this patch (or something similar).

> How does your can_decompose_p patch come into play here?

Not at all, I think.  It's a different problem, we're not creating
superfluous DImode pseudos here.

>> It also seems somewhat hackish to me to treat some operations specially
>> for no really good reason.  Why do we care about the type of the source
>> operand?  Can't we just decompose anything that occurs in a SET_DEST?
> 
> We aren't trying to enhance lower-subreg,

I think we are, if that's the best solution for the problem.

IMO my mini-DCE is unambiguously beneficial as it only deletes
instructions, it should be free in the normal case since it's guarded by
the "First see if there are any multi-word pseudo-registers" test, and
cheap even when there are multi-word pseudos.  It's not explicitly tied
to multiplications.  On the other hand it has very little effect in
general beyond fixing the testcase.  Since we may be able to do better,
I'm withdrawing the patch for now, until we decide whether IRA will be
enhanced to cope better with DImode lifetimes.

Once (if) IRA is fixed, a plausible approach for lower-subreg would be
to decompose DImode regs that are set once in a non-decomposable
context.  After the initializing insn, we can move its subregs to new
SImode pseudos, just as we're doing for ASM_OPERANDS now.


Bernd


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]