| Message ID | 20230215084754.3816747-2-marcin.nowakowski@fungible.com |
|---|---|
| State | New |
| Headers | show |
| Series | target/mips: misc microMIPS fixes | expand |
On 2/14/23 22:47, Marcin Nowakowski wrote: > @@ -4860,6 +4860,7 @@ static void gen_compute_branch(DisasContext *ctx, uint32_t opc, > target_ulong btgt = -1; > int blink = 0; > int bcond_compute = 0; > + int jal_mask = 0; Better to limit the scope of the variable to the block below. > @@ -4917,6 +4918,11 @@ static void gen_compute_branch(DisasContext *ctx, uint32_t opc, > break; > case OPC_J: > case OPC_JAL: > + /* Jump to immediate */ > + jal_mask = ctx->hflags & MIPS_HFLAG_M16 ? 0xF8000000 : 0xF0000000; > + btgt = ((ctx->base.pc_next + insn_bytes) & jal_mask) | > + (uint32_t)offset; Ideally we wouldn't have one huge helper function, and could pass down the mask from the translator. But that's on-going cleanup. Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
On 15/2/23 21:21, Richard Henderson wrote: > On 2/14/23 22:47, Marcin Nowakowski wrote: >> @@ -4860,6 +4860,7 @@ static void gen_compute_branch(DisasContext >> *ctx, uint32_t opc, >> target_ulong btgt = -1; >> int blink = 0; >> int bcond_compute = 0; >> + int jal_mask = 0; > > Better to limit the scope of the variable to the block below. > >> @@ -4917,6 +4918,11 @@ static void gen_compute_branch(DisasContext >> *ctx, uint32_t opc, >> break; >> case OPC_J: >> case OPC_JAL: >> + /* Jump to immediate */ >> + jal_mask = ctx->hflags & MIPS_HFLAG_M16 ? 0xF8000000 : >> 0xF0000000; >> + btgt = ((ctx->base.pc_next + insn_bytes) & jal_mask) | >> + (uint32_t)offset; > > Ideally we wouldn't have one huge helper function, and could pass down > the mask from the translator. But that's on-going cleanup. Yes, this is the approach taken in decodetree conversion. I hope to rebase / respin incorporating Jiaxun patches some day... > Reviewed-by: Richard Henderson <richard.henderson@linaro.org> > > > r~
在2023年2月15日二月 下午8:50，Philippe Mathieu-Daudé写道： > On 15/2/23 21:21, Richard Henderson wrote: >> On 2/14/23 22:47, Marcin Nowakowski wrote: >>> @@ -4860,6 +4860,7 @@ static void gen_compute_branch(DisasContext >>> *ctx, uint32_t opc, >>> target_ulong btgt = -1; >>> int blink = 0; >>> int bcond_compute = 0; >>> + int jal_mask = 0; >> >> Better to limit the scope of the variable to the block below. >> >>> @@ -4917,6 +4918,11 @@ static void gen_compute_branch(DisasContext >>> *ctx, uint32_t opc, >>> break; >>> case OPC_J: >>> case OPC_JAL: >>> + /* Jump to immediate */ >>> + jal_mask = ctx->hflags & MIPS_HFLAG_M16 ? 0xF8000000 : >>> 0xF0000000; >>> + btgt = ((ctx->base.pc_next + insn_bytes) & jal_mask) | >>> + (uint32_t)offset; >> >> Ideally we wouldn't have one huge helper function, and could pass down >> the mask from the translator. But that's on-going cleanup. > > Yes, this is the approach taken in decodetree conversion. > > I hope to rebase / respin incorporating Jiaxun patches some day... Which series are you referring? Just caught some time so I might able to help. > >> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> >> >> >> r~
diff --git a/target/mips/tcg/translate.c b/target/mips/tcg/translate.c index 624e6b7786..5d46f24141 100644 --- a/target/mips/tcg/translate.c +++ b/target/mips/tcg/translate.c @@ -4860,6 +4860,7 @@ static void gen_compute_branch(DisasContext *ctx, uint32_t opc, target_ulong btgt = -1; int blink = 0; int bcond_compute = 0; + int jal_mask = 0; TCGv t0 = tcg_temp_new(); TCGv t1 = tcg_temp_new(); @@ -4917,6 +4918,11 @@ static void gen_compute_branch(DisasContext *ctx, uint32_t opc, break; case OPC_J: case OPC_JAL: + /* Jump to immediate */ + jal_mask = ctx->hflags & MIPS_HFLAG_M16 ? 0xF8000000 : 0xF0000000; + btgt = ((ctx->base.pc_next + insn_bytes) & jal_mask) | + (uint32_t)offset; + break; case OPC_JALX: /* Jump to immediate */ btgt = ((ctx->base.pc_next + insn_bytes) & (int32_t)0xF0000000) |
microMIPS J & JAL instructions perform a jump in a 128MB region and 5 top bits of the address need to be preserved. This is different behavior compared to standard mips systems, where the jump is executed within a 256MB region. Note that microMIPS32 instruction set documentation appears to have inconsistent information regarding JALX32 instruction - it is written in the doc that: "To execute a procedure call within the current 256 MB-aligned region (...) The low 26 bits of the target address is the target field shifted left 2 bits." But the target address is already 26 bits. Moreover, the operation description indicates that 28 bits are copied, so the statement about use of 26 bits is _most likely_ incorrect and the corresponding code remains the same as for standard mips instruction set. Signed-off-by: Marcin Nowakowski <marcin.nowakowski@fungible.com> --- target/mips/tcg/translate.c | 6 ++++++ 1 file changed, 6 insertions(+)