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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to