Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

invalid register name and invalid operand for instruction in arch/powerpc/kernel/vdso{32,64}/gettimeofday.S #764

Closed
tpimh opened this issue Nov 2, 2019 · 9 comments
Assignees
Labels
[ARCH] powerpc This bug impacts ARCH=powerpc [BUG] llvm A bug that should be fixed in upstream LLVM [FIXED][LLVM] 12 This bug was fixed in LLVM 12.0 [TOOL] integrated-as The issue is relevant to LLVM integrated assembler

Comments

@tpimh
Copy link

tpimh commented Nov 2, 2019

Build of ppc32 and ppc64 kernels with integrated-as enabled failing:

arch/powerpc/kernel/vdso32/gettimeofday.S:34:20: error: invalid register name
  .cfi_register lr,%r12
                   ^
arch/powerpc/kernel/vdso32/gettimeofday.S:71:10: error: invalid operand for instruction
 cmpli 0,%r3,0
         ^
arch/powerpc/kernel/vdso32/gettimeofday.S:72:10: error: invalid operand for instruction
 cmpli 1,%r3,1
         ^
arch/powerpc/kernel/vdso32/gettimeofday.S:77:20: error: invalid register name
  .cfi_register lr,%r12
                   ^
arch/powerpc/kernel/vdso32/gettimeofday.S:107:16: error: invalid operand for instruction
        cmpl 0,%r8,%r0
               ^
arch/powerpc/kernel/vdso32/gettimeofday.S:160:10: error: invalid operand for instruction
 cmpli 0,%r4,0
         ^
arch/powerpc/kernel/vdso32/gettimeofday.S:189:20: error: invalid register name
  .cfi_register lr,%r12
                   ^

Build of ppc64le kernel with integrated-as enabled failing:

arch/powerpc/kernel/vdso64/gettimeofday.S:25:20: error: invalid register name
  .cfi_register lr,%r12
                   ^
arch/powerpc/kernel/vdso64/gettimeofday.S:72:20: error: invalid register name
  .cfi_register lr,%r12
                   ^
arch/powerpc/kernel/vdso64/gettimeofday.S:218:20: error: invalid register name
  .cfi_register lr,%r12
                   ^
@tpimh tpimh added [BUG] llvm A bug that should be fixed in upstream LLVM [TOOL] integrated-as The issue is relevant to LLVM integrated assembler low priority This bug is not critical and not a priority [ARCH] powerpc This bug impacts ARCH=powerpc labels Nov 2, 2019
@tpimh tpimh changed the title invalid register name and invalid operand for instruction in arch/powerpc/kernel/vdso32/gettimeofday.S invalid register name and invalid operand for instruction in arch/powerpc/kernel/vdso{32,64}/gettimeofday.S Nov 2, 2019
@nickdesaulniers
Copy link
Member

https://developer.ibm.com/articles/l-ppc/ makes it sound like the %r convention is unconventional.
https://godbolt.org/z/Yme6zt

cc @arndb

for cmpli, looks like Clang wasn't expecting a register operand, I suppose.

@tpimh tpimh removed the low priority This bug is not critical and not a priority label Feb 9, 2020
@daxtens
Copy link

daxtens commented Feb 24, 2021

It looks like some of this might be resolved by b2f663073917babb84f5ba9129451e8d564cba1a ("[PowerPC] Allow a '%' prefix for registers in CFI directives"). I'm building llvm from git now to have a look.

@daxtens
Copy link

daxtens commented Feb 24, 2021

However, it seems a bigger issue might be clang -integrated-as not liking e.g. mulld r3, r3, r4 and only accepting mulld 3, 3, 4; ppc linux asm is exclusively written with the r prefix. Weirdly my reading of llvm/lib/Target/PowerPC/AsmParser/PPCAsmParser.cpp is that it should recognise the r prefix, and should have done since 2013, so maybe I'm misunderstanding how all the pieces fit together...

@daxtens
Copy link

daxtens commented Feb 24, 2021

@cmriedl points out that this could be due to llvm's assembler not supporting -mregnames: gnu as also doesn't like the r prefix unless you pass that option.

@daxtens
Copy link

daxtens commented Feb 24, 2021

@mpe pointed out that our register names are actually #defined, so llvm is actually rejecting %r0, %r1 etc, which is even odder. However, I can work around this for now by just changing the defines, for example #define r0 0.

@nickdesaulniers
Copy link
Member

Yes, from my godbolt link, it looks like .cfi_directive is fixed, and cmpli is generally broken (regardless of the % or not). So this is two distinct issues.

fengguang pushed a commit to 0day-ci/linux that referenced this issue Feb 25, 2021
This is dumb but makes the llvm integrated assembler happy.
ClangBuiltLinux#764

Signed-off-by: Daniel Axtens <dja@axtens.net>
ruscur pushed a commit to ruscur/linux that referenced this issue Feb 25, 2021
This is dumb but makes the llvm integrated assembler happy.
ClangBuiltLinux#764

Signed-off-by: Daniel Axtens <dja@axtens.net>
ruscur pushed a commit to ruscur/linux that referenced this issue Feb 25, 2021
This is dumb but makes the llvm integrated assembler happy.
ClangBuiltLinux#764

Signed-off-by: Daniel Axtens <dja@axtens.net>
ruscur pushed a commit to ruscur/linux that referenced this issue Feb 25, 2021
This is dumb but makes the llvm integrated assembler happy.
ClangBuiltLinux#764

Signed-off-by: Daniel Axtens <dja@axtens.net>
ruscur pushed a commit to ruscur/linux that referenced this issue Feb 25, 2021
This is dumb but makes the llvm integrated assembler happy.
ClangBuiltLinux#764

Signed-off-by: Daniel Axtens <dja@axtens.net>
aik pushed a commit to aik/linux that referenced this issue Apr 15, 2021
This is dumb but makes the llvm integrated assembler happy.
ClangBuiltLinux#764

Signed-off-by: Daniel Axtens <dja@axtens.net>
@nickdesaulniers
Copy link
Member

cmpli is #1419 . Let's use this issue to track .cfi_register.

@nemanjai
Copy link

The .cfi_register issue was fixed on Nov. 18, 2020 with commit b2f663073917b.

@nickdesaulniers
Copy link
Member

Indeed, I can no longer reproduce with:

$ ARCH=powerpc make LLVM=1 -j72 pseries_defconfig arch/powerpc/kernel/vdso32/
# or
$ ARCH=powerpc make LLVM=1 -j72 ppc44x_defconfig arch/powerpc/kernel/vdso32/
# or
$ ARCH=powerpc make LLVM=1 -j72 powernv_defconfig arch/powerpc/kernel/vdso32/

(working around #1484, #1418 and #672).

The .cfi_register issue was fixed on Nov. 18, 2020 with commit b2f663073917b.

lol, I reviewed that patch...forgot about this issue. Looks like this shipped in clang-12. Thanks @gwelymernans !

@nickdesaulniers nickdesaulniers added the [FIXED][LLVM] 12 This bug was fixed in LLVM 12.0 label Oct 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[ARCH] powerpc This bug impacts ARCH=powerpc [BUG] llvm A bug that should be fixed in upstream LLVM [FIXED][LLVM] 12 This bug was fixed in LLVM 12.0 [TOOL] integrated-as The issue is relevant to LLVM integrated assembler
Projects
None yet
Development

No branches or pull requests

5 participants