[PATCH 1/2] target/arm: Make VQDMULL undefined when U=1

[PATCH 1/2] target/arm: Make VQDMULL undefined when U=1

[Fredrik Strupe]

According to Arm ARM, VQDMULL is only valid when U=0, while having
U=1 is unallocated.

Signed-off-by: Fredrik Strupe <fredrik@strupe.net>
Fixes: 695272dcb976 ("target-arm: Handle UNDEF cases for Neon 3-regs-different-widths")
---
 target/arm/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/arm/translate.c b/target/arm/translate.c
index 9f9f4e19e0..dfe9dbbcfd 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -5817,7 +5817,7 @@ static int disas_neon_data_insn(DisasContext *s, uint32_t insn)
                     {0, 0, 0, 0}, /* VMLSL */
                     {0, 0, 0, 9}, /* VQDMLSL */
                     {0, 0, 0, 0}, /* Integer VMULL */
-                    {0, 0, 0, 1}, /* VQDMULL */
+                    {0, 0, 0, 9}, /* VQDMULL */
                     {0, 0, 0, 0xa}, /* Polynomial VMULL */
                     {0, 0, 0, 7}, /* Reserved: always UNDEF */
                 };
-- 
2.20.1

[PATCH 2/2] target/arm: Fail on invalid size for VMUL (float)

[Fredrik Strupe]

Bit 1 of VMUL (float)'s size field encodes the opcode and must be 0,
with 1 making it undefined. Thus, make VMUL (float) instructions
with size=0b10 or size=0b11 (size >= 2) undefined.

(U is 1 for VMUL, while it is 0 for VMLA/VMLS.)

Signed-off-by: Fredrik Strupe <fredrik@strupe.net>
---
 target/arm/translate.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/target/arm/translate.c b/target/arm/translate.c
index dfe9dbbcfd..4268eed9b7 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -5237,6 +5237,11 @@ static int disas_neon_data_insn(DisasContext *s, uint32_t insn)
         }
         case NEON_3R_FLOAT_MULTIPLY:
         {
+            /* Size bit 1 of VMUL (float) encodes the op and must be 0 */
+            if (u && size >= 2)  {
+                return 1;
+            }
+
             TCGv_ptr fpstatus = get_fpstatus_ptr(1);
             gen_helper_vfp_muls(tmp, tmp, tmp2, fpstatus);
             if (!u) {
-- 
2.20.1

Re: [PATCH 2/2] target/arm: Fail on invalid size for VMUL (float)

[Peter Maydell]

On Wed, 8 Apr 2020 at 16:29, Fredrik Strupe <fredrik@strupe.net> wrote:
>
> Bit 1 of VMUL (float)'s size field encodes the opcode and must be 0,
> with 1 making it undefined. Thus, make VMUL (float) instructions
> with size=0b10 or size=0b11 (size >= 2) undefined.
>
> (U is 1 for VMUL, while it is 0 for VMLA/VMLS.)
>
> Signed-off-by: Fredrik Strupe <fredrik@strupe.net>
> ---
>  target/arm/translate.c | 5 +++++
>  1 file changed, 5 insertions(+)

Thanks for this patch. I'm actually in the middle of a
refactoring of this code to use decodetree, but I'll make
sure I check that the refactoring fixes this decode bug.

Also undef-checks like this in the old neon decode structure
should be in the switch (op) outside the loop-for-each-element:
compare NEON_3R_FLOAT_CMP; but it's a bit moot with the
refactoring as all that code will be deleted.

thanks
-- PMM

Re: [PATCH 2/2] target/arm: Fail on invalid size for VMUL (float)

[Fredrik Strupe]

On 08.04.2020 18:27, Peter Maydell wrote:
> On Wed, 8 Apr 2020 at 16:29, Fredrik Strupe <fredrik@strupe.net> wrote:
>>
>> Bit 1 of VMUL (float)'s size field encodes the opcode and must be 0,
>> with 1 making it undefined. Thus, make VMUL (float) instructions
>> with size=0b10 or size=0b11 (size >= 2) undefined.
>>
>> (U is 1 for VMUL, while it is 0 for VMLA/VMLS.)
>>
>> Signed-off-by: Fredrik Strupe <fredrik@strupe.net>
>> ---
>>  target/arm/translate.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>
> Thanks for this patch. I'm actually in the middle of a
> refactoring of this code to use decodetree, but I'll make
> sure I check that the refactoring fixes this decode bug.
>
> Also undef-checks like this in the old neon decode structure
> should be in the switch (op) outside the loop-for-each-element:
> compare NEON_3R_FLOAT_CMP; but it's a bit moot with the
> refactoring as all that code will be deleted.
>
> thanks
> -- PMM

Alright, glad to see it being fixed.

Fredrik

Re: [PATCH 1/2] target/arm: Make VQDMULL undefined when U=1

[Peter Maydell]

On Wed, 8 Apr 2020 at 15:54, Fredrik Strupe <fredrik@strupe.net> wrote:
>
> According to Arm ARM, VQDMULL is only valid when U=0, while having
> U=1 is unallocated.
>
> Signed-off-by: Fredrik Strupe <fredrik@strupe.net>
> Fixes: 695272dcb976 ("target-arm: Handle UNDEF cases for Neon 3-regs-different-widths")
> ---

I'm going to apply this patch to target-arm.next because
my refactoring of Neon hasn't got to this group of insns
yet, so putting the fix into the tree is the easiest way
to remind me to handle it during the refactoring.

(The other patch is in code I've already refactored; I'm
planning to send out a series with that refactoring shortly.)

thanks
-- PMM