| Message ID | 20240117213646.159697-2-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tcg/s390x: Fix chacha20-s390 | expand |
On 17/01/2024 22.36, Richard Henderson wrote: > While the format names the second vector register 'v3', > it is still in the second position (bits 12-15) and > the argument to RXB must match. > > Example error: > - e7 00 00 10 2a 33 verllf %v16,%v0,16 > + e7 00 00 10 2c 33 verllf %v16,%v16,16 > > Cc: qemu-stable@nongnu.org > Reported-by: Michael Tokarev <mjt@tls.msk.ru> > Fixes: 22cb37b4172 ("tcg/s390x: Implement vector shift operations") > Fixes: 79cada8693d ("tcg/s390x: Implement tcg_out_dup*_vec") > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > tcg/s390x/tcg-target.c.inc | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/tcg/s390x/tcg-target.c.inc b/tcg/s390x/tcg-target.c.inc > index fbee43d3b0..7f6b84aa2c 100644 > --- a/tcg/s390x/tcg-target.c.inc > +++ b/tcg/s390x/tcg-target.c.inc > @@ -683,7 +683,7 @@ static void tcg_out_insn_VRIc(TCGContext *s, S390Opcode op, > tcg_debug_assert(is_vector_reg(v3)); > tcg_out16(s, (op & 0xff00) | ((v1 & 0xf) << 4) | (v3 & 0xf)); > tcg_out16(s, i2); > - tcg_out16(s, (op & 0x00ff) | RXB(v1, 0, v3, 0) | (m4 << 12)); > + tcg_out16(s, (op & 0x00ff) | RXB(v1, v3, 0, 0) | (m4 << 12)); > } > > static void tcg_out_insn_VRRa(TCGContext *s, S390Opcode op, > @@ -738,7 +738,7 @@ static void tcg_out_insn_VRSa(TCGContext *s, S390Opcode op, TCGReg v1, > tcg_debug_assert(is_vector_reg(v3)); > tcg_out16(s, (op & 0xff00) | ((v1 & 0xf) << 4) | (v3 & 0xf)); > tcg_out16(s, b2 << 12 | d2); > - tcg_out16(s, (op & 0x00ff) | RXB(v1, 0, v3, 0) | (m4 << 12)); > + tcg_out16(s, (op & 0x00ff) | RXB(v1, v3, 0, 0) | (m4 << 12)); > } > > static void tcg_out_insn_VRSb(TCGContext *s, S390Opcode op, TCGReg v1, > @@ -762,7 +762,7 @@ static void tcg_out_insn_VRSc(TCGContext *s, S390Opcode op, TCGReg r1, > tcg_debug_assert(is_vector_reg(v3)); > tcg_out16(s, (op & 0xff00) | (r1 << 4) | (v3 & 0xf)); > tcg_out16(s, b2 << 12 | d2); > - tcg_out16(s, (op & 0x00ff) | RXB(0, 0, v3, 0) | (m4 << 12)); > + tcg_out16(s, (op & 0x00ff) | RXB(0, v3, 0, 0) | (m4 << 12)); > } > > static void tcg_out_insn_VRX(TCGContext *s, S390Opcode op, TCGReg v1, I double-checked the Principles of Operation that VRI-c, VRS-a and VRS-c are the only encodings where this could happen, and yes, your modification looks right to me: Reviewed-by: Thomas Huth <thuth@redhat.com> Do you want to take it through our TCG branch or shall I pick it up for my s390x branch?
On 1/18/24 17:50, Thomas Huth wrote:
> Do you want to take it through our TCG branch or shall I pick it up for my s390x branch?
I can take it through tcg.
r~
On 17/1/24 22:36, Richard Henderson wrote: > While the format names the second vector register 'v3', > it is still in the second position (bits 12-15) and > the argument to RXB must match. > > Example error: > - e7 00 00 10 2a 33 verllf %v16,%v0,16 > + e7 00 00 10 2c 33 verllf %v16,%v16,16 > > Cc: qemu-stable@nongnu.org > Reported-by: Michael Tokarev <mjt@tls.msk.ru> > Fixes: 22cb37b4172 ("tcg/s390x: Implement vector shift operations") > Fixes: 79cada8693d ("tcg/s390x: Implement tcg_out_dup*_vec") > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > tcg/s390x/tcg-target.c.inc | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/tcg/s390x/tcg-target.c.inc b/tcg/s390x/tcg-target.c.inc > index fbee43d3b0..7f6b84aa2c 100644 > --- a/tcg/s390x/tcg-target.c.inc > +++ b/tcg/s390x/tcg-target.c.inc > @@ -683,7 +683,7 @@ static void tcg_out_insn_VRIc(TCGContext *s, S390Opcode op, > tcg_debug_assert(is_vector_reg(v3)); > tcg_out16(s, (op & 0xff00) | ((v1 & 0xf) << 4) | (v3 & 0xf)); > tcg_out16(s, i2); > - tcg_out16(s, (op & 0x00ff) | RXB(v1, 0, v3, 0) | (m4 << 12)); > + tcg_out16(s, (op & 0x00ff) | RXB(v1, v3, 0, 0) | (m4 << 12)); 🎩 Chapeau.
diff --git a/tcg/s390x/tcg-target.c.inc b/tcg/s390x/tcg-target.c.inc index fbee43d3b0..7f6b84aa2c 100644 --- a/tcg/s390x/tcg-target.c.inc +++ b/tcg/s390x/tcg-target.c.inc @@ -683,7 +683,7 @@ static void tcg_out_insn_VRIc(TCGContext *s, S390Opcode op, tcg_debug_assert(is_vector_reg(v3)); tcg_out16(s, (op & 0xff00) | ((v1 & 0xf) << 4) | (v3 & 0xf)); tcg_out16(s, i2); - tcg_out16(s, (op & 0x00ff) | RXB(v1, 0, v3, 0) | (m4 << 12)); + tcg_out16(s, (op & 0x00ff) | RXB(v1, v3, 0, 0) | (m4 << 12)); } static void tcg_out_insn_VRRa(TCGContext *s, S390Opcode op, @@ -738,7 +738,7 @@ static void tcg_out_insn_VRSa(TCGContext *s, S390Opcode op, TCGReg v1, tcg_debug_assert(is_vector_reg(v3)); tcg_out16(s, (op & 0xff00) | ((v1 & 0xf) << 4) | (v3 & 0xf)); tcg_out16(s, b2 << 12 | d2); - tcg_out16(s, (op & 0x00ff) | RXB(v1, 0, v3, 0) | (m4 << 12)); + tcg_out16(s, (op & 0x00ff) | RXB(v1, v3, 0, 0) | (m4 << 12)); } static void tcg_out_insn_VRSb(TCGContext *s, S390Opcode op, TCGReg v1, @@ -762,7 +762,7 @@ static void tcg_out_insn_VRSc(TCGContext *s, S390Opcode op, TCGReg r1, tcg_debug_assert(is_vector_reg(v3)); tcg_out16(s, (op & 0xff00) | (r1 << 4) | (v3 & 0xf)); tcg_out16(s, b2 << 12 | d2); - tcg_out16(s, (op & 0x00ff) | RXB(0, 0, v3, 0) | (m4 << 12)); + tcg_out16(s, (op & 0x00ff) | RXB(0, v3, 0, 0) | (m4 << 12)); } static void tcg_out_insn_VRX(TCGContext *s, S390Opcode op, TCGReg v1,
While the format names the second vector register 'v3', it is still in the second position (bits 12-15) and the argument to RXB must match. Example error: - e7 00 00 10 2a 33 verllf %v16,%v0,16 + e7 00 00 10 2c 33 verllf %v16,%v16,16 Cc: qemu-stable@nongnu.org Reported-by: Michael Tokarev <mjt@tls.msk.ru> Fixes: 22cb37b4172 ("tcg/s390x: Implement vector shift operations") Fixes: 79cada8693d ("tcg/s390x: Implement tcg_out_dup*_vec") Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- tcg/s390x/tcg-target.c.inc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)