Warning: unpredictable: identical transfer and status registers --`stxr w4,x5,[x4] using aarch64 poky gcc 8.3

Warning: unpredictable: identical transfer and status registers --`stxr w4,x5,[x4] using aarch64 poky gcc 8.3

[Peng Fan]

Hi,

We met an issue when building a piece jailhouse hypervisor code, "stxr   %w0, %3, %2\n\t" is 
compiled as "stxr w4,x5,[x4]" which triggers the warning 
"Warning: unpredictable: identical transfer and status registers"
After folder the do while into asm code, it is compiled as "stxr w1, x4, [x2]", no warning.

aarch64 poky gcc 8.3 triggers warning, aarch64 poky gcc 7.3 does not trigger warning, but has generated
same machine code. Not sure this is gcc bug or not, is my patch the correct fix? please help.

Detailed info below, the jailhouse repo code: 
https://github.com/siemens/jailhouse/blob/master/hypervisor/arch/arm64/include/asm/bitops.h#L77

static inline int test_and_set_bit(int nr, volatile unsigned long *addr)
{
        u32 ret;
        u64 test, tmp;

        BITOPT_ALIGN(nr, addr);

        /* AARCH64_TODO: using Inner Shareable DMB at the moment,
         * revisit when we will deal with shareability domains */

        do {
                asm volatile (
                        "ldxr   %3, %2\n\t"
                        "ands   %1, %3, %4\n\t"
                        "b.ne   1f\n\t"
                        "orr    %3, %3, %4\n\t"
                        "1:\n\t"
                        "stxr   %w0, %3, %2\n\t"
                        "dmb    ish\n\t"
                        : "=r" (ret), "=&r" (test),
                          "+Q" (*(volatile unsigned long *)addr),
                          "=r" (tmp)
                        : "r" (1ul << nr));
        } while (ret);
        return !!(test);
}

void panic_printk(const char *fmt, ...)
{
        unsigned long cpu_id = phys_processor_id();
        va_list ap;

        if (test_and_set_bit(0, &panic_in_progress) && panic_cpu != cpu_id)
                return;
        panic_cpu = cpu_id;

        va_start(ap, fmt);

        __vprintk(fmt, ap);

        va_end(ap);
}

