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

[Richard Biener <richard dot guenther at gmail dot com>]

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,
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.