Re: [PATCH 04/13] target/hexagon: add v68 HVX IEEE float arithmetic insns

[Matheus Bernardino <matheus.bernardino@oss.qualcomm.com>]

On Mon, Mar 23, 2026 at 5:29 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:
>>
>> Add HVX IEEE floating-point arithmetic instructions:
>> - vmpy_sf_sf, vmpy_sf_hf, vmpy_hf_hf: multiply operations
>> - vdmpy_sf_hf: dot-product multiply
>> - vmpy_sf_hf_acc, vmpy_hf_hf_acc, vdmpy_sf_hf_acc: multiply-accumulate
>> - vadd_sf_sf, vsub_sf_sf, vadd_sf_hf, vsub_sf_hf: add/sub with sf output
>> - vadd_hf_hf, vsub_hf_hf: add/sub with hf output
>>
>> Signed-off-by: Matheus Tavares Bernardino <matheus.bernardino@oss.qualcomm.com>
>> ---
>>  target/hexagon/mmvec/kvx_ieee.h              | 47 ++++++++++
>>  target/hexagon/mmvec/macros.h                |  1 +
>>  target/hexagon/mmvec/mmvec.h                 |  2 +
>>  target/hexagon/attribs_def.h.inc             |  4 +
>>  target/hexagon/mmvec/kvx_ieee.c              | 87 ++++++++++++++++++
>>  target/hexagon/hex_common.py                 |  1 +
>>  target/hexagon/imported/mmvec/encode_ext.def | 18 ++++
>>  target/hexagon/imported/mmvec/ext.idef       | 93 ++++++++++++++++++++
>>  target/hexagon/meson.build                   |  1 +
>>  9 files changed, 254 insertions(+)
>>  create mode 100644 target/hexagon/mmvec/kvx_ieee.h
>>  create mode 100644 target/hexagon/mmvec/kvx_ieee.c
>
>
> I'm curious why the prefix is kvx instead of hvx.

Actually, not sure either... These files were imported from the arch
simulator. Brian suggested it was a left over acronym that didn't
catch on. I'll rename to hvx_ieee

>>
>> diff --git a/target/hexagon/mmvec/kvx_ieee.h b/target/hexagon/mmvec/kvx_ieee.h
>> new file mode 100644
>> index 0000000000..e92ddebeb9
>> --- /dev/null
>> +++ b/target/hexagon/mmvec/kvx_ieee.h
>> @@ -0,0 +1,47 @@
>> +/*
>> + *  Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>> + *
>> + *  SPDX-License-Identifier: GPL-2.0-or-later
>> + */
>> +
>> +#ifndef HEXAGON_KVX_IEEE_H
>> +#define HEXAGON_KVX_IEEE_H
>> +
>> +#include "fpu/softfloat.h"
>> +
>> +/* Hexagon canonical NaN */
>> +#define FP32_DEF_NAN      0x7FFFFFFF
>> +#define FP16_DEF_NAN      0x7FFF
>
>
> These are the same as the scalar core, right?  If so, there's already a call to set_float_default_nan_pattern in hexagon_cpu_reset_hold.
>
> If the patterns are different, you'll need to call set_float_default_nan_pattern before each scalar FP instruction and before each HVX FP instruction.

Not the same, no. I'll adjust the set_float_default_nan_pattern call
accordingly.

