Re: [PATCH 03/13] target/hexagon/cpu: add HVX IEEE FP extension

[Taylor Simpson <ltaylorsimpson@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 4310 bytes --]

On Tue, Mar 24, 2026 at 1:21 PM Brian Cain <brian.cain@oss.qualcomm.com>
wrote:

>
>
> On Tue, Mar 24, 2026 at 1:48 PM Taylor Simpson <ltaylorsimpson@gmail.com>
> wrote:
>
>>
>>
>> On Tue, Mar 24, 2026 at 10:52 AM Matheus Bernardino <
>> matheus.bernardino@oss.qualcomm.com> wrote:
>>
>>> On Mon, Mar 23, 2026 at 4:33 PM Taylor Simpson <ltaylorsimpson@gmail.com>
>>> wrote:
>>> >
>>> >
>>> >
>>> > On Mon, Mar 23, 2026 at 7:15 AM Matheus Tavares Bernardino <
>>> matheus.bernardino@oss.qualcomm.com> wrote:
>>> >>
>>> >> This flag will be used to control the HVX IEEE float instructions,
>>> which
>>> >> are only available at some Hexagon cores. When unavailable, the
>>> >> instruction is essentially treated as a no-op.
>>> >>
>>> >> Signed-off-by: Matheus Tavares Bernardino <
>>> matheus.bernardino@oss.qualcomm.com>
>>> >> ---
>>> >>  target/hexagon/cpu.h             |  1 +
>>> >>  target/hexagon/translate.h       |  1 +
>>> >>  target/hexagon/attribs_def.h.inc |  3 +++
>>> >>  target/hexagon/cpu.c             |  1 +
>>> >>  target/hexagon/decode.c          | 22 ++++++++++++++++++++++
>>> >>  target/hexagon/translate.c       |  1 +
>>> >>  6 files changed, 29 insertions(+)
>>> >>
>>> >>
>>> >> diff --git a/target/hexagon/decode.c b/target/hexagon/decode.c
>>> >> index dbc9c630e8..d832a64a17 100644
>>> >> --- a/target/hexagon/decode.c
>>> >> +++ b/target/hexagon/decode.c
>>> >> @@ -696,6 +696,18 @@ static bool pkt_has_write_conflict(Packet *pkt)
>>> >>      return !bitmap_empty(conflict, 32);
>>> >>  }
>>> >>
>>> >> +static void convert_to_nop(Insn *insn)
>>> >> +{
>>> >> +    bool is_endloop = insn->is_endloop;
>>> >> +    memset(insn, 0, sizeof(*insn));
>>> >> +    insn->opcode = A2_nop;
>>> >> +    insn->new_read_idx = -1;
>>> >> +    insn->dest_idx = -1;
>>> >> +    insn->generate = opcode_genptr[insn->opcode];
>>> >> +    insn->iclass = 0b111;
>>> >> +    insn->is_endloop = is_endloop;
>>> >> +}
>>> >> +
>>> >>  /*
>>> >>   * decode_packet
>>> >>   * Decodes packet with given words
>>> >> @@ -746,6 +758,16 @@ int decode_packet(DisasContext *ctx, int
>>> max_words, const uint32_t *words,
>>> >>          /* Ran out of words! */
>>> >>          return 0;
>>> >>      }
>>> >> +
>>> >> +    /* Disable HVX IEEE instruction if extension is disabled. */
>>> >> +    if (!ctx->ieee_fp_extension) {
>>> >> +        for (i = 0; i < num_insns; i++) {
>>> >> +            if (GET_ATTRIB(pkt->insn[i].opcode, A_HVX_IEEE_FP)) {
>>> >> +                convert_to_nop(&pkt->insn[i]);
>>> >> +            }
>>> >> +        }
>>> >> +    }
>>> >> +
>>> >
>>> >
>>> > Better to leave the instruction alone and turn it into a nop by not
>>> generating any TCG.
>>> >
>>> > That way, the disassembly (-d in_asm) will still show what's actually
>>> in the binary.  You could add the check in gen_tcg_funcs.py.
>>> >
>>> > You could also consider adding some sort of marker in the disassembly
>>> to indicate that the flag is needed for the instruction to do anything.
>>>
>>> Ah, good idea. Will do both for the next round, thanks.
>>>
>>
>> Note that we'll need to be careful with packets that use the result
>> vector in a .new context.  For example
>>     { V0.sf = vadd(V1.sf,V2.sf)
>>       vmem(R19+#0x0) = V0.new }
>> The problem is that the store wants to read the value from future_VRegs.
>> However, if the vadd is  nop, there is junk in future_VRegs.  So, we'll
>> either have to get the store to read from the real VRegs or have the vadd
>> copy the old value of the destination into the future_VRegs value.  The
>> first option will be more efficient because it will avoid the vector copy.
>>
>>
> For the sake of ease-of-verification we'll want to do whatever the ISS
> does.  It's not very obvious to me what it would do in this packet context
> based on the description of the nop-like behavior, but we'll follow the
> ISS' lead.  In practical terms the garbage in future_VRegs is probably just
> as bad or good as any other value - if you bothered to execute this packet
> on the target w/o support for this opcode you probably don't care much
> about the result.
>

I'll be interested to know what the ISS and hardware do in this case.

Thanks,
Taylor

[-- Attachment #2: Type: text/html, Size: 6166 bytes --]