| Message ID | 20210225031006.1204774-5-dja@axtens.net (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
| Series | WIP support for the LLVM integrated assembler | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (626a6c3d2e20da80aaa710104f34ea6037b28b33) |
| snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 70 lines checked |
| snowpatch_ozlabs/needsstable | success | Patch has no Fixes tags |
On Thu, Feb 25, 2021 at 02:10:02PM +1100, Daniel Axtens wrote: > This is dumb but makes the llvm integrated assembler happy. > https://github.com/ClangBuiltLinux/linux/issues/764 > -#define r0 %r0 > +#define r0 0 This is a big step back (compare 9a13a524ba37). If you use a new enough GAS, you can use the -mregnames option and just say "r0" directly (so not define it at all, or define it to itself). === addi 3,3,3 addi r3,r3,3 addi %r3,%r3,3 addi 3,3,3 addi r3,r3,r3 addi %r3,%r3,%r3 === $ as t.s -o t.o -mregnames t.s: Assembler messages: t.s:6: Warning: invalid register expression t.s:7: Warning: invalid register expression Many people do not like bare numbers. It is a bit like not wearing seatbelts (but so is all assembler code really: you just have to pay attention). A better argument is that it is harder to read for people not used to assembler code like this. We used to have "#define r0 0" etc., and that was quite problematic. Like that "addi r3,r3,r3" example, but also, people wrote "r0" where only a plain 0 is allowed (like in "lwzx r3,0,r3": "r0" would be misleading there!) Segher
Segher Boessenkool <segher@kernel.crashing.org> writes: > On Thu, Feb 25, 2021 at 02:10:02PM +1100, Daniel Axtens wrote: >> This is dumb but makes the llvm integrated assembler happy. >> https://github.com/ClangBuiltLinux/linux/issues/764 > >> -#define r0 %r0 > >> +#define r0 0 > > This is a big step back (compare 9a13a524ba37). > > If you use a new enough GAS, you can use the -mregnames option and just > say "r0" directly (so not define it at all, or define it to itself). > > === > addi 3,3,3 > addi r3,r3,3 > addi %r3,%r3,3 > > addi 3,3,3 > addi r3,r3,r3 > addi %r3,%r3,%r3 > === > > $ as t.s -o t.o -mregnames > t.s: Assembler messages: > t.s:6: Warning: invalid register expression > t.s:7: Warning: invalid register expression > > > Many people do not like bare numbers. It is a bit like not wearing > seatbelts (but so is all assembler code really: you just have to pay > attention). A better argument is that it is harder to read for people > not used to assembler code like this. > > We used to have "#define r0 0" etc., and that was quite problematic. > Like that "addi r3,r3,r3" example, but also, people wrote "r0" where > only a plain 0 is allowed (like in "lwzx r3,0,r3": "r0" would be > misleading there!) So an overarching comment on all of these patches is that they're not intended to be ready to merge, nor are they necessarily what I think is the best solution. I'm just swinging a big hammer to see how far towards LLVM_IAS=1 I can get on powerpc, and I accept I'm going to have to come back and clean things up. Anyway, noted, I'll push harder on trying to get llvm to accept %rN: there was a patch that went in after llvm-11 that should help. Kind regards, Daniel > > > Segher
Excerpts from Daniel Axtens's message of February 26, 2021 10:12 am: > Segher Boessenkool <segher@kernel.crashing.org> writes: > >> On Thu, Feb 25, 2021 at 02:10:02PM +1100, Daniel Axtens wrote: >>> This is dumb but makes the llvm integrated assembler happy. >>> https://github.com/ClangBuiltLinux/linux/issues/764 >> >>> -#define r0 %r0 >> >>> +#define r0 0 >> >> This is a big step back (compare 9a13a524ba37). >> >> If you use a new enough GAS, you can use the -mregnames option and just >> say "r0" directly (so not define it at all, or define it to itself). >> >> === >> addi 3,3,3 >> addi r3,r3,3 >> addi %r3,%r3,3 >> >> addi 3,3,3 >> addi r3,r3,r3 >> addi %r3,%r3,%r3 >> === >> >> $ as t.s -o t.o -mregnames >> t.s: Assembler messages: >> t.s:6: Warning: invalid register expression >> t.s:7: Warning: invalid register expression >> >> >> Many people do not like bare numbers. It is a bit like not wearing >> seatbelts (but so is all assembler code really: you just have to pay >> attention). A better argument is that it is harder to read for people >> not used to assembler code like this. >> >> We used to have "#define r0 0" etc., and that was quite problematic. >> Like that "addi r3,r3,r3" example, but also, people wrote "r0" where >> only a plain 0 is allowed (like in "lwzx r3,0,r3": "r0" would be >> misleading there!) > > So an overarching comment on all of these patches is that they're not > intended to be ready to merge, nor are they necessarily what I think is > the best solution. I'm just swinging a big hammer to see how far towards > LLVM_IAS=1 I can get on powerpc, and I accept I'm going to have to come > back and clean things up. > > Anyway, noted, I'll push harder on trying to get llvm to accept %rN: > there was a patch that went in after llvm-11 that should help. If you put it under ifdef CONFIG_CC_IS_CLANG in the meantime I think that would be okay. Then we get error checking with gcc compiles and llvm at least builds with its assembler which would be nice. Thanks, Nick
diff --git a/arch/powerpc/include/asm/ppc_asm.h b/arch/powerpc/include/asm/ppc_asm.h index 3dceb64fc9af..49da2cf4c2d5 100644 --- a/arch/powerpc/include/asm/ppc_asm.h +++ b/arch/powerpc/include/asm/ppc_asm.h @@ -509,38 +509,38 @@ END_FTR_SECTION_NESTED(CPU_FTR_CELL_TB_BUG, CPU_FTR_CELL_TB_BUG, 96) * Use R0-31 only when really nessesary. */ -#define r0 %r0 -#define r1 %r1 -#define r2 %r2 -#define r3 %r3 -#define r4 %r4 -#define r5 %r5 -#define r6 %r6 -#define r7 %r7 -#define r8 %r8 -#define r9 %r9 -#define r10 %r10 -#define r11 %r11 -#define r12 %r12 -#define r13 %r13 -#define r14 %r14 -#define r15 %r15 -#define r16 %r16 -#define r17 %r17 -#define r18 %r18 -#define r19 %r19 -#define r20 %r20 -#define r21 %r21 -#define r22 %r22 -#define r23 %r23 -#define r24 %r24 -#define r25 %r25 -#define r26 %r26 -#define r27 %r27 -#define r28 %r28 -#define r29 %r29 -#define r30 %r30 -#define r31 %r31 +#define r0 0 +#define r1 1 +#define r2 2 +#define r3 3 +#define r4 4 +#define r5 5 +#define r6 6 +#define r7 7 +#define r8 8 +#define r9 9 +#define r10 10 +#define r11 11 +#define r12 12 +#define r13 13 +#define r14 14 +#define r15 15 +#define r16 16 +#define r17 17 +#define r18 18 +#define r19 19 +#define r20 20 +#define r21 21 +#define r22 22 +#define r23 23 +#define r24 24 +#define r25 25 +#define r26 26 +#define r27 27 +#define r28 28 +#define r29 29 +#define r30 30 +#define r31 31 /* Floating Point Registers (FPRs) */
This is dumb but makes the llvm integrated assembler happy. https://github.com/ClangBuiltLinux/linux/issues/764 Signed-off-by: Daniel Axtens <dja@axtens.net> --- arch/powerpc/include/asm/ppc_asm.h | 64 +++++++++++++++--------------- 1 file changed, 32 insertions(+), 32 deletions(-)