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: [PATCH 4.8, i386]: Enable post-reload compare optimization pass (PR28685)


On Tue, Feb 7, 2012 at 11:00 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Mon, Feb 6, 2012 at 10:30 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>
>>> Hmm. ?Well, the only thing that's going to work for x86 is the double-compare
>>> elimination portion.
>>>
>>> If we want to use this pass for x86, then for 4.8 we should also fix the
>>> discrepancy between the compare-elim canonical
>>>
>>> ?[(operate)
>>> ? (set-cc)]
>>>
>>> and the combine canonical
>>>
>>> ?[(set-cc)
>>> ? (operate)]
>>>
>>> (Because of the simplicity of the substitution in compare-elim, I prefer
>>> the former as the canonical canonical.)
>>
>> You are probably referring to following testcase:
>>
>> --cut here--
>> int test (int a, int b)
>> {
>> ?int lt = a + b < 0;
>> ?int eq = a + b == 0;
>> ?if (lt)
>> ? ?return 1;
>> ?return eq;
>> }
>> --cut here--
>>
>> where combine creates:
>>
>> Trying 8 -> 9:
>> Successfully matched this instruction:
>> (parallel [
>> ? ? ? ?(set (reg:CCZ 17 flags)
>> ? ? ? ? ? ?(compare:CCZ (plus:SI (reg/v:SI 63 [ a ])
>> ? ? ? ? ? ? ? ? ? ?(reg/v:SI 64 [ b ]))
>> ? ? ? ? ? ? ? ?(const_int 0 [0])))
>> ? ? ? ?(set (reg:SI 60 [ D.1710 ])
>> ? ? ? ? ? ?(plus:SI (reg/v:SI 63 [ a ])
>> ? ? ? ? ? ? ? ?(reg/v:SI 64 [ b ])))
>> ? ?])
>
> Attached patch teaches combine to swap operands of a double set
> pattern and retries recognition. Also added are minimum
> target-dependant changes to handle the testcase above.
>
> Unfortunately, compare elimination was not able to remove redundant
> compare, although the testcase is carefully crafted to require only
> sign flag to be valid. Following enters compare-elim pass:
>
> (insn 9 8 10 2 (parallel [
> ? ? ? ? ? ?(set (reg:SI 5 di [orig:60 D.1710 ] [60])
> ? ? ? ? ? ? ? ?(plus:SI (reg/v:SI 5 di [orig:63 a ] [63])
> ? ? ? ? ? ? ? ? ? ?(reg/v:SI 4 si [orig:64 b ] [64])))
> ? ? ? ? ? ?(set (reg:CCZ 17 flags)
> ? ? ? ? ? ? ? ?(compare:CCZ (plus:SI (reg/v:SI 5 di [orig:63 a ] [63])
> ? ? ? ? ? ? ? ? ? ? ? ?(reg/v:SI 4 si [orig:64 b ] [64]))
> ? ? ? ? ? ? ? ? ? ?(const_int 0 [0])))
> ? ? ? ?]) cmp.c:4 261 {*addsi_2}
> ? ? (nil))
>
> (note 10 9 33 2 NOTE_INSN_DELETED)
>
> (insn 33 10 34 2 (set (reg:QI 1 dx [65])
> ? ? ? ?(eq:QI (reg:CCZ 17 flags)
> ? ? ? ? ? ?(const_int 0 [0]))) cmp.c:4 595 {*setcc_qi}
> ? ? (nil))
>
> (insn 34 33 30 2 (set (reg:SI 1 dx [65])
> ? ? ? ?(zero_extend:SI (reg:QI 1 dx [65]))) cmp.c:4 123
> {*zero_extendqisi2_movzbl}
> ? ? (nil))
>
> (insn 30 34 29 2 (set (reg/v:SI 0 ax [orig:59 eq ] [59])
> ? ? ? ?(const_int 1 [0x1])) cmp.c:6 64 {*movsi_internal}
> ? ? (expr_list:REG_EQUAL (const_int 1 [0x1])
> ? ? ? ?(nil)))
>
> (insn 29 30 31 2 (set (reg:CCGOC 17 flags)
> ? ? ? ?(compare:CCGOC (reg:SI 5 di [orig:60 D.1710 ] [60])
> ? ? ? ? ? ?(const_int 0 [0]))) cmp.c:6 2 {*cmpsi_ccno_1}
> ? ? (nil))
>
> (insn 31 29 25 2 (set (reg/v:SI 0 ax [orig:59 eq ] [59])
> ? ? ? ?(if_then_else:SI (ge (reg:CCGOC 17 flags)
> ? ? ? ? ? ? ? ?(const_int 0 [0]))
> ? ? ? ? ? ?(reg:SI 1 dx [65])
> ? ? ? ? ? ?(reg/v:SI 0 ax [orig:59 eq ] [59]))) cmp.c:6 903 {*movsicc_noc}
> ? ? (nil))
>
> The resulting code still includes redundant test that sets sign flag:
>
> test:
> ? ? ? ?addl ? ?%esi, %edi
> ? ? ? ?movl ? ?$1, %eax
> ? ? ? ?sete ? ?%dl
> ?>> ? testl ? %edi, %edi
> ? ? ? ?movzbl ?%dl, %edx
> ? ? ? ?cmovns ?%edx, %eax
> ? ? ? ?ret
>
> (BTW: I think that the change to combine.c would be nice to have, to
> find more other combine opportunities. I will propose the patch
> separately.)

Shouldn't there be a canonical order for parallels throughout the whole
compiler?  Maybe just enforced by gen_rtx_PARALLEL / RTL checking?
At least as far as I understand "execution order" of insns inside a PARALLEL
is undefined.

Richard.

> Uros.


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