Hi Bastian,
> Hi David,
>
> On 6/5/19 8:11 AM, David Brenken wrote:
> > From: David Brenken <david.bren...@efs-auto.de>
> >
> > Signed-off-by: Andreas Konopik <andreas.kono...@efs-auto.de>
> > Signed-off-by: David Brenken <david.bren...@efs-auto.de>
> > Signed-off-by: Georg Hofstetter <georg.hofstet...@efs-auto.de>
> > Signed-off-by: Robert Rasche <robert.ras...@efs-auto.de>
> > Signed-off-by: Lars Biermanski <lars.bierman...@efs-auto.de>
> >
> > ---
> > target/tricore/translate.c | 11 ++++++++---
> > 1 file changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/target/tricore/translate.c b/target/tricore/translate.c
> > index a8b4de647a..19d8db7a46 100644
> > --- a/target/tricore/translate.c
> > +++ b/target/tricore/translate.c
> > @@ -7004,6 +7004,7 @@ static void
> decode_rrpw_extract_insert(CPUTriCoreState *env, DisasContext *ctx)
> > uint32_t op2;
> > int r1, r2, r3;
> > int32_t pos, width;
> > + TCGv temp1, temp2;
> >
> > op2 = MASK_OP_RRPW_OP2(ctx->opcode);
> > r1 = MASK_OP_RRPW_S1(ctx->opcode);
> > @@ -7042,9 +7043,13 @@ static void
> decode_rrpw_extract_insert(CPUTriCoreState *env, DisasContext *ctx)
> > }
> > break;
> > case OPC2_32_RRPW_INSERT:
> > - if (pos + width <= 31) {
> > - tcg_gen_deposit_tl(cpu_gpr_d[r3], cpu_gpr_d[r1], cpu_gpr_d[r2],
> > - width, pos);
> Can you explain the problem causing the bug? Deposit looks fine to me.
> After reading the specs again, I agree that the check needs to be <= 32.
The bug was recognized because of different behavior between actual hardware
and QEMU.
Just from looking at it I would say that deposit masks and then shifts the arg2
(D[b]) while the
manual states to first shift D[b] and then mask it. I remember that it was a
corner case (e.g.
width + pos = 31 or 32).
I also thought that using the direct insert instruction is more convenient
since it was present anyway.
>
>
> > + if (pos + width <= 32) {
> > + temp1 = tcg_const_i32(pos); /* pos */
> > + temp2 = tcg_const_i32(width); /* width */
> Useless comments.
I will remove them in a second version of the patchset.
>
>
> Cheers,
>
> Bastian
Best regards
David
