On Fri, Sep 29, 2017 at 04:26:31PM +0100, Maciej W. Rozycki wrote:
> Optimize `__read_64bit_c0_split' and reduce the instruction count by 1,
> observing that a DSLL/DSRA pair by 32, is equivalent to SLL by 0, which
> architecturally truncates the value requested to 32 bits on 64-bit MIPS
> hardware regardless of whether the input operand is or is not a properly
> sign-extended 32-bit value.
>
> Signed-off-by: Maciej W. Rozycki <macro@xxxxxxxxxx>
> ---
> Tested by compilation only to verify syntax correctnes as I do not know
> if this execution path is actually used by any configuration (suggestions
> welcome). I believe it to be technically correct though, being
> sufficiently straightforward to verify by proofreading, and an obvious
> improvement.
Agreed, I did something similar locally too but didn't bother to submit
it :).
>
> Therefore, please apply.
>
> NB if this turns out indeed used, then we might have to do something
> about DMFC0 hazard avoidance for the sake of MIPS III support, and also
> choose to use an MFC0/MFHC0 instruction pair instead on MIPS64r5+.
This would have to depend on MVH bit, but in practice I suspect it isn't
worthwhile doing it here instead of in a separate macro to use depending
on the register.
Using MVH would have the advantage of avoiding the potential window when
a 32-bit EJTAG or Cache error handler potentially canonicalises register
state I suppose.
That's another advantage of this patch actually, it reduces the size of
that window to a single instruction.
Reviewed-by: James Hogan <james.hogan@xxxxxxxxxx>
Cheers
James
>
> Maciej
>
> ---
> arch/mips/include/asm/mipsregs.h | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
> linux-mips-read-64bit-c0-split-sll.diff
> Index: linux-sfr-test/arch/mips/include/asm/mipsregs.h
> ===================================================================
> --- linux-sfr-test.orig/arch/mips/include/asm/mipsregs.h 2017-07-08 15:32:02.000000000 +0100
> +++ linux-sfr-test/arch/mips/include/asm/mipsregs.h 2017-09-29 01:02:01.390974000 +0100
> @@ -1344,19 +1344,17 @@ do { \
> if (sel == 0) \
> __asm__ __volatile__( \
> ".set\tmips64\n\t" \
> - "dmfc0\t%M0, " #source "\n\t" \
> - "dsll\t%L0, %M0, 32\n\t" \
> - "dsra\t%M0, %M0, 32\n\t" \
> - "dsra\t%L0, %L0, 32\n\t" \
> + "dmfc0\t%L0, " #source "\n\t" \
> + "dsra\t%M0, %L0, 32\n\t" \
> + "sll\t%L0, %L0, 0\n\t" \
> ".set\tmips0" \
> : "=r" (__val)); \
> else \
> __asm__ __volatile__( \
> ".set\tmips64\n\t" \
> - "dmfc0\t%M0, " #source ", " #sel "\n\t" \
> - "dsll\t%L0, %M0, 32\n\t" \
> - "dsra\t%M0, %M0, 32\n\t" \
> - "dsra\t%L0, %L0, 32\n\t" \
> + "dmfc0\t%L0, " #source ", " #sel "\n\t" \
> + "dsra\t%M0, %L0, 32\n\t" \
> + "sll\t%L0, %L0, 0\n\t" \
> ".set\tmips0" \
> : "=r" (__val)); \
> local_irq_restore(__flags); \
Attachment:
signature.asc
Description: Digital signature
