On Thu, Dec 1, 2016 at 12:14 PM, Kyrill Tkachov
<kyrylo.tkac...@foss.arm.com> wrote:
>
> On 24/11/16 15:12, Richard Biener wrote:
>>
>> On Thu, Nov 24, 2016 at 2:57 PM, Kyrill Tkachov
>> <kyrylo.tkac...@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.
I think so.
Richard.
> 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.tkac...@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.tkac...@arm.com>
>>>
>>> PR target/48863
>>> PR inline-asm/70184
>>> * gcc.target/arm/pr48863.c: New test.
>
>
