RE: [PATCH v2 5/7] [X86]: Add FP8 to FP4 and vice-versa converts.

[Sharma, Dipesh]

AMD General

Hi,

> -----Original Message-----
> From: Jiang, Haochen <haochen.ji...@intel.com>
> Sent: 21 July 2026 11:35
> To: Sharma, Dipesh <dipesh.sha...@amd.com>; gcc-patches@gcc.gnu.org
> Cc: Liu, Hongtao <hongtao....@intel.com>; ubiz...@gmail.com;
> honza.hubi...@gmail.com; Kumar, Venkataramanan
> <venkataramanan.ku...@amd.com>
> Subject: RE: [PATCH v2 5/7] [X86]: Add FP8 to FP4 and vice-versa converts.
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> > From: Dipesh Sharma <dipesh.sha...@amd.com>
> > Sent: Monday, July 20, 2026 2:25 AM
> >
> > diff --git a/gcc/config/i386/avx10v2auxintrin.h
> > b/gcc/config/i386/avx10v2auxintrin.h
> > index a862f3173eb..bc004a24dfb 100644
> > --- a/gcc/config/i386/avx10v2auxintrin.h
> > +++ b/gcc/config/i386/avx10v2auxintrin.h
> > @@ -1195,6 +1195,187 @@ _mm512_maskz_cvthf8_ps (__mmask16 __U,
> __m128i
> > __A)
> >                                                    (__mmask16) __U);
> > }
> >
> > +// VCVTBF82BF4S
> > +
> > +extern __inline __m128i
> > +__attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
> > +_mm_cvtbf8_bf4s (__m128i __A)
>
> The intrin name is not following the naming convention, should be
> _mm_cvts_bf8_bf4, similar for all the occurrences in this patch.
I will update it.
>
> > +{
> > +  return (__m128i) __builtin_ia32_vcvtbf82bf4s128 ((__v16qi) __A); }
> > +
> > +extern __inline void
> > +__attribute__ ((__gnu_inline__, __always_inline__, __artificial__))
> > +_mm_cvtbf8_bf4s_storeu (void * __P, __m128i __A) {
> > +  __builtin_ia32_vcvtbf82bf4s128mem ((__v8qi *) __P, (__v16qi) __A);
> > +}
>
> We don't need these storeu intrin, vpmovssdb is a kind of move, that is why
> we need them.
I understand the notion that move intuitively implies memory move, but
instruction definition states it supports a memory destination. Is it not 
better to add a memory store intrinsic ?

VCVTBF82BF4S xmm1/m64, xmm2 EVEX.128.F3.MAP5.W1 3D /r
VCVTBF82BF4S xmm1/m128, ymm2 EVEX.256.F3.MAP5.W1 3D /r
VCVTBF82BF4S ymm1/m256, zmm2 EVEX.512.F3.MAP5.W1 3D /r

