[Bug target/128247] New: Strided broadcast of a single element may overwrite other elements on RISC-V

[law at gcc dot gnu.org via Gcc-bugs]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=128247

            Bug ID: 128247
           Summary: Strided broadcast of a single element may overwrite
                    other elements on RISC-V
           Product: gcc
           Version: 17.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: law at gcc dot gnu.org
  Target Milestone: ---

LLM review noted concerns with this pattern:

(define_expand "@pred_strided_broadcast<mode>"
  [(set (match_operand:V_VLS 0 "register_operand")
        (if_then_else:V_VLS
          (unspec:<VM>
            [(match_operand:<VM> 1 "strided_broadcast_mask_operand")
             (match_operand 4 "vector_length_operand")
             (match_operand 5 "const_int_operand")
             (match_operand 6 "const_int_operand")
             (match_operand 7 "const_int_operand")
             (reg:SI VL_REGNUM)
             (reg:SI VTYPE_REGNUM)] UNSPEC_VPREDICATE)
          (vec_duplicate:V_VLS
            (match_operand:<VEL> 3 "strided_broadcast_operand"))
          (match_operand:V_VLS 2 "vector_merge_operand")))]
  "TARGET_VECTOR"
{
  if (satisfies_constraint_Wb1 (operands[1]))
    {
      /* If we're asked to set a single element (like vmv.s.x but we
         need to go via memory here) and the tail policy is agnostic
         we can overwrite all elements.
         Thus, set the mask to broadcast.  */
      operands[1] = CONSTM1_RTX (<VM>mode);
      if (!satisfies_constraint_vu (operands[2])
          && GET_MODE_SIZE (<VEL>mode) > UNITS_PER_WORD)
        {
          /* Case 2: vmv.s.x (TU, x == memory) ==>
             vl = 0 or 1; + vlse.v (TU) in RV32 system  */
          /* In this case we must not overwrite the residual elements,
             so set the vector length to 0/1.  */
          operands[4] = riscv_vector::gen_avl_for_scalar_move (operands[4]);
        }
    }
})

In particular note the condition that adjusts operands[4].  Now consider this
LLM generated testcase:

/* { dg-do run { target { riscv_v && rv64 } } } */
/* { dg-options "-O2 -march=rv64gcv_zvfhmin -mabi=lp64d" } */
#include "riscv_vector.h"

/* Without Zvfh there is no vfmv.s.f, so the scalar move of an _Float16
   goes through the strided-broadcast fallback.  It must still write only
   element 0 and leave elements 1..vl-1 of the merge operand alone.  */

__attribute__ ((noinline, noclone)) vfloat16m1_t
f (vfloat16m1_t m, _Float16 x, size_t vl)
{
  return __riscv_vfmv_s_f_f16m1_tu (m, x, vl);
}

unsigned short Init[8], Out[8];
volatile unsigned short XB = 0x3c00;    /* 1.0 */
volatile size_t VL = 4;

int
main (void)
{
  for (int i = 0; i < 8; i++)
    Init[i] = 0x1111;
  unsigned short xb = XB;
  _Float16 x;
  __builtin_memcpy (&x, &xb, 2);
  vfloat16m1_t m
    = __riscv_vreinterpret_v_u16m1_f16m1 (__riscv_vle16_v_u16m1 (Init, 8));
  vfloat16m1_t r = f (m, x, VL);
  __riscv_vse16_v_u16m1 (Out, __riscv_vreinterpret_v_f16m1_u16m1 (r), 8);
  if (Out[0] != 0x3c00)
    __builtin_abort ();
  for (int i = 1; i < 8; i++)
    if (Out[i] != 0x1111)
      __builtin_abort ();
  __builtin_exit (0);
}

Generates this for "f":

f:
        fmv.x.s a5,fa0  # 26    [c=4 l=4]  *movhf_softfloat/5
        slli    a5,a5,16
        srai    a5,a5,16
        addi    sp,sp,-16       # 28    [c=4 l=4]  *adddi3/1
        addi    t0,sp,14        # 27    [c=4 l=4]  *adddi3/1
        sh      a5,14(sp)       # 16    [c=4 l=4]  *movhf_softfloat/3
        vsetvli zero,a0,e16,m1,tu,ma    # 39    [c=0 l=4] 
vsetvl_discard_resultdi
        vlse16.v        v8,0(t0),zero   # 12    [c=12 l=4] 
*pred_strided_broadcastrvvm1hf_zvfhmin/3
        addi    sp,sp,16        # 32    [c=4 l=4]  *adddi3/1
        jr      ra              # 33    [c=0 l=4]  simple_return_internal

Note the vsetvl gets VL from a0, which comes from the caller.  So even with the
tail undistubed policy we're writing more elements than we should AFAICT.