>> +
>> +/*
>> + * IEEE - FP ADD/SUB/MPY instructionsFP
>> + */
>> +uint32_t fp_mult_sf_sf(uint32_t a1, uint32_t a2, float_status *fp_status);
>> +uint32_t fp_add_sf_sf(uint32_t a1, uint32_t a2, float_status *fp_status);
>> +uint32_t fp_sub_sf_sf(uint32_t a1, uint32_t a2, float_status *fp_status);
>> +
>> +uint16_t fp_mult_hf_hf(uint16_t a1, uint16_t a2, float_status *fp_status);
>> +uint16_t fp_add_hf_hf(uint16_t a1, uint16_t a2, float_status *fp_status);
>> +uint16_t fp_sub_hf_hf(uint16_t a1, uint16_t a2, float_status *fp_status);
>> +
>> +uint32_t fp_mult_sf_hf(uint16_t a1, uint16_t a2, float_status *fp_status);
>> +uint32_t fp_add_sf_hf(uint16_t a1, uint16_t a2, float_status *fp_status);
>> +uint32_t fp_sub_sf_hf(uint16_t a1, uint16_t a2, float_status *fp_status);
>> +
>> +/*
>> + * IEEE - FP Accumulate instructions
>> + */
>> +uint16_t fp_mult_hf_hf_acc(uint16_t a1, uint16_t a2, uint16_t acc,
>> +                           float_status *fp_status);
>> +uint32_t fp_mult_sf_hf_acc(uint16_t a1, uint16_t a2, uint32_t acc,
>> +                           float_status *fp_status);
>> +
>> +/*
>> + * IEEE - FP Reduce instructions
>> + */
>> +uint32_t fp_vdmpy(uint16_t a1, uint16_t a2, uint16_t a3, uint16_t a4,
>> +                  float_status *fp_status);
>> +uint32_t fp_vdmpy_acc(uint32_t acc, uint16_t a1, uint16_t a2, uint16_t a3,
>> +                      uint16_t a4, float_status *fp_status);
>> +
>
>
> Consider using macros similar to the ones in the .c file to create these protos.

Hmm, I think in this case, the boilerplate size will outweight the
benefit of the macros.

