From 4169bef1b66fd19e482f7a0d7cb8eb081ed14263 Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Fri, 8 Jan 2021 21:38:09 -0500 Subject: [PATCH] pan/bi: Fix M1/M2 decoding in disassembler C's definition of the % operator has a footgun around sign conversion. Avoid it and just use bitwise arithemtic instead like the hardware would, fixing the disassembly and making buggy assembly more obvious. Fixes: 08a9e5e3e89 ("pan/bi: Decode M values in disasm") Signed-off-by: Alyssa Rosenzweig Reviewed-by: Boris Brezillon Part-of: (cherry picked from commit a69c73988b26aaaa6bffde1b3fef5fdc4a7a90a7) --- .pick_status.json | 2 +- src/panfrost/bifrost/disassemble.c | 16 +++++++++------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index 3422798597c..aaf8a9de722 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -1093,7 +1093,7 @@ "description": "pan/bi: Fix M1/M2 decoding in disassembler", "nominated": true, "nomination_type": 1, - "resolution": 0, + "resolution": 1, "master_sha": null, "because_sha": "08a9e5e3e892e9acc7fcfc2cefb45990efa62e40" }, diff --git a/src/panfrost/bifrost/disassemble.c b/src/panfrost/bifrost/disassemble.c index f4c940c2da6..ee9e406dd1c 100644 --- a/src/panfrost/bifrost/disassemble.c +++ b/src/panfrost/bifrost/disassemble.c @@ -614,15 +614,17 @@ static bool dump_clause(FILE *fp, uint32_t *words, unsigned *size, unsigned offs consts.raw[const_idx + 1] = const1; /* Calculate M values from A, B and 4-bit - * unsigned arithmetic */ + * unsigned arithmetic. Mathematically it + * should be (A - B) % 16 but we use this + * alternate form to avoid sign issues */ - signed A1 = bits(words[2], 0, 4); - signed B1 = bits(words[3], 28, 32); - signed A2 = bits(words[1], 0, 4); - signed B2 = bits(words[2], 28, 32); + unsigned A1 = bits(words[2], 0, 4); + unsigned B1 = bits(words[3], 28, 32); + unsigned A2 = bits(words[1], 0, 4); + unsigned B2 = bits(words[2], 28, 32); - unsigned M1 = (A1 - B1) % 16; - unsigned M2 = (A2 - B2) % 16; + unsigned M1 = (16 + A1 - B1) & 0xF; + unsigned M2 = (16 + A2 - B2) & 0xF; decode_M(&consts.mods[const_idx], M1, M2, false);