Messages in this thread | | | Date | Tue, 24 Jul 2012 17:40:16 +0100 | From | Catalin Marinas <> | Subject | Re: [07/36] AArch64: Assembly macros and definitions |
| |
On Fri, Jul 20, 2012 at 03:22:04PM +0100, Christopher Covington wrote: > On 01/-10/-28163 02:59 PM, Catalin Marinas wrote: > > This patch introduces several assembly macros and definitions used in > > the .S files across arch/aarch64/ like IRQ disabling/enabling, together > > with asm-offsets.c. > > [...] > > > diff --git a/arch/aarch64/include/asm/assembler.h b/arch/aarch64/include/asm/assembler.h > > new file mode 100644 > > index 0000000..c6ac3cf > > --- /dev/null > > +++ b/arch/aarch64/include/asm/assembler.h > > [...] > > > +#define USER(x...) \ > > +9999: x; \ > > + .section __ex_table,"a"; \ > > + .align 3; \ > > + .quad 9999b,9001f; \ > > + .previous > > It appears that this macro depends on a 9001 label being defined in the > source file somewhere after the macro is used. It might be preferable to > incorporate the label into the macro if possible, or pass the label as > an argument to the macro.
Good point, I changed to USER(l, x...).
> > +/* > > + * User access macros. > > + */ > > + .macro usracc, instr, reg, reg2, ptr, inc, rept, abort > > + .rept \rept > > +9999: > > + .if \inc == 1 > > + \instr\()b \reg, [\ptr], #\inc > > + .elseif \inc == 4 > > + \instr\() \reg, [\ptr], #\inc > > + .elseif \inc == 8 > > + \instr\() \reg, [\ptr], #\inc > > + .elseif \inc == 16 > > + \instr\() \reg, \reg2, [\ptr], #\inc > > + .else > > + .error "Unsupported inc macro argument" > > + .endif > > + > > + .section __ex_table,"a" > > + .align 3 > > + .quad 9999b, \abort > > + .previous > > Could you use the USER preprocessor macro here to reduce this small > duplication of directives? > > > + .endr > > + .endm > > + > > + .macro ldrusr, reg, ptr, inc, rept=1, abort=9001f > > + usracc ldr, \reg, \reg, \ptr, \inc, \rept, \abort > > + .endm > > + > > + .macro ldrpusr, reg, reg2, ptr, rept=1, abort=9001f > > + usracc ldp, \reg, \reg2, \ptr, 16, \rept, \abort > > + .endm > > How about "ldpusr" to more directly match the assembly? > > (Also, the 9001 label strikes again.)
For now I removed these entirely. I have additional patches for optimised library functions that make use of these but I don't plan to push any of them until I can benchmark on real hardware.
Thanks.
-- Catalin
| |