>>
>> +#endif
>> diff --git a/target/hexagon/mmvec/kvx_ieee.c b/target/hexagon/mmvec/kvx_ieee.c
>> new file mode 100644
>> index 0000000000..b763899aa3
>> --- /dev/null
>> +++ b/target/hexagon/mmvec/kvx_ieee.c
>> @@ -0,0 +1,87 @@
>> +/*
>> + *  Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>> + *
>> + *  SPDX-License-Identifier: GPL-2.0-or-later
>> + */
>> +
>> +#include "qemu/osdep.h"
>> +#include "kvx_ieee.h"
>> +
>> +#define DEF_FP_INSN_2(name, rt, a1t, a2t, op) \
>> +    uint##rt##_t fp_##name(uint##a1t##_t a1, uint##a2t##_t a2, \
>> +                           float_status *fp_status) { \
>> +        float##a1t f1 = make_float##a1t(a1); \
>> +        float##a2t f2 = make_float##a2t(a2); \
>> +        \
>> +        if (float##a1t##_is_any_nan(f1) || float##a2t##_is_any_nan(f2)) { \
>> +            return FP##rt##_DEF_NAN; \
>> +        } \
>
>
> These nan checks shouldn't be needed if you're using QEMU softfloat properly.

Ah, indeed. Just checked that. Will change

>> +
>> diff --git a/target/hexagon/imported/mmvec/ext.idef b/target/hexagon/imported/mmvec/ext.idef
>> index 03d31f6181..3f0d8e366e 100644
>> --- a/target/hexagon/imported/mmvec/ext.idef
>> +++ b/target/hexagon/imported/mmvec/ext.idef
>> @@ -2895,9 +2895,102 @@ EXTINSN(V6_vprefixqw,"Vd32.w=prefixsum(Qv4)",   ATTRIBS(A_EXTENSION,A_CVI,A_CVI_
>>      }
>>      } )
>>
>> +/* KVX - IEEE FP Instructions */
>>
>> +/* Single pipe, 32-bit output */
>> +#define ITERATOR_INSN_IEEE_FP_32(WIDTH,TAG,SYNTAX,DESCR,CODE) \
>> +EXTINSN(V6_##TAG, SYNTAX, \
>> +ATTRIBS(A_EXTENSION,A_HVX_IEEE_FP,A_CVI,A_CVI_VX,A_HVX_IEEE_FP_OUT_32), \
>> +DESCR, DO_FOR_EACH_CODE(WIDTH, CODE))
>>
>> +/* Single pipe, 16-bit output */
>> +#define ITERATOR_INSN_IEEE_FP_16(WIDTH,TAG,SYNTAX,DESCR,CODE) \
>> +EXTINSN(V6_##TAG, SYNTAX, \
>> +ATTRIBS(A_EXTENSION,A_HVX_IEEE_FP,A_CVI,A_CVI_VX,A_HVX_IEEE_FP_OUT_16), \
>> +DESCR, DO_FOR_EACH_CODE(WIDTH, CODE))
>>
>> +/* Two pipes: P2 & P3, single output: P2, 32-bit output */
>> +#define ITERATOR_INSN_IEEE_FP_DOUBLE_SINGLE_32(WIDTH,TAG,SYNTAX,DESCR,CODE) \
>> +EXTINSN(V6_##TAG, SYNTAX, \
>> +ATTRIBS(A_EXTENSION,A_HVX_IEEE_FP,A_CVI,A_CVI_VX_DV,A_HVX_IEEE_FP_OUT_32), \
>> +DESCR, DO_FOR_EACH_CODE(WIDTH, CODE))
>> +
>> +/* Two pipes: P2 & P3, two outputs, 32-bit output */
>> +#define ITERATOR_INSN_IEEE_FP_DOUBLE_32(WIDTH,TAG,SYNTAX,DESCR,CODE) \
>> +EXTINSN(V6_##TAG, SYNTAX, \
>> +ATTRIBS(A_EXTENSION,A_HVX_IEEE_FP,A_CVI,A_CVI_VX_DV,A_HVX_IEEE_FP_OUT_32), \
>> +DESCR, DO_FOR_EACH_CODE(WIDTH, CODE))
>> +
>> +/*
>> + * single pipe, accumulate instruction, produces 16-bit output, requires 16-bit
>> + * accumulate input
>> + */
>> +#define ITERATOR_INSN_IEEE_FP_ACC_16(WIDTH,TAG,SYNTAX,DESCR,CODE) \
>> +EXTINSN(V6_##TAG, SYNTAX, \
>> +ATTRIBS(A_EXTENSION,A_HVX_IEEE_FP,A_CVI,A_CVI_VX,A_HVX_IEEE_FP_ACC,A_HVX_IEEE_FP_OUT_16,A_CVI_VX_NO_TMP_LD), \
>> +DESCR, DO_FOR_EACH_CODE(WIDTH, CODE))
>> +
>> +/*
>> + * single pipe, accumulate instruction, produces 32-bit output, requires 32-bit
>> + * accumulate input
>> + */
>> +#define ITERATOR_INSN_IEEE_FP_ACC_32(WIDTH,TAG,SYNTAX,DESCR,CODE) \
>> +EXTINSN(V6_##TAG, SYNTAX, \
>> +ATTRIBS(A_EXTENSION,A_HVX_IEEE_FP,A_CVI,A_CVI_VX,A_HVX_IEEE_FP_ACC,A_HVX_IEEE_FP_OUT_32,A_CVI_VX_NO_TMP_LD), \
>> +DESCR, DO_FOR_EACH_CODE(WIDTH, CODE))
>> +
>> +/* IEEE FP multiply instructions */
>> +ITERATOR_INSN_IEEE_FP_DOUBLE_SINGLE_32(32, vmpy_sf_sf,
>> +    "Vd32.sf=vmpy(Vu32.sf,Vv32.sf)", "Vector IEEE mul: sf",
>> +    VdV.sf[i] = fp_mult_sf_sf(VuV.sf[i], VvV.sf[i], &env->fp_status))
>
>
> Do these instructions interact with the FP bits in USR (e.g., rounding mode, FP exceptions)?

They do not. I'll add a new env->hvx_fp_status and use that for the
default nan. This way we can avoid messing up with the scalar
fp_status.