>
> > diff --git a/gcc/config/i386/sse.md b/gcc/config/i386/sse.md index
> > 1583c59db3b..dd6393cabf9 100644
> > --- a/gcc/config/i386/sse.md
> > +++ b/gcc/config/i386/sse.md
> > @@ -34380,3 +34383,86 @@
> >    "TARGET_AVX10V2AUX"
> >
> > "vcvt<convertfp82ps>\t{%1,
> %0<mask_operand2>|%0<mask_operand2>,%<i
> > ptrssebvec_3>1}"
> >    [(set_attr "prefix" "evex")])
> > +
> > +;; FP8 to FP4 converts (VCVTBF82BF4S, VCVTHF82BF4S) - no masking
> > +
> > +(define_int_iterator UNSPEC_CONVERTFP82BF4
> > +  [UNSPEC_VCVTBF82BF4S UNSPEC_VCVTHF82BF4S])
> > +
> > +(define_int_attr convertfp82bf4
> > +  [(UNSPEC_VCVTBF82BF4S "bf82bf4s")
> > +   (UNSPEC_VCVTHF82BF4S "hf82bf4s")])
>
> please add "s" to iterator to explicit mention its saturate.
Ok.
>
> > +
> > +(define_mode_iterator FP82FP4_PART [V32QI V64QI])
> > +
> > +(define_mode_attr fp82fp4_dest
> > +  [(V32QI "V16QI") (V64QI "V32QI")])
>
> VI1_AVX512_3264 and <ssehalfvecmode> is doing the same job.
> Please first check if there is existing iterator instead of adding new ones.
Ok.
>
> > +(define_expand "vcvt<convertfp82bf4>v16qi"
> > +  [(set (match_operand:V16QI 0 "register_operand")
> > +     (vec_concat:V16QI
> > +       (unspec:V8QI
> > +         [(match_operand:V16QI 1 "register_operand")]
> > +         UNSPEC_CONVERTFP82BF4)
> > +       (match_dup 2)))]
> > +  "TARGET_AVX10V2AUX"
> > +  "operands[2] = CONST0_RTX (V8QImode);")
> > +
> > +(define_insn "*vcvt<convertfp82bf4>v16qi"
> > +  [(set (match_operand:V16QI 0 "register_operand" "=v")
> > +     (vec_concat:V16QI
> > +       (unspec:V8QI
> > +         [(match_operand:V16QI 1 "register_operand" "v")]
> > +         UNSPEC_CONVERTFP82BF4)
> > +       (match_operand:V8QI 2 "const0_operand")))]
> > +  "TARGET_AVX10V2AUX"
> > +  "vcvt<convertfp82bf4>\t{%1, %0|%0, %1}"
> > +  [(set_attr "prefix" "evex")
> > +   (set_attr "mode" "TI")])
> > +
> > +(define_insn "vcvt<convertfp82bf4><mode>"
> > +  [(set (match_operand:<fp82fp4_dest> 0 "register_operand" "=v")
> > +     (unspec:<fp82fp4_dest>
> > +       [(match_operand:FP82FP4_PART 1 "register_operand" "v")]
> > +       UNSPEC_CONVERTFP82BF4))]
> > +  "TARGET_AVX10V2AUX"
> > +  "vcvt<convertfp82bf4>\t{%1, %0|%0, %1}"
> > +  [(set_attr "prefix" "evex")
> > +   (set_attr "mode" "<sseinsnmode>")])
>
> You need to use similar treatment to memory after eliminating not needed
> memory pattern.
Ok.
>
> > +
> > +;; FP4 to FP8 converts (VCVTBF42HF8) with masking
> > +
> > +(define_mode_iterator FP42FP8_MODE [V16QI V32QI V64QI])
> > +
> > +(define_mode_attr fp82fp4_src
> > +  [(V16QI "V16QI") (V32QI "V16QI") (V64QI "V32QI")])
> > +
>
> Similar as the previous patch, you could use VI1_AVX512VL and V16QI.
Ok.
>
> > +(define_mode_attr iptr_fp42fp8
> > +  [(V16QI "q") (V32QI "") (V64QI "")])
> > +
> > +(define_insn "vcvtbf42hf8<mode><mask_name>"
> > +  [(set (match_operand:FP42FP8_MODE 0 "register_operand" "=v")
> > +     (unspec:FP42FP8_MODE
> > +       [(match_operand:<fp82fp4_src> 1 "nonimmediate_operand" "vm")]
> > +       UNSPEC_VCVTBF42HF8))]
> > +  "TARGET_AVX10V2AUX"
> > +
> > "vcvtbf42hf8\t{%1, %0<mask_operand2>|%0<mask_operand2>,
> %<iptr_fp4
> > 2fp8>1}"
> > +  [(set_attr "prefix" "evex")
> > +   (set_attr "mode" "<sseinsnmode>")])
>
> Thx,
> Haochen

Thanks,
Dipesh