Bug 85745 - variable with asm register assignment allocated in wrong reg
Summary: variable with asm register assignment allocated in wrong reg
Status: RESOLVED INVALID
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: 8.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: ra
: 86673 (view as bug list)
Depends on:
Blocks:
 
Reported: 2018-05-11 06:29 UTC by Bernd Edlinger
Modified: 2018-07-25 17:29 UTC (History)
1 user (show)

See Also:
Host:
Target: arm
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments
reduced test case (613 bytes, text/plain)
2018-05-11 06:29 UTC, Bernd Edlinger
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Bernd Edlinger 2018-05-11 06:29:43 UTC
Created attachment 44114 [details]
reduced test case

This prevents linux v4.7 and before to be built with gcc-8.1.0 on armv7 targets.

$ arm-linux-gnueabihf-gcc -v
Using built-in specs.
COLLECT_GCC=arm-linux-gnueabihf-gcc
COLLECT_LTO_WRAPPER=/home/ed/gnu/arm-linux-gnueabihf-linux64/libexec/gcc/arm-linux-gnueabihf/8.1.0/lto-wrapper
Target: arm-linux-gnueabihf
Configured with: ../gcc-8.1.0/configure --prefix=/home/ed/gnu/arm-linux-gnueabihf-linux64 --target=arm-linux-gnueabihf --enable-languages=all --with-arch=armv7-a --with-tune=cortex-a9 --with-fpu=vfpv3-d16 --with-float=hard
Thread model: posix
gcc version 8.1.0 (GCC) 

$ arm-linux-gnueabihf-gcc -save-temps -Os -c test.c
test.s: Assembler messages:
test.s:39: Error: .err encountered


in test.s we see, that __r2 is allocated in r3 but that is unexpected,
it should have been r2:
sys_oabi_sendmsg:
        @ args = 0, pretend = 0, frame = 0
        @ frame_needed = 0, uses_anonymous_args = 0
        cmp     r1, #0
        bxeq    lr
        uxth    r2, r2
        cmp     r2, #1
        cmpeq   r0, #0
        bxne    lr
        mov     r3, #0
        str     lr, [sp, #-4]!
        ldr     r3, [r3]
        mov     r0, r1
        sub     r1, r3, #1
        mov     r3, #110
        .syntax divided
@ 37 "000.c" 1
        .ifnc r0,r0; .ifnc r0r0,fpr11; .ifnc r0r0,r11fp; .ifnc r0r0,ipr12; .ifnc r0r0,r12ip; .err; .endif; .endif; .endif; .endif; .endif
        .ifnc r3,r2; .ifnc r3r2,fpr11; .ifnc r3r2,r11fp; .ifnc r3r2,ipr12; .ifnc r3r2,r12ip; .err; .endif; .endif; .endif; .endif; .endif
        .ifnc r1,r1; .ifnc r1r1,fpr11; .ifnc r1r1,r11fp; .ifnc r1r1,ipr12; .ifnc r1r1,r12ip; .err; .endif; .endif; .endif; .endif; .endif
        bl      __put_user_4
@ 0 "" 2
        .arm
        .syntax unified
        ldr     pc, [sp], #4


The after v4.8 this macro was a bit simplified, and causes less register
pressure, but I think it works only by chance.

I am not sure if that is a wrong expectation or a compiler bug.
It would be fixed when __r2 is declared volatile for instance.
Comment 1 Jakub Jelinek 2018-05-11 15:58:06 UTC
The reason this happens is that the register variable is marked const.  Don't do that.  If it is const, the compiler optimizes it more aggressively - it will happily fold uses of the variable to the constant ininitializer, so the inline asm becomes "r" (110) instead of "r" (__r2) and thus it can use any register.
This is how C++ behaved for years and how C in GCC behaves since the folding improvements.
--- gcc/c/c-fold.c	2018-05-10 19:39:13.750529734 +0200
+++ gcc/c/c-fold.c	2018-05-11 17:57:12.941957170 +0200
@@ -165,7 +165,10 @@ c_fully_fold_internal (tree expr, bool i
   if (!IS_EXPR_CODE_CLASS (kind) || kind == tcc_statement)
     {
       /* Except for variables which we can optimize to its initializer.  */
-      if (VAR_P (expr) && !lval && (optimize || in_init))
+      if (VAR_P (expr)
+	  && !lval
+	  && (optimize || in_init)
+	  && !DECL_HARD_REGISTER (expr))
 	{
 	  if (in_init)
 	    ret = decl_constant_value_1 (expr, true);
would be a workaround for this, but not really sure if we want to use it.
Comment 2 Bernd Edlinger 2018-05-11 17:07:03 UTC
Removing const fixes the build for me.
Thanks for investigating!
Comment 3 Bernd Edlinger 2018-05-14 08:42:59 UTC
just in case someone runs into the same issue, this would
be the linux patch that Jakub suggested:

commit 20dfb4d2eb648bd947adbb729d963f78df75ffee
Author: Bernd Edlinger <bernd.edlinger@hotmail.de>
Date:   Fri May 11 18:51:03 2018 +0200

    Fix compilation with gcc-8

diff --git a/arch/arm/include/asm/uaccess.h b/arch/arm/include/asm/uaccess.h
index 74b17d0..dc64fa2 100644
--- a/arch/arm/include/asm/uaccess.h
+++ b/arch/arm/include/asm/uaccess.h
@@ -220,7 +220,7 @@ extern int __put_user_8(void *, unsigned long long);
        ({                                                              \
                unsigned long __limit = current_thread_info()->addr_limit - 1; \
                const typeof(*(p)) __user *__tmp_p = (p);               \
-               register const typeof(*(p)) __r2 asm("r2") = (x);       \
+               register typeof(*(p)) __r2 asm("r2") = (x);             \
                register const typeof(*(p)) __user *__p asm("r0") = __tmp_p; \
                register unsigned long __l asm("r1") = __limit;         \
                register int __e asm("r0");                             \
Comment 4 Andrew Pinski 2018-07-25 17:28:59 UTC
C++ version of this issue is bug 56715.
Comment 5 Andrew Pinski 2018-07-25 17:29:11 UTC
*** Bug 86673 has been marked as a duplicate of this bug. ***