RE: [PATCH 1/2] Hexagon (iclass): update J4_hintjumpr slot constraints

[Taylor Simpson]

> -----Original Message-----
> From: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>
> Sent: Friday, January 13, 2023 7:39 AM
> To: qemu-devel@nongnu.org
> Cc: Taylor Simpson <tsimpson@quicinc.com>; Brian Cain
> <bcain@quicinc.com>; richard.henderson@linaro.org
> Subject: [PATCH 1/2] Hexagon (iclass): update J4_hintjumpr slot constraints
> 
> The Hexagon PRM says that "The assembler automatically encodes
> instructions in the packet in the proper order. In the binary encoding of a
> packet, the instructions must be ordered from Slot 3 down to Slot 0."
> 
> Prior to the architecture version v73, the slot constraints from instruction
> "hintjr" only allowed it to be executed at slot 2.
> With that in mind, consider the packet:
> 
>     {
>         hintjr(r0)
>         nop
>         nop
>         if (!p0) memd(r1+#0) = r1:0
>     }
> 
> To satisfy the ordering rule quoted from the PRM, the assembler would,
> thus, move one of the nops to the first position, so that it can be assigned 
> to
> slot 3 and the subsequent hintjr to slot 2.
> 
> However, since v73, hintjr can be executed at either slot 2 or 3. So there is 
> no
> need to reorder that packet and the assembler will encode it as is. When
> QEMU tries to execute it, however, we end up hitting a "misaliged store"
> exception because both the store and the hintjr will be assigned to store 0,
> and some functions like `slot_is_predicated()` expect the decode machinery
> to assign only one instruction per slot. In particular, the mentioned function
> will traverse the packet until it finds the first instruction at the desired 
> slot
> which, for slot 0, will be hintjr. Since hintjr is not predicated, the result 
> is that
> we try to execute the store regardless of the predicate. And because the
> predicate is false, we had not previously loaded hex_store_addr[0] or
> hex_store_width[0]. As a result, the store will decide de width based on
> trash memory, causing it to be misaligned.
> 
> Update the slot constraints for hintjr so that QEMU can properly handle such
> encodings.
> 
> Note: to avoid similar-but-not-identical issues in the future, we should look
> for multiple instructions at the same slot during decoding time and throw an
> invalid packet exception. That will be done in the subsequent commit.
> 
> Signed-off-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>
> ---
>  target/hexagon/iclass.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/target/hexagon/iclass.c b/target/hexagon/iclass.c index
> 6091286993..081116fc4d 100644
> --- a/target/hexagon/iclass.c
> +++ b/target/hexagon/iclass.c
> @@ -51,8 +51,10 @@ SlotMask find_iclass_slots(Opcode opcode, int itype)
>          return SLOTS_0;
>      } else if ((opcode == J2_trap0) ||
>                 (opcode == Y2_isync) ||
> -               (opcode == J2_pause) || (opcode == J4_hintjumpr)) {
> +               (opcode == J2_pause)) {
>          return SLOTS_2;
> +    } else if (opcode == J4_hintjumpr) {
> +        return SLOTS_23;
>      } else if (GET_ATTRIB(opcode, A_CRSLOT23)) {
>          return SLOTS_23;
>      } else if (GET_ATTRIB(opcode, A_RESTRICT_PREFERSLOT0)) {

Reviewed-by: Taylor Simpson <tsimpson@quicinc.com>