| Message ID | 20210305132629.755627-3-kbastian@mail.uni-paderborn.de |
|---|---|
| State | New |
| Headers | show |
| Series | tricore: IMASK/EXTR corner case fixes | expand |
On 3/5/21 2:26 PM, Bastian Koppelmann wrote: > if width was 0 we would run into the assertion: > > qemu-system-tricore: ../upstream/tcg/tcg-op.c:217: tcg_gen_sari_i32: Assertion `arg2 >= 0 && arg2 < 32' failed.o Maybe strip "../upstream/" > > The instruction manuel specifies undefined behaviour for this case. So "manual" > we bring this in line with the golden Infineon simlator 'tsim', which > simply writes 0 to the result in case of width=0. > > Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de> > --- > target/tricore/translate.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/target/tricore/translate.c b/target/tricore/translate.c > index 67a7f646a2..d8b773ab37 100644 > --- a/target/tricore/translate.c > +++ b/target/tricore/translate.c > @@ -6998,10 +6998,16 @@ static void decode_rrpw_extract_insert(DisasContext *ctx) > pos = MASK_OP_RRPW_POS(ctx->opcode); > width = MASK_OP_RRPW_WIDTH(ctx->opcode); > > + Spurious change? > switch (op2) { > case OPC2_32_RRPW_EXTR: > + if (width == 0) { > + tcg_gen_movi_tl(cpu_gpr_d[r3], 0); > + break; > + } > + > if (pos + width <= 32) { > - /* optimize special cases */ > + /* optimize special cases */ Spurious change? > if ((pos == 0) && (width == 8)) { > tcg_gen_ext8s_tl(cpu_gpr_d[r3], cpu_gpr_d[r1]); > } else if ((pos == 0) && (width == 16)) { > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
On 3/5/21 5:26 AM, Bastian Koppelmann wrote: > case OPC2_32_RRPW_EXTR: > + if (width == 0) { > + tcg_gen_movi_tl(cpu_gpr_d[r3], 0); > + break; > + } > + > if (pos + width <= 32) { > - /* optimize special cases */ > + /* optimize special cases */ > if ((pos == 0) && (width == 8)) { Indentation is definitely off, both with the unrelated comment change Phil pointed out, and in the new code block. Otherwise, Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
diff --git a/target/tricore/translate.c b/target/tricore/translate.c index 67a7f646a2..d8b773ab37 100644 --- a/target/tricore/translate.c +++ b/target/tricore/translate.c @@ -6998,10 +6998,16 @@ static void decode_rrpw_extract_insert(DisasContext *ctx) pos = MASK_OP_RRPW_POS(ctx->opcode); width = MASK_OP_RRPW_WIDTH(ctx->opcode); + switch (op2) { case OPC2_32_RRPW_EXTR: + if (width == 0) { + tcg_gen_movi_tl(cpu_gpr_d[r3], 0); + break; + } + if (pos + width <= 32) { - /* optimize special cases */ + /* optimize special cases */ if ((pos == 0) && (width == 8)) { tcg_gen_ext8s_tl(cpu_gpr_d[r3], cpu_gpr_d[r1]); } else if ((pos == 0) && (width == 16)) {
if width was 0 we would run into the assertion: qemu-system-tricore: ../upstream/tcg/tcg-op.c:217: tcg_gen_sari_i32: Assertion `arg2 >= 0 && arg2 < 32' failed.o The instruction manuel specifies undefined behaviour for this case. So we bring this in line with the golden Infineon simlator 'tsim', which simply writes 0 to the result in case of width=0. Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de> --- target/tricore/translate.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)