[Bug target/86673] [8/9 regression] inline asm sometimes ignores 'register asm("reg")' declarations
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86673 --- Comment #18 from Alexander Monakov --- Author: amonakov Date: Mon Jul 30 12:26:37 2018 New Revision: 263065 URL: https://gcc.gnu.org/viewcvs?rev=263065=gcc=rev Log: doc: discourage const/volatile on register variables (PR 86673) PR target/86673 * doc/extend.texi (Global Register Variables): Discourage use of type qualifiers. (Local Register Variables): Likewise. Modified: trunk/gcc/ChangeLog trunk/gcc/doc/extend.texi
[Bug target/86673] [8/9 regression] inline asm sometimes ignores 'register asm("reg")' declarations
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86673 Andrew Pinski changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |DUPLICATE --- Comment #17 from Andrew Pinski --- Dup of bug 85745. *** This bug has been marked as a duplicate of bug 85745 ***
[Bug target/86673] [8/9 regression] inline asm sometimes ignores 'register asm("reg")' declarations
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86673 --- Comment #16 from Alexander Monakov --- > I don't see what should we warn about. I think the suggestion was to warn when a register const variable appears in constraints, because the asm is not then guaranteed to receive that operand in that register, contrary to user's expectations? So basically warn about this non-intuitive aspect of local reg vars so the places where the code makes a wrong assumption can be identified and fixed.
[Bug target/86673] [8/9 regression] inline asm sometimes ignores 'register asm("reg")' declarations
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86673
--- Comment #15 from Jakub Jelinek ---
register const void __user *__p asm("r0") = __ptr;
etc. isn't a problem, here __p isn't const, only *__p is. The problem is if
the register variable itself is const.
So
arch/h8300/kernel/sim-console.c:register const unsigned _len
__asm__("er2") = n;
If n can be a constant or is const var initialized to constant etc.
The cases where a const register var doesn't have an initializer aren't a
problem either.
I don't see what should we warn about.
[Bug target/86673] [8/9 regression] inline asm sometimes ignores 'register asm("reg")' declarations
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86673 Alexander Monakov changed: What|Removed |Added CC||jakub at gcc dot gnu.org --- Comment #14 from Alexander Monakov --- Indeed, I agree the documentation should mention that (I can write a patch). IIRC 'volatile register' also does not behave intuitively (I'll try to double check). I'm unsure about a warning, it seems "doing it right" might require changes in both C and C++ frontends, and issuing a warning only if an asm statement using the register variable is present? Adding Jakub to Cc.
[Bug target/86673] [8/9 regression] inline asm sometimes ignores 'register asm("reg")' declarations
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86673
--- Comment #13 from Arnd Bergmann ---
(In reply to Andreas Schwab from comment #12)
> arch/h8300/kernel/sim-console.c: register const int fd __asm__("er0") =
> 1;
I found that too, and then noticed it is already fixed in linux-next:
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=14cf9451be78f8a
Ard points out that most of the other ones are pointers to const data, which
are not a problem. This leaves the arm put_user bug as the only definite
problem that needs to be addressed in older kernels.
The three arch/riscv instances of 'const register unsigned long gp __asm__
("gp")' are different because they are never passed into an inline assembly as
far as I can tell. This seems to be unsupported for local register variables
according to the gcc documentation, but if that's a problem, it's unrelated to
this bug.
[Bug target/86673] [8/9 regression] inline asm sometimes ignores 'register asm("reg")' declarations
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86673
--- Comment #12 from Andreas Schwab ---
arch/h8300/kernel/sim-console.c:register const int fd __asm__("er0") =
1; /* stdout */
[Bug target/86673] [8/9 regression] inline asm sometimes ignores 'register asm("reg")' declarations
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86673
--- Comment #11 from Arnd Bergmann ---
I have checked all instances of 'register const' or 'const register' in the
current linux kernel (4.18-rc), and we never assign a constant expression to
any of them, so I guess none of them are affected:
arch/arm/include/asm/uaccess.h: register const void __user *__p
asm("r0") = __ptr; \
arch/h8300/kernel/sim-console.c:register const char *_ptr
__asm__("er1") = s;
arch/h8300/kernel/sim-console.c:register const unsigned _len
__asm__("er2") = n;
arch/mips/include/asm/uaccess.h:register const void __user *__cu_from_r
__asm__("$5"); \
arch/mips/include/asm/uaccess.h:register const void *__cu_from_r
__asm__("$5"); \
arch/riscv/kernel/process.c:const register unsigned long gp __asm__
("gp");
arch/riscv/kernel/stacktrace.c: const register unsigned long current_sp
__asm__ ("sp");
arch/riscv/kernel/stacktrace.c: const register unsigned long current_sp
__asm__ ("sp");
Should we drop the 'const' for all of them as a rule? If there is no use case
for ever using a 'const register' variable and it can lead to bugs, should gcc
warn about it in the future?
Should this issue be mentioned in the documentation in
https://gcc.gnu.org/onlinedocs/gcc/Local-Register-Variables.html?
I also checked all instances in linux-4.4, and the ARM put_user() helper is the
only one I see that gets a constant expression input, so I suppose that is all
that needs to be fixed in backports, unless someone thinks we should get rid of
all them in backports as well.
[Bug target/86673] [8/9 regression] inline asm sometimes ignores 'register asm("reg")' declarations
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86673 Alexander Monakov changed: What|Removed |Added CC||amonakov at gcc dot gnu.org --- Comment #10 from Alexander Monakov --- See PR 85745 where Jakub said, 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.
[Bug target/86673] [8/9 regression] inline asm sometimes ignores 'register asm("reg")' declarations
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86673
--- Comment #9 from Arnd Bergmann ---
Reproduced on arm64 and x86 as well, see x86 version:
void fn1() {
register const int h asm("edx") = 1;
__asm__(".ifnc %0,edx; .err; .endif" :: "r"(h));
}
[Bug target/86673] [8/9 regression] inline asm sometimes ignores 'register asm("reg")' declarations
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86673
--- Comment #8 from Thomas Preud'homme ---
(In reply to Thomas Preud'homme from comment #7)
> (In reply to Thomas Preud'homme from comment #6)
> > The following simpler testcase also shows the problem:
> >
> > void fn1() {
> > register const int h asm("r2") = 1;
> > __asm__(".ifnc %0,r2; .err; .endif\n\t"
> > "bl __put_user_4" :: "r"(h));
> > }
>
> The register label gets optimized during gimple stages.
Dump for original already shows propagation of the constant has happened which
later lead to the removal of the register declaration:
;; Function fn1 (null)
;; enabled by -tree-original
{
register const int h __asm__ (*r2) = 1;
register const int h __asm__ (*r2) = 1;
__asm__ __volatile__(".ifnc %0,r2; .err; .endif\n\tbl\t__put_user_4"::"r" 1);
}
[Bug target/86673] [8/9 regression] inline asm sometimes ignores 'register asm("reg")' declarations
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86673
--- Comment #7 from Thomas Preud'homme ---
(In reply to Thomas Preud'homme from comment #6)
> The following simpler testcase also shows the problem:
>
> void fn1() {
> register const int h asm("r2") = 1;
> __asm__(".ifnc %0,r2; .err; .endif\n\t"
> "bl __put_user_4" :: "r"(h));
> }
The register label gets optimized during gimple stages.
[Bug target/86673] [8/9 regression] inline asm sometimes ignores 'register asm("reg")' declarations
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86673
--- Comment #6 from Thomas Preud'homme ---
The following simpler testcase also shows the problem:
void fn1() {
register const int h asm("r2") = 1;
__asm__(".ifnc %0,r2; .err; .endif\n\t"
"bl __put_user_4" :: "r"(h));
}
[Bug target/86673] [8/9 regression] inline asm sometimes ignores 'register asm("reg")' declarations
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86673
--- Comment #5 from Arnd Bergmann ---
(In reply to Andreas Schwab from comment #4)
> Why are you using empty constraints when a register is required?
creduce did that, it had no effect on the result. The original source looks
like:
#define __get_user_x_64t(__r2, __p, __e, __l, __s) \
__asm__ __volatile__ ( \
__asmeq("%0", "r0") __asmeq("%1", "r2") \
__asmeq("%3", "r1") \
"bl __get_user_64t_" #__s \
: "=" (__e), "=r" (__r2) \
: "0" (__p), "r" (__l) \
: __GUP_CLOBBER_##__s)
[Bug target/86673] [8/9 regression] inline asm sometimes ignores 'register asm("reg")' declarations
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86673 --- Comment #4 from Andreas Schwab --- Why are you using empty constraints when a register is required?
[Bug target/86673] [8/9 regression] inline asm sometimes ignores 'register asm("reg")' declarations
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86673
Ramana Radhakrishnan changed:
What|Removed |Added
Target||arm-none-linux-gnueabi ,
||arm-none-eabi
Status|UNCONFIRMED |NEW
Last reconfirmed||2018-07-25
CC||ramana at gcc dot gnu.org
Known to work||7.2.0
Summary|inline asm sometimes|[8/9 regression] inline asm
|ignores 'register |sometimes ignores 'register
|asm("reg")' declarations|asm("reg")' declarations
Ever confirmed|0 |1
Known to fail||8.1.0, 9.0
--- Comment #3 from Ramana Radhakrishnan ---
Confirmed.