The asm code:
0000ffffc0207a94 <panic_printk>:
    ffffc0207a94:       a9b67bfd        stp     x29, x30, [sp, #-160]!
    ffffc0207a98:       910003fd        mov     x29, sp
    ffffc0207a9c:       f9000bf3        str     x19, [sp, #16]
    ffffc0207aa0:       aa0003f3        mov     x19, x0
    ffffc0207aa4:       a9068ba1        stp     x1, x2, [x29, #104]
    ffffc0207aa8:       a90793a3        stp     x3, x4, [x29, #120]
    ffffc0207aac:       a9089ba5        stp     x5, x6, [x29, #136]
    ffffc0207ab0:       f9004fa7        str     x7, [x29, #152]
    ffffc0207ab4:       97ffed27        bl      ffffc0202f50 <phys_processor_id>
    ffffc0207ab8:       f0000081        adrp    x1, ffffc021a000 <system_config>
    ffffc0207abc:       d2800022        mov     x2, #0x1                        // #1
    ffffc0207ac0:       91006024        add     x4, x1, #0x18
    ffffc0207ac4:       c85f7c85        ldxr    x5, [x4]
    ffffc0207ac8:       ea0200a3        ands    x3, x5, x2
    ffffc0207acc:       54000041        b.ne    ffffc0207ad4 <panic_printk+0x40>  // b.any
    ffffc0207ad0:       aa0200a5        orr     x5, x5, x2
    ffffc0207ad4:       c8047c85        stxr    w4, x5, [x4]
    ffffc0207ad8:       d5033bbf        dmb     ish
    ffffc0207adc:       35ffff24        cbnz    w4, ffffc0207ac0 <panic_printk+0x2c>

Note pc ffffc0207ad4 has instruction stxr w4, x5, [x4] which is triggers warning using aarch64 poky gcc 8.3

I did a fix to the code as below:
--- a/hypervisor/arch/arm64/include/asm/bitops.h
+++ b/hypervisor/arch/arm64/include/asm/bitops.h
@@ -83,21 +83,21 @@ static inline int test_and_set_bit(int nr, volatile unsigned long *addr)

        /* AARCH64_TODO: using Inner Shareable DMB at the moment,
         * revisit when we will deal with shareability domains */
+       asm volatile (
+               "1:\n\t"
+               "ldxr   %3, %2\n\t"
+               "ands   %1, %3, %4\n\t"
+               "b.ne   2f\n\t"
+               "orr    %3, %3, %4\n\t"
+               "2:\n\t"
+               "stxr   %w0, %3, %2\n\t"
+               "dmb    ish\n\t"
+               "cbnz   %w0, 1b\n\t"
+               : "=r" (ret), "=&r" (test),
+                 "+Q" (*(volatile unsigned long *)addr),
+                 "=r" (tmp)
+               : "r" (1ul << nr));

-       do {
-               asm volatile (
-                       "ldxr   %3, %2\n\t"
-                       "ands   %1, %3, %4\n\t"
-                       "b.ne   1f\n\t"
-                       "orr    %3, %3, %4\n\t"
-                       "1:\n\t"
-                       "stxr   %w0, %3, %2\n\t"
-                       "dmb    ish\n\t"
-                       : "=r" (ret), "=&r" (test),
-                         "+Q" (*(volatile unsigned long *)addr),
-                         "=r" (tmp)
-                       : "r" (1ul << nr));
-       } while (ret);
        return !!(test);
 }

Then the asm code as below:
0000ffffc02079e0 <panic_printk>:
    ffffc02079e0:       a9b67bfd        stp     x29, x30, [sp, #-160]!
    ffffc02079e4:       910003fd        mov     x29, sp
    ffffc02079e8:       f9000bf3        str     x19, [sp, #16]
    ffffc02079ec:       aa0003f3        mov     x19, x0
    ffffc02079f0:       a9068be1        stp     x1, x2, [sp, #104]
    ffffc02079f4:       a90793e3        stp     x3, x4, [sp, #120]
    ffffc02079f8:       a9089be5        stp     x5, x6, [sp, #136]
    ffffc02079fc:       f9004fe7        str     x7, [sp, #152]
    ffffc0207a00:       97ffed44        bl      ffffc0202f10 <phys_processor_id>
    ffffc0207a04:       d0000082        adrp    x2, ffffc0219000 <system_config>
    ffffc0207a08:       d2800021        mov     x1, #0x1                        // #1
    ffffc0207a0c:       91006042        add     x2, x2, #0x18
    ffffc0207a10:       c85f7c44        ldxr    x4, [x2]
    ffffc0207a14:       ea010083        ands    x3, x4, x1
    ffffc0207a18:       54000041        b.ne    ffffc0207a20 <panic_printk+0x40>  // b.any
    ffffc0207a1c:       aa010084        orr     x4, x4, x1
    ffffc0207a20:       c8017c44        stxr    w1, x4, [x2]
    ffffc0207a24:       d5033bbf        dmb     ish
    ffffc0207a28:       35ffff41        cbnz    w1, ffffc0207a10 <panic_printk+0x30>

And there is no warning, the instruction is correct "stxr w1, x4, [x2]"

Re: Warning: unpredictable: identical transfer and status registers --`stxr w4,x5,[x4] using aarch64 poky gcc 8.3

[Segher Boessenkool]

OneWed, Feb 13, 2019 at 07:13:21AM +0000, Peng Fan wrote:
> We met an issue when building a piece jailhouse hypervisor code, "stxr   %w0, %3, %2\n\t" is 
> compiled as "stxr w4,x5,[x4]" which triggers the warning 
> "Warning: unpredictable: identical transfer and status registers"

This is not a GCC question.

The three registers, in order, are status, transfer, and base.  The warning
claims transfer and status are identical, but in fact base and status are.
The code (in binutils, gas/config/tc-aarch64.c) is

    case ldstexcl:
      /* It is unpredictable if the destination and status registers are the
         same.  */
      if ((aarch64_get_operand_class (opnds[0].type)
           == AARCH64_OPND_CLASS_INT_REG)
          && (aarch64_get_operand_class (opnds[1].type)
              == AARCH64_OPND_CLASS_INT_REG)
          && (opnds[0].reg.regno == opnds[1].reg.regno
              || opnds[0].reg.regno == opnds[2].reg.regno))
        as_warn (_("unpredictable: identical transfer and status registers"
                   " --`%s'"),
                 str);

so either that op0 == op2 test is spurious, or the warning message is
misleading.

Please ask on binutils@sourceware.org and/or file a bug at
https://sourceware.org/bugzilla/ ?


Segher

RE: Warning: unpredictable: identical transfer and status registers --`stxr w4,x5,[x4] using aarch64 poky gcc 8.3

[Peng Fan]

> -----Original Message-----
> From: jailhouse-dev@googlegroups.com
> [mailto:jailhouse-dev@googlegroups.com] On Behalf Of Segher Boessenkool
> Sent: 2019年2月13日 17:11
> To: Peng Fan <peng.fan@nxp.com>
> Cc: gcc@gcc.gnu.org; james.greenhalgh@arm.com; nd@arm.com;
> jailhouse-dev@googlegroups.com; will.deacon@arm.com; Catalin Marinas
> <catalin.marinas@arm.com>
> Subject: Re: Warning: unpredictable: identical transfer and status registers
> --`stxr w4,x5,[x4] using aarch64 poky gcc 8.3
> 
> OneWed, Feb 13, 2019 at 07:13:21AM +0000, Peng Fan wrote:
> > We met an issue when building a piece jailhouse hypervisor code,
> "stxr   %w0, %3, %2\n\t" is
> > compiled as "stxr w4,x5,[x4]" which triggers the warning
> > "Warning: unpredictable: identical transfer and status registers"
> 
> This is not a GCC question.
> 
> The three registers, in order, are status, transfer, and base.  The warning
> claims transfer and status are identical, but in fact base and status are.
> The code (in binutils, gas/config/tc-aarch64.c) is
> 
>     case ldstexcl:
>       /* It is unpredictable if the destination and status registers are the
>          same.  */
>       if ((aarch64_get_operand_class (opnds[0].type)
>            == AARCH64_OPND_CLASS_INT_REG)
>           && (aarch64_get_operand_class (opnds[1].type)
>               == AARCH64_OPND_CLASS_INT_REG)
>           && (opnds[0].reg.regno == opnds[1].reg.regno
>               || opnds[0].reg.regno == opnds[2].reg.regno))
>         as_warn (_("unpredictable: identical transfer and status registers"
>                    " --`%s'"),
>                  str);
> 
> so either that op0 == op2 test is spurious, or the warning message is
> misleading.

From aarch64 spec, "stxr w4,x5,[x4]"'s behavior is chip implementation defined,
so I think the warning is correct.

> 
> Please ask on binutils@sourceware.org and/or file a bug at
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsour
> ceware.org%2Fbugzilla%2F&amp;data=02%7C01%7Cpeng.fan%40nxp.com%
> 7C2463376de5aa417d3d6508d69197e4df%7C686ea1d3bc2b4c6fa92cd99c5c
> 301635%7C0%7C0%7C636856478900947075&amp;sdata=pg%2FZmo91Cxfji
> ESlBiR5shmsdxoYWslpfZeAXk5TV30%3D&amp;reserved=0 ?

Ok. I'll post questions to binutils@sourceware.org

Thanks,
Peng.

> 
> 
> Segher
> 
> --
> You received this message because you are subscribed to the Google Groups
> "Jailhouse" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to jailhouse-dev+unsubscribe@googlegroups.com.
> For more options, visit
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgrou
> ps.google.com%2Fd%2Foptout&amp;data=02%7C01%7Cpeng.fan%40nxp.co
> m%7C2463376de5aa417d3d6508d69197e4df%7C686ea1d3bc2b4c6fa92cd9
> 9c5c301635%7C0%7C0%7C636856478900947075&amp;sdata=oiNHshxQvku
> OjuTiRKlK%2F3em5DIGNysm1UbZZKuHcjg%3D&amp;reserved=0.

Re: Warning: unpredictable: identical transfer and status registers --`stxr w4,x5,[x4] using aarch64 poky gcc 8.3

[Andreas Schwab]

On Feb 13 2019, Peng Fan <peng.fan@nxp.com> wrote:

> static inline int test_and_set_bit(int nr, volatile unsigned long *addr)
> {
>         u32 ret;
>         u64 test, tmp;
>
>         BITOPT_ALIGN(nr, addr);
>
>         /* AARCH64_TODO: using Inner Shareable DMB at the moment,
>          * revisit when we will deal with shareability domains */
>
>         do {
>                 asm volatile (
>                         "ldxr   %3, %2\n\t"
>                         "ands   %1, %3, %4\n\t"
>                         "b.ne   1f\n\t"
>                         "orr    %3, %3, %4\n\t"
>                         "1:\n\t"
>                         "stxr   %w0, %3, %2\n\t"
>                         "dmb    ish\n\t"
>                         : "=r" (ret), "=&r" (test),
>                           "+Q" (*(volatile unsigned long *)addr),
>                           "=r" (tmp)
>                         : "r" (1ul << nr));

%3 is modified early, but not marked earlyclobber.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: Warning: unpredictable: identical transfer and status registers --`stxr w4,x5,[x4] using aarch64 poky gcc 8.3

[Michael Matz]

Hi,

On Wed, 13 Feb 2019, Peng Fan wrote:

>                 asm volatile (
>                         "ldxr   %3, %2\n\t"
>                         "ands   %1, %3, %4\n\t"
>                         "b.ne   1f\n\t"
>                         "orr    %3, %3, %4\n\t"
>                         "1:\n\t"
>                         "stxr   %w0, %3, %2\n\t"
>                         "dmb    ish\n\t"
>                         : "=r" (ret), "=&r" (test),
>                           "+Q" (*(volatile unsigned long *)addr),
>                           "=r" (tmp)
>                         : "r" (1ul << nr));

As Andreas says, you need to add an early-clobber for op3 for correctness 
(to force it into a different register from op4).  And you also need an 
early-clobber on op0 to force it into a different register from op2 (which 
for purposes of register assignment is an input operand holding an 
address).


Ciao,
Michael.

RE: Warning: unpredictable: identical transfer and status registers --`stxr w4,x5,[x4] using aarch64 poky gcc 8.3

[Peng Fan]

Hi Michael,

> -----Original Message-----
> From: Michael Matz [mailto:matz@suse.de]
> Sent: 2019年2月13日 22:28
> To: Peng Fan <peng.fan@nxp.com>
> Cc: gcc@gcc.gnu.org; james.greenhalgh@arm.com; nd@arm.com;
> jailhouse-dev@googlegroups.com; will.deacon@arm.com; Catalin Marinas
> <catalin.marinas@arm.com>
> Subject: Re: Warning: unpredictable: identical transfer and status registers
> --`stxr w4,x5,[x4] using aarch64 poky gcc 8.3
> 
> Hi,
> 
> On Wed, 13 Feb 2019, Peng Fan wrote:
> 
> >                 asm volatile (
> >                         "ldxr   %3, %2\n\t"
> >                         "ands   %1, %3, %4\n\t"
> >                         "b.ne   1f\n\t"
> >                         "orr    %3, %3, %4\n\t"
> >                         "1:\n\t"
> >                         "stxr   %w0, %3, %2\n\t"
> >                         "dmb    ish\n\t"
> >                         : "=r" (ret), "=&r" (test),
> >                           "+Q" (*(volatile unsigned long *)addr),
> >                           "=r" (tmp)
> >                         : "r" (1ul << nr));
> 
> As Andreas says, you need to add an early-clobber for op3 for correctness (to
> force it into a different register from op4).  And you also need an
> early-clobber on op0 to force it into a different register from op2 (which for
> purposes of register assignment is an input operand holding an address).

So the fix should be the following, right?
        do {
                asm volatile (
                        "ldxr   %3, %2\n\t"
                        "ands   %1, %3, %4\n\t"
                        "b.ne   1f\n\t"
                        "orr    %3, %3, %4\n\t"
                        "1:\n\t"
                        "stxr   %w0, %3, %2\n\t"
                        "dmb    ish\n\t"
                        : "=&r" (ret), "=&r" (test),
                          "+Q" (*(volatile unsigned long *)addr),
                          "=&r" (tmp)
                        : "r" (1ul << nr));
        } while (ret);

Thanks,
Peng.

> 
> 
> Ciao,
> Michael.

RE: Warning: unpredictable: identical transfer and status registers --`stxr w4,x5,[x4] using aarch64 poky gcc 8.3

[Peng Fan]

> -----Original Message-----
> From: Andreas Schwab [mailto:schwab@suse.de]
> Sent: 2019年2月13日 21:59
> To: Peng Fan <peng.fan@nxp.com>
> Cc: gcc@gcc.gnu.org; james.greenhalgh@arm.com; nd@arm.com;
> jailhouse-dev@googlegroups.com; will.deacon@arm.com; Catalin Marinas
> <catalin.marinas@arm.com>
> Subject: Re: Warning: unpredictable: identical transfer and status registers
> --`stxr w4,x5,[x4] using aarch64 poky gcc 8.3
> 
> On Feb 13 2019, Peng Fan <peng.fan@nxp.com> wrote:
> 
> > static inline int test_and_set_bit(int nr, volatile unsigned long
> > *addr) {
> >         u32 ret;
> >         u64 test, tmp;
> >
> >         BITOPT_ALIGN(nr, addr);
> >
> >         /* AARCH64_TODO: using Inner Shareable DMB at the moment,
> >          * revisit when we will deal with shareability domains */
> >
> >         do {
> >                 asm volatile (
> >                         "ldxr   %3, %2\n\t"
> >                         "ands   %1, %3, %4\n\t"
> >                         "b.ne   1f\n\t"
> >                         "orr    %3, %3, %4\n\t"
> >                         "1:\n\t"
> >                         "stxr   %w0, %3, %2\n\t"
> >                         "dmb    ish\n\t"
> >                         : "=r" (ret), "=&r" (test),
> >                           "+Q" (*(volatile unsigned long *)addr),
> >                           "=r" (tmp)
> >                         : "r" (1ul << nr));
> 
> %3 is modified early, but not marked earlyclobber.

Thanks, I'll try add earlyclobber.

Peng

> 
> Andreas.
> 
> --
> Andreas Schwab, SUSE Labs, schwab@suse.de GPG Key fingerprint = 0196
> BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7 "And now for something
> completely different."

RE: Warning: unpredictable: identical transfer and status registers --`stxr w4,x5,[x4] using aarch64 poky gcc 8.3

[Michael Matz]

Hi,

On Wed, 13 Feb 2019, Peng Fan wrote:

> So the fix should be the following, right?

Yup.


Ciao,
Michael.

RE: Warning: unpredictable: identical transfer and status registers --`stxr w4,x5,[x4] using aarch64 poky gcc 8.3

[Peng Fan]

> -----Original Message-----
> From: Michael Matz [mailto:matz@suse.de]
> Sent: 2019年2月13日 22:45
> To: Peng Fan <peng.fan@nxp.com>
> Cc: gcc@gcc.gnu.org; james.greenhalgh@arm.com; nd@arm.com;
> jailhouse-dev@googlegroups.com; will.deacon@arm.com; Catalin Marinas
> <catalin.marinas@arm.com>
> Subject: RE: Warning: unpredictable: identical transfer and status registers
> --`stxr w4,x5,[x4] using aarch64 poky gcc 8.3
> 
> Hi,
> 
> On Wed, 13 Feb 2019, Peng Fan wrote:
> 
> > So the fix should be the following, right?
> 
> Yup.

Thanks for your help.

Thanks,
Peng.

> 
> 
> Ciao,
> Michael.