Message ID | 20211019094812.614056-12-frederic.petrot@univ-grenoble-alpes.fr |
---|---|
State | New |
Headers | show |
Series | Adding partial support for 128-bit riscv target | expand |
On 10/19/21 2:48 AM, Frédéric Pétrot wrote: > The 128-bit bitwise instructions do not need any function prototype change > as the functions can be applied independently on the lower and upper part of > the registers. > > Signed-off-by: Frédéric Pétrot <frederic.petrot@univ-grenoble-alpes.fr> > Co-authored-by: Fabien Portas <fabien.portas@grenoble-inp.org> > --- > target/riscv/translate.c | 22 ++++++++++++++++++++++ > 1 file changed, 22 insertions(+) > > diff --git a/target/riscv/translate.c b/target/riscv/translate.c > index e8f08f921e..71982f6284 100644 > --- a/target/riscv/translate.c > +++ b/target/riscv/translate.c > @@ -429,6 +429,17 @@ static bool gen_logic_imm_fn(DisasContext *ctx, arg_i *a, DisasExtend ext, > > gen_set_gpr(ctx, a->rd, dest); > > + if (get_xl_max(ctx) == MXL_RV128) { > + if (get_ol(ctx) == MXL_RV128) { > + uint64_t immh = -(a->imm < 0); > + src1 = get_gprh(ctx, a->rs1); > + dest = dest_gprh(ctx, a->rd); > + > + func(dest, src1, immh); > + } > + gen_set_gprh(ctx, a->rd, dest); > + } If ol < RV128, you're storing the low dest into the gprh, which is wrong. It should be the sign-extension of the low part. But that should happen for all writes. Earlier, I suggested gen_set_gpr128 instead of gen_set_gprh. I think this should be written if (get_xl(ctx) == MXL_RV128) { TCGv src1h = get_gprh(ctx, a->rs1); TCGv desth = dest_gprh(ctx, a->rd); func(dest, src1h, -(a->imm < 0)); gen_set_gpr128(ctx, a->rd, dest, desth); } else { gen_set_gpr(ctx, a->rd, dest); } Where gen_set_gpr will handle the sign-extension to 128-bits. > @@ -443,6 +454,17 @@ static bool gen_logic(DisasContext *ctx, arg_r *a, DisasExtend ext, > > gen_set_gpr(ctx, a->rd, dest); > > + if (get_xl_max(ctx) == MXL_RV128) { > + if (get_ol(ctx) == MXL_RV128) { > + dest = dest_gprh(ctx, a->rd); > + src1 = get_gprh(ctx, a->rs1); > + src2 = get_gprh(ctx, a->rs2); > + > + func(dest, src1, src2); > + } > + gen_set_gprh(ctx, a->rd, dest); > + } Similarly. r~
Le 20/10/2021 à 19:47, Richard Henderson a écrit : > On 10/19/21 2:48 AM, Frédéric Pétrot wrote: >> The 128-bit bitwise instructions do not need any function prototype change >> as the functions can be applied independently on the lower and upper part of >> the registers. >> >> Signed-off-by: Frédéric Pétrot <frederic.petrot@univ-grenoble-alpes.fr> >> Co-authored-by: Fabien Portas <fabien.portas@grenoble-inp.org> >> --- >> target/riscv/translate.c | 22 ++++++++++++++++++++++ >> 1 file changed, 22 insertions(+) >> >> diff --git a/target/riscv/translate.c b/target/riscv/translate.c >> index e8f08f921e..71982f6284 100644 >> --- a/target/riscv/translate.c >> +++ b/target/riscv/translate.c >> @@ -429,6 +429,17 @@ static bool gen_logic_imm_fn(DisasContext *ctx, arg_i *a, >> DisasExtend ext, >> gen_set_gpr(ctx, a->rd, dest); >> + if (get_xl_max(ctx) == MXL_RV128) { >> + if (get_ol(ctx) == MXL_RV128) { >> + uint64_t immh = -(a->imm < 0); >> + src1 = get_gprh(ctx, a->rs1); >> + dest = dest_gprh(ctx, a->rd); >> + >> + func(dest, src1, immh); >> + } >> + gen_set_gprh(ctx, a->rd, dest); >> + } > > If ol < RV128, you're storing the low dest into the gprh, which is wrong. It > should be the sign-extension of the low part. But that should happen for all > writes. Thanks for your feedback (on the other parts too) that I'll apply. On this specific case, in gen_set_gprh I check that the operation is not on 128 bit in which case I propagate the sign of the low part instead of using dest (the spec says that the sign should propagate to misa_xl_max, irrelevant of xl). This implicitly forces the order in which the functions must be called as you noticed, and introducing a higher level function as you suggest would indeed make things more readable, and this can probably be applied in most places. Frédéric
diff --git a/target/riscv/translate.c b/target/riscv/translate.c index e8f08f921e..71982f6284 100644 --- a/target/riscv/translate.c +++ b/target/riscv/translate.c @@ -429,6 +429,17 @@ static bool gen_logic_imm_fn(DisasContext *ctx, arg_i *a, DisasExtend ext, gen_set_gpr(ctx, a->rd, dest); + if (get_xl_max(ctx) == MXL_RV128) { + if (get_ol(ctx) == MXL_RV128) { + uint64_t immh = -(a->imm < 0); + src1 = get_gprh(ctx, a->rs1); + dest = dest_gprh(ctx, a->rd); + + func(dest, src1, immh); + } + gen_set_gprh(ctx, a->rd, dest); + } + return true; } @@ -443,6 +454,17 @@ static bool gen_logic(DisasContext *ctx, arg_r *a, DisasExtend ext, gen_set_gpr(ctx, a->rd, dest); + if (get_xl_max(ctx) == MXL_RV128) { + if (get_ol(ctx) == MXL_RV128) { + dest = dest_gprh(ctx, a->rd); + src1 = get_gprh(ctx, a->rs1); + src2 = get_gprh(ctx, a->rs2); + + func(dest, src1, src2); + } + gen_set_gprh(ctx, a->rd, dest); + } + return true; }
The 128-bit bitwise instructions do not need any function prototype change as the functions can be applied independently on the lower and upper part of the registers. Signed-off-by: Frédéric Pétrot <frederic.petrot@univ-grenoble-alpes.fr> Co-authored-by: Fabien Portas <fabien.portas@grenoble-inp.org> --- target/riscv/translate.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)