[PATCH][ree] PR rtl-optimization/78038: Handle global register dataflow definitions in ree

[Kyrill Tkachov]

On 20/10/16 19:52, Jeff Law wrote:
> On 10/20/2016 08:57 AM, Kyrill Tkachov wrote:
>> Hi all,
>>
>> In this PR we've got code like this:
>> register struct test2_s *g __asm__("x28");
>>
>> void
>> do_something ()
>> {
>>   test_fptr ();
>>   struct test2_s *p1 = 0;
>>   *p1 = *g;
>> }
>>
>> And we get an ICE in gcc/ree.c:655 in get_sub_rtx.
>> The problem is when we try to process the defining insn of register x28
>> from the insn:
>> (set (reg/f:DI 1 x1 [orig:74 g.1_2 ] [74])
>>     (zero_extend:DI (reg/v:SI 28 x28 [ g ])))
>>
>> The dataflow reports the insn setting x28 to be:
>> (call_insn 8 7 10 2 (parallel [
>>             (call (mem:DI (reg/f:DI 0 x0 [orig:77 test_fptr ] [77]) [0
>> *test_fptr.0_1 S8 A8])
>>                 (const_int 0 [0]))
>>             (use (const_int 0 [0]))
>>             (clobber (reg:DI 30 x30))
>>         ]) "ree.c":14 41 {*call_reg}
>>      (expr_list:REG_CALL_DECL (nil)
>>         (nil))
>>     (expr_list (clobber (reg:DI 17 x17))
>>         (expr_list (clobber (reg:DI 16 x16))
>>             (nil))))
>>
>> which, as you can see, doesn't actually set x28.
>> AFAICS the reason dataflow reports call_insn 8 as defining x28 is
>> because x28 is a global register variable
>> and that means that every function call is considered to define it.
>> But the ree pass can't really use any of that. It only wants some SET
>> RTL patterns to play with.
>> One solution is to bail out at the ICE location, in get_sub_rtx, when no
>> SETs are found inside the parallel.
>> But I think we shouldn't be getting this far along anyway.
>> So this patch prevents the call_insn from being recognised as a defining
>> insn for x28 for the purposes of ree.
>> It makes sure that if the register we're extending is a global register
>> that all the insns in the def chain
>> actually set it directly as determined by set_of from rtlanal.c.
>> This should hopefully still allow ree to optimise extensions of global
>> registers as long as they are combined
>> with legitimate defining insns.
>>
>> Bootstrapped and tested on aarch64, arm, x86_64.
>>
>> Ok for trunk?
>>
>> Thanks,
>> Kyrill
>>
>> 2016-10-20  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>>
>>     PR rtl-optimization/78038
>>     * ree.c (get_defs): Return NULL if a defining insn for REG cannot
>>     be deduced to set REG through the RTL structure.
>>     (make_defs_and_copies_lists): Return false on a failing get_defs call.
>>
>> 2016-10-20  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>>
>>     PR rtl-optimization/78038
>>     * gcc.target/aarch64/pr78038.c: New test.
> OK.

Thanks.
As this patch has been in trunk for more than a week with no issues I'd like to apply it to the release branches.
I've so far bootstrapped and tested it on the GCC 6 branch on aarch64-none-linux-gnu and I'll be applying it there.

Kyrill

>
> Jeff
>