[PATCH][TER] PR 48863: Don't replace expressions across local register variable definitions

Kyrill Tkachov kyrylo.tkachov@foss.arm.com
Thu Dec 1 11:14:00 GMT 2016


On 24/11/16 15:12, Richard Biener wrote:
> On Thu, Nov 24, 2016 at 2:57 PM, Kyrill Tkachov
> <kyrylo.tkachov@foss.arm.com> wrote:
>> Hi all,
>>
>> In this bug we have TER during out-of-ssa misbehaving. A minimal example is:
>> void foo(unsigned a, float b)
>> {
>>    unsigned c = (unsigned) b;      // 1
>>    register unsigned r0 __asm__("r0") = a; // 2
>>    register unsigned r1 __asm__("r1") = c; // 3
>>      __asm__ volatile( "str %[r1], [%[r0]]\n"
>>                        :
>>                        : [r0] "r" (r0),
>>                          [r1] "r" (r1));
>> }
>>
>> Statement 1 can produce a libcall to convert float b into an int and TER
>> moves it
>> into statement 3. But the libcall clobbers r0, which we want set to 'a' in
>> statement 2.
>> So r0 gets clobbered by the argument to the conversion libcall.
>>
>> TER already has code to avoid substituting across function calls and ideally
>> we'd teach it
>> to not substitute expressions that can perform a libcall across register
>> variable definitions
>> where the register can be clobbered in a libcall, but that information is
>> not easy to get hold
>> off in a general way at this level.
>>
>> So this patch disallows replacement across any local register variable
>> definition. It does this
>> by keeping track of local register definitions encountered in a similar way
>> to which calls are
>> counted for TER purposes.
>>
>> I hope this is not too big a hammer as local register variables are not very
>> widely used and we
>> only disable replacement across them and it allows us to fix the wrong-code
>> bug on some
>> inline-asm usage scenarios where gcc currently miscompiles code following
>> its documented
>> advice [1]
>>
>> Bootstrapped and tested on arm-none-linux-gnueabihf, aarch64-none-linux-gnu,
>> x86_64.
>>
>> Is this approach acceptable?
> Ok.

Thanks.
This has been in trunk for a week without any problems.
Is it ok to backport to the branches?
I have bootstrapped and tested arm-none-linux-gnueabihf on them.

Kyrill

> Thanks,
> Richard.
>
>> Thanks,
>> Kyrill
>>
>> [1]
>> https://gcc.gnu.org/onlinedocs/gcc/Local-Register-Variables.html#Local-Register-Variables
>>
>> 2016-11-24  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>>
>>      PR target/48863
>>      PR inline-asm/70184
>>      * tree-ssa-ter.c (temp_expr_table): Add reg_vars_cnt field.
>>      (new_temp_expr_table): Initialise reg_vars_cnt.
>>      (free_temp_expr_table): Release reg_vars_cnt.
>>      (process_replaceable): Add reg_vars_cnt argument, set reg_vars_cnt
>>      field of TAB.
>>      (find_replaceable_in_bb): Use the above to record register variable
>>      write occurrences and cancel replacement across them.
>>
>> 2016-11-24  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>>
>>      PR target/48863
>>      PR inline-asm/70184
>>      * gcc.target/arm/pr48863.c: New test.



More information about the Gcc-patches mailing list