[PATCH] PPC64 Poor assembly coding style
Linas Vepstas
linas at austin.ibm.com
Tue Nov 9 05:35:19 EST 2004
I wrote:
> Doug Maxey reported a bug with the latest/greatest gas assembler
> that demonstrates some poor coding style in entry.S and head.S.
> The following patch cleans up that style, and also avoids assembler
> confusion. Basically, in entry.S,
>
> cmpldi 0,r0,NR_syscalls should be written as either
>
> cmpldi r0,NR_syscalls or as cmpldi cr0,r0,NR_syscalls
>
> All three forms are theoretically equivalent; in practice,
> I find the first alternative the cleanest (and also consistent
> with usage elsewhere in the files).
>
> The new assembler seems to be mistaking NR_syscalls for a register
> number, which is clearly out of bounds (its not in 0..31).
> I think it would be cleaner overall to just drop the superfluous
> leading cr0. There are two other confusing usages, in head.S:
> I propose that
> cmpldi cr0,r5,0 should be cmpldi r5,0
> cmpld 0,r6,r5 should be cmpld r6,r5
After mailing this, a grep revealed that its not just cmpl
but in fact many cmp instructions that are sometimes written
one way, and sometimes another. The following larger patch
cleans up all the various usages, making (I beleive) the code
slightly easier to read.
--linas
Signed-off-by: Linas Vepstas <linas at linas.org>
-------------- next part --------------
===== arch/ppc64/kernel/entry.S 1.46 vs edited =====
--- 1.46/arch/ppc64/kernel/entry.S 2004-10-07 16:52:16 -05:00
+++ edited/arch/ppc64/kernel/entry.S 2004-11-08 12:18:08 -06:00
@@ -122,7 +122,7 @@ SystemCall_common:
andi. r11,r10,_TIF_SYSCALL_T_OR_A
bne- syscall_dotrace
syscall_dotrace_cont:
- cmpldi 0,r0,NR_syscalls
+ cmpldi r0,NR_syscalls
bge- syscall_enosys
system_call: /* label this so stack traces look sane */
@@ -204,7 +204,7 @@ syscall_enosys:
syscall_error:
lbz r11,TI_SC_NOERR(r12)
- cmpwi 0,r11,0
+ cmpwi r11,0
bne- syscall_error_cont
neg r3,r3
oris r5,r5,0x1000 /* Set SO bit in CR */
@@ -313,7 +313,7 @@ _GLOBAL(ppc32_rt_sigreturn)
_GLOBAL(ppc64_rt_sigreturn)
bl .sys_rt_sigreturn
-80: cmpdi 0,r3,0
+80: cmpdi r3,0
blt syscall_exit
clrrdi r4,r1,THREAD_SHIFT
ld r4,TI_FLAGS(r4)
@@ -488,7 +488,7 @@ _GLOBAL(ret_from_except_lite)
restore:
#ifdef CONFIG_PPC_ISERIES
ld r5,SOFTE(r1)
- cmpdi 0,r5,0
+ cmpdi r5,0
beq 4f
/* Check for pending interrupts (iSeries) */
ld r3,PACALPPACA+LPPACAANYINT(r13)
===== arch/ppc64/kernel/head.S 1.81 vs edited =====
--- 1.81/arch/ppc64/kernel/head.S 2004-10-19 02:18:43 -05:00
+++ edited/arch/ppc64/kernel/head.S 2004-11-08 12:19:46 -06:00
@@ -156,7 +156,7 @@ _GLOBAL(__secondary_hold)
/* All secondary cpu's wait here until told to start. */
100: ld r4,__secondary_hold_spinloop at l(0)
- cmpdi 0,r4,1
+ cmpdi r4,1
bne 100b
#ifdef CONFIG_HMT
@@ -326,7 +326,7 @@ label##_Iseries: \
mtspr SPRG1,r13; /* save r13 */ \
EXCEPTION_PROLOG_ISERIES_1(PACA_EXGEN); \
lbz r10,PACAPROCENABLED(r13); \
- cmpwi 0,r10,0; \
+ cmpwi r10,0; \
beq- label##_Iseries_masked; \
EXCEPTION_PROLOG_ISERIES_2; \
b label##_common; \
@@ -638,7 +638,7 @@ SystemReset_Iseries:
ori r24,r24,MSR_RI
mtmsrd r24 /* RI on */
lhz r24,PACAPACAINDEX(r13) /* Get processor # */
- cmpwi 0,r24,0 /* Are we processor 0? */
+ cmpwi r24,0 /* Are we processor 0? */
beq .__start_initialization_iSeries /* Start up the first processor */
mfspr r4,CTRLF
li r5,RUNLATCH /* Turn off the run light */
@@ -657,7 +657,7 @@ SystemReset_Iseries:
addi r1,r3,THREAD_SIZE
subi r1,r1,STACK_FRAME_OVERHEAD
- cmpwi 0,r23,0
+ cmpwi r23,0
beq iseries_secondary_smp_loop /* Loop until told to go */
#ifdef SECONDARY_PROCESSORS
bne .__secondary_start /* Loop until told to go */
@@ -1219,7 +1219,7 @@ _GLOBAL(pseries_secondary_smp_init)
ld r1,PACAEMERGSP(r13)
subi r1,r1,STACK_FRAME_OVERHEAD
- cmpwi 0,r23,0
+ cmpwi r23,0
#ifdef CONFIG_SMP
#ifdef SECONDARY_PROCESSORS
bne .__secondary_start
@@ -1303,7 +1303,7 @@ _GLOBAL(__start_initialization_multiplat
/*
* Are we booted from a PROM Of-type client-interface ?
*/
- cmpldi cr0,r5,0
+ cmpldi r5,0
bne .__boot_from_prom /* yes -> prom */
/* Save parameters */
@@ -1439,7 +1439,7 @@ _GLOBAL(copy_and_flush)
dcbst r6,r3 /* write it to memory */
sync
icbi r6,r3 /* flush the icache line */
- cmpld 0,r6,r5
+ cmpld r6,r5
blt 4b
sync
addi r5,r5,8
@@ -1472,7 +1472,7 @@ _STATIC(load_up_fpu)
#ifndef CONFIG_SMP
ld r3,last_task_used_math at got(r2)
ld r4,0(r3)
- cmpdi 0,r4,0
+ cmpdi r4,0
beq 1f
/* Save FP state to last_task_used_math's THREAD struct */
addi r4,r4,THREAD
@@ -1528,11 +1528,11 @@ _GLOBAL(giveup_fpu)
ori r5,r5,MSR_FP
mtmsrd r5 /* enable use of fpu now */
isync
- cmpdi 0,r3,0
+ cmpdi r3,0
beqlr- /* if no previous owner, done */
addi r3,r3,THREAD /* want THREAD of task */
ld r5,PT_REGS(r3)
- cmpdi 0,r5,0
+ cmpdi r5,0
SAVE_32FPRS(0, r3)
mffs fr0
stfd fr0,THREAD_FPSCR(r3)
@@ -1578,7 +1578,7 @@ _STATIC(load_up_altivec)
#ifndef CONFIG_SMP
ld r3,last_task_used_altivec at got(r2)
ld r4,0(r3)
- cmpdi 0,r4,0
+ cmpdi r4,0
beq 1f
/* Save VMX state to last_task_used_altivec's THREAD struct */
addi r4,r4,THREAD
@@ -1600,7 +1600,7 @@ _STATIC(load_up_altivec)
* all 1's
*/
mfspr r4,SPRN_VRSAVE
- cmpdi 0,r4,0
+ cmpdi r4,0
bne+ 1f
li r4,-1
mtspr SPRN_VRSAVE,r4
@@ -1647,11 +1647,11 @@ _GLOBAL(giveup_altivec)
oris r5,r5,MSR_VEC at h
mtmsrd r5 /* enable use of VMX now */
isync
- cmpdi 0,r3,0
+ cmpdi r3,0
beqlr- /* if no previous owner, done */
addi r3,r3,THREAD /* want THREAD of task */
ld r5,PT_REGS(r3)
- cmpdi 0,r5,0
+ cmpdi r5,0
SAVE_32VRS(0,r4,r3)
mfvscr vr0
li r4,THREAD_VSCR
===== arch/ppc64/kernel/misc.S 1.92 vs edited =====
--- 1.92/arch/ppc64/kernel/misc.S 2004-10-27 16:35:17 -05:00
+++ edited/arch/ppc64/kernel/misc.S 2004-11-08 12:21:40 -06:00
@@ -82,10 +82,10 @@ _GLOBAL(local_irq_disable)
_GLOBAL(local_irq_restore)
lbz r5,PACAPROCENABLED(r13)
/* Check if things are setup the way we want _already_. */
- cmpw 0,r3,r5
+ cmpw r3,r5
beqlr
/* are we enabling interrupts? */
- cmpdi 0,r3,0
+ cmpdi r3,0
stb r3,PACAPROCENABLED(r13)
beqlr
/* Check pending interrupts */
@@ -374,7 +374,7 @@ _GLOBAL(__flush_dcache_icache)
* The *_ns versions don't do byte-swapping.
*/
_GLOBAL(_insb)
- cmpwi 0,r5,0
+ cmpwi r5,0
mtctr r5
subi r4,r4,1
blelr-
@@ -387,7 +387,7 @@ _GLOBAL(_insb)
blr
_GLOBAL(_outsb)
- cmpwi 0,r5,0
+ cmpwi r5,0
mtctr r5
subi r4,r4,1
blelr-
@@ -398,7 +398,7 @@ _GLOBAL(_outsb)
blr
_GLOBAL(_insw)
- cmpwi 0,r5,0
+ cmpwi r5,0
mtctr r5
subi r4,r4,2
blelr-
@@ -411,7 +411,7 @@ _GLOBAL(_insw)
blr
_GLOBAL(_outsw)
- cmpwi 0,r5,0
+ cmpwi r5,0
mtctr r5
subi r4,r4,2
blelr-
@@ -422,7 +422,7 @@ _GLOBAL(_outsw)
blr
_GLOBAL(_insl)
- cmpwi 0,r5,0
+ cmpwi r5,0
mtctr r5
subi r4,r4,4
blelr-
@@ -435,7 +435,7 @@ _GLOBAL(_insl)
blr
_GLOBAL(_outsl)
- cmpwi 0,r5,0
+ cmpwi r5,0
mtctr r5
subi r4,r4,4
blelr-
@@ -447,7 +447,7 @@ _GLOBAL(_outsl)
/* _GLOBAL(ide_insw) now in drivers/ide/ide-iops.c */
_GLOBAL(_insw_ns)
- cmpwi 0,r5,0
+ cmpwi r5,0
mtctr r5
subi r4,r4,2
blelr-
@@ -461,7 +461,7 @@ _GLOBAL(_insw_ns)
/* _GLOBAL(ide_outsw) now in drivers/ide/ide-iops.c */
_GLOBAL(_outsw_ns)
- cmpwi 0,r5,0
+ cmpwi r5,0
mtctr r5
subi r4,r4,2
blelr-
@@ -472,7 +472,7 @@ _GLOBAL(_outsw_ns)
blr
_GLOBAL(_insl_ns)
- cmpwi 0,r5,0
+ cmpwi r5,0
mtctr r5
subi r4,r4,4
blelr-
@@ -485,7 +485,7 @@ _GLOBAL(_insl_ns)
blr
_GLOBAL(_outsl_ns)
- cmpwi 0,r5,0
+ cmpwi r5,0
mtctr r5
subi r4,r4,4
blelr-
@@ -526,7 +526,7 @@ _GLOBAL(identify_cpu)
lwz r8,CPU_SPEC_PVR_MASK(r3)
and r8,r8,r7
lwz r9,CPU_SPEC_PVR_VALUE(r3)
- cmplw 0,r9,r8
+ cmplw r9,r8
beq 1f
addi r3,r3,CPU_SPEC_ENTRY_SIZE
b 1b
@@ -670,7 +670,7 @@ _GLOBAL(kernel_thread)
li r4,0 /* new sp (unused) */
li r0,__NR_clone
sc
- cmpdi 0,r3,0 /* parent or child? */
+ cmpdi r3,0 /* parent or child? */
bne 1f /* return if parent */
li r0,0
stdu r0,-STACK_FRAME_OVERHEAD(r1)
More information about the Linuxppc64-dev
mailing list