On Tue, May 12, 2009 at 03:45:16PM -0700, David Daney wrote: > If the TLB refill handler is too bit and needs to be split, there is > no need to branch around the split if the branch target would be an > eret. Since the eret returns from the handler, control flow never > passes it. A branch to an eret is equivalent to the eret itself. > > Signed-off-by: David Daney <ddaney@xxxxxxxxxxxxxxxxxx> > --- > arch/mips/mm/tlbex.c | 50 +++++++++++++++++++++++++++++++++----------------- > 1 files changed, 33 insertions(+), 17 deletions(-) > > diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c > index 4dc4f3e..ffa7104 100644 > --- a/arch/mips/mm/tlbex.c > +++ b/arch/mips/mm/tlbex.c > @@ -723,28 +731,36 @@ static void __cpuinit build_r4000_tlb_refill_handler(void) > uasm_copy_handler(relocs, labels, tlb_handler, p, f); > final_len = p - tlb_handler; > } else { > - u32 *split = tlb_handler + 30; > - > - /* > - * Find the split point. > - */ > - if (uasm_insn_has_bdelay(relocs, split - 1)) > - split--; > + u32 *split; > + if (split_on_eret) { > + split = tlb_handler + 32; > + } else { > + split = tlb_handler + 30; It would be a shame to pass up an opportunity to eliminate some of the pile of magic constants we have in the MIPS tree. Given that the MIPS documentation describes the size of the TLB Refill handler as 0x80 bytes, we could add something like: /* Maximum # of instructions in exception vector for TLB Refill handler */ #define MIPS64_TLB_REFILL_OPS (0x80 / sizeof(union mips_instruction)) and could change the last few lines of the code above to, for example: if (split_on_eret) { split = tlb_handler + MIPS64_TLB_REFILL_OPS; } else { split = tlb_handler + MIPS64_TLB_REFILL_OPS - 2; (you'd need to include asm/inst.h to get union mips_instruction defined) > + > + /* > + * Find the split point. > + */ > + if (uasm_insn_has_bdelay(relocs, split - 1)) > + split--; > + } The code itself makes sense. Does this case actually happen much, or was this just an itch? Reviewed-by: David VomLehn <dvomlehn@xxxxxxxxx>