Re: [PATCH v2 5/7] i386: Don't use operands array as storage

feedable Tue, 25 Aug 2026 03:35:22 -0700


On 25/08/2026 12:51, Hongtao Liu wrote:

On Tue, Aug 25, 2026 at 7:27 AM feedable via Sourceware Forge
<[email protected]> wrote:

From: feedable <[email protected]>

ix86_split_long_move is using parts of the the operands array that are not
set by the recog machionery as storage space, which makes the logic confusing
and also interferes with making the operand count dynamic.

Use local arrays instead.

One question, does the match_dup in the split output template but not
in input pattern also be considered improper storage of operands
array?

No, if an operand appears in the pattern in any way, it should beaccounted for by the recog machinery, at least that's the idea.

.i.e for below pattern in i386.md, input pattern only has 2 operands,
but the output template has match_dup 4 and match_dup 5.
Does genconfig count operand numbers for split's output template? Does
dynamic operand count handle that?

Yes, it counts the total number of operand variables (dup and match), so
this should be allowed as written.

(define_insn_and_split "*cmp<dwi>_doubleword"
  [(set (reg:CCZ FLAGS_REG)
        (compare:CCZ (match_operand:<DWI> 0 "nonimmediate_operand")
                     (match_operand:<DWI> 1 "general_operand")))]
  "ix86_pre_reload_split ()"
  "#"
  "&& 1"
  [(parallel [(set (reg:CCZ FLAGS_REG)
                   (compare:CCZ (ior:DWIH (match_dup 4) (match_dup 5))
                                (const_int 0)))
              (set (match_dup 4) (ior:DWIH (match_dup 4) (match_dup 5)))])]

  split_double_mode (<DWI>mode, &operands[0], 2, &operands[0], &operands[2]);

  operands[4] = gen_reg_rtx (<MODE>mode);

  /* Special case comparisons against -1.  */
  if (operands[1] == constm1_rtx && operands[3] == constm1_rtx)
    {
      emit_insn (gen_and<mode>3 (operands[4], operands[0], operands[2]));
      emit_insn (gen_cmp_1 (<MODE>mode, operands[4], constm1_rtx));
      DONE;
    }

gcc/ChangeLog:

         * config/i386/i386-expand.cc (ix86_split_to_parts): Use automatic
         storage for temporaries.
         (ix86_split_long_move): Likewise.
         * config/i386/i386.md: Likewise.
---
  gcc/config/i386/i386-expand.cc |  26 ++++----
  gcc/config/i386/i386.md        | 113 ++++++++++++++++-----------------
  2 files changed, 69 insertions(+), 70 deletions(-)

diff --git a/gcc/config/i386/i386-expand.cc b/gcc/config/i386/i386-expand.cc
index 34c9599f928e6..8f42297263d3c 100644
--- a/gcc/config/i386/i386-expand.cc
+++ b/gcc/config/i386/i386-expand.cc
@@ -6420,8 +6420,7 @@ ix86_split_to_parts (rtx operand, rtx *parts, 
machine_mode mode)

  /* Emit insns to perform a move or push of DI, DF, XF, and TF values.
     Return false when normal moves are needed; true when all required
-   insns have been emitted.  Operands 2-4 contain the input values
-   int the correct order; operands 5-7 contain the output values.  */
+   insns have been emitted.  */

  void
  ix86_split_long_move (rtx operands[])
@@ -6602,6 +6601,7 @@ ix86_split_long_move (rtx operands[])
        return;
      }

+  rtx move_src[4], move_dst[4];
    /* Choose correct order to not overwrite the source before it is copied.  */
    if ((REG_P (part[0][0])
         && REG_P (part[1][1])
@@ -6615,31 +6615,31 @@ ix86_split_long_move (rtx operands[])
      {
        for (i = 0, j = nparts - 1; i < nparts; i++, j--)
         {
-         operands[2 + i] = part[0][j];
-         operands[6 + i] = part[1][j];
+         move_src[i] = part[0][j];
+         move_dst[i] = part[1][j];
         }
      }
    else
      {
        for (i = 0; i < nparts; i++)
         {
-         operands[2 + i] = part[0][i];
-         operands[6 + i] = part[1][i];
+         move_src[i] = part[0][i];
+         move_dst[i] = part[1][i];
         }
      }

    /* Attempt to locally unCSE nonzero constants.  */
    for (j = 0; j < nparts - 1; j++)
-    if (CONST_INT_P (operands[6 + j])
-       && operands[6 + j] != const0_rtx
-       && REG_P (operands[2 + j]))
+    if (CONST_INT_P (move_dst[j])
+       && move_dst[j] != const0_rtx
+       && REG_P (move_src[j]))
        for (i = j; i < nparts - 1; i++)
-       if (CONST_INT_P (operands[7 + i])
-           && INTVAL (operands[7 + i]) == INTVAL (operands[6 + j]))
-         operands[7 + i] = operands[2 + j];
+       if (CONST_INT_P (move_dst[1 + i])
+           && INTVAL (move_dst[1 + i]) == INTVAL (move_dst[j]))
+         move_dst[1 + i] = move_src[j];

    for (i = 0; i < nparts; i++)
-    emit_move_insn (operands[2 + i], operands[6 + i]);
+    emit_move_insn (move_src[i], move_dst[i]);

    return;
  }
diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md
index 4e18797743f48..63f744daab63e 100644
--- a/gcc/config/i386/i386.md
+++ b/gcc/config/i386/i386.md
@@ -3779,8 +3779,8 @@
    "&& reload_completed"
    [(const_int 0)]
  {
-  operands[4] = gen_lowpart (DImode, operands[1]);
-  split_double_concat (TImode, operands[0], operands[4], operands[2]);
+  rtx temp = gen_lowpart (DImode, operands[1]);
+  split_double_concat (TImode, operands[0], temp, operands[2]);
    DONE;
  })

@@ -3801,8 +3801,8 @@
    "&& reload_completed"
    [(const_int 0)]
  {
-  operands[4] = gen_highpart (DImode, operands[1]);
-  split_double_concat (TImode, operands[0], operands[2], operands[4]);
+  rtx temp = gen_highpart (DImode, operands[1]);
+  split_double_concat (TImode, operands[0], operands[2], temp);
    DONE;
  })

@@ -3821,8 +3821,8 @@
    "&& reload_completed"
    [(const_int 0)]
  {
-  operands[4] = gen_highpart (SImode, operands[1]);
-  split_double_concat (DImode, operands[0], operands[2], operands[4]);
+  rtx temp = gen_highpart (SImode, operands[1]);
+  split_double_concat (DImode, operands[0], operands[2], temp);
    DONE;
  })

@@ -5117,10 +5117,11 @@
    [(const_int 0)]
  {
    rtx bits = GEN_INT (<MODE_SIZE> * BITS_PER_UNIT - 1);
+  rtx lowpart, highpart;

-  split_double_mode (<DWI>mode, &operands[0], 1, &operands[3], &operands[4]);
+  split_double_mode (<DWI>mode, &operands[0], 1, &lowpart, &highpart);

-  emit_move_insn (operands[3], operands[1]);
+  emit_move_insn (lowpart, operands[1]);

    /* Generate a cltd if possible and doing so it profitable.  */
    if ((optimize_function_for_size_p (cfun) || TARGET_USE_CLTD)
@@ -5134,7 +5135,7 @@
        emit_move_insn (operands[2], operands[1]);
        emit_insn (gen_ashr<mode>3_cvt (operands[2], operands[2], bits));
      }
-  emit_move_insn (operands[4], operands[2]);
+  emit_move_insn (highpart, operands[2]);
    DONE;
  })

@@ -5191,29 +5192,30 @@
    [(const_int 0)]
  {
    rtx bits = GEN_INT (<MODE_SIZE> * BITS_PER_UNIT - 1);
+  rtx lowpart, highpart;

-  split_double_mode (<DWI>mode, &operands[0], 1, &operands[3], &operands[4]);
+  split_double_mode (<DWI>mode, &operands[0], 1, &lowpart, &highpart);

-  if (REGNO (operands[3]) != REGNO (operands[1]))
-    emit_move_insn (operands[3], operands[1]);
+  if (REGNO (lowpart) != REGNO (operands[1]))
+    emit_move_insn (lowpart, operands[1]);

    rtx src = operands[1];
-  if (REGNO (operands[3]) == AX_REG)
-    src = operands[3];
+  if (REGNO (lowpart) == AX_REG)
+    src = lowpart;

    /* Generate a cltd if possible and doing so it profitable.  */
    if ((optimize_function_for_size_p (cfun) || TARGET_USE_CLTD)
        && REGNO (src) == AX_REG
-      && REGNO (operands[4]) == DX_REG)
+      && REGNO (highpart) == DX_REG)
      {
-      emit_insn (gen_ashr<mode>3_cvt (operands[4], src, bits));
+      emit_insn (gen_ashr<mode>3_cvt (highpart, src, bits));
        DONE;
      }

-  if (REGNO (operands[4]) != REGNO (operands[1]))
-    emit_move_insn (operands[4], operands[1]);
+  if (REGNO (highpart) != REGNO (operands[1]))
+    emit_move_insn (highpart, operands[1]);

-  emit_insn (gen_ashr<mode>3_cvt (operands[4], operands[4], bits));
+  emit_insn (gen_ashr<mode>3_cvt (highpart, highpart, bits));
    DONE;
  })

@@ -6317,16 +6319,16 @@
         || TARGET_AVX512VL)"
    [(const_int 0)]
  {
-  operands[3] = lowpart_subreg (<ssevecmode>mode, operands[0], <MODE>mode);
-  operands[4] = lowpart_subreg (V4SImode, operands[0], <MODE>mode);
+  rtx lowpart_sse = lowpart_subreg (<ssevecmode>mode, operands[0], <MODE>mode);
+  rtx lowpart_v4si = lowpart_subreg (V4SImode, operands[0], <MODE>mode);

-  emit_insn (gen_sse2_loadld (operands[4],
+  emit_insn (gen_sse2_loadld (lowpart_v4si,
                               CONST0_RTX (V4SImode), operands[1]));

    if (<ssevecmode>mode == V4SFmode)
-    emit_insn (gen_floatv4siv4sf2 (operands[3], operands[4]));
+    emit_insn (gen_floatv4siv4sf2 (lowpart_sse, lowpart_v4si));
    else
-    emit_insn (gen_sse2_cvtdq2pd (operands[3], operands[4]));
+    emit_insn (gen_sse2_cvtdq2pd (lowpart_sse, lowpart_v4si));
    DONE;
  })

@@ -13330,32 +13332,32 @@
    [(const_int:DWIH 0)]
  {
    bool emit_insn_deleted_note_p = false;
+  rtx lowpart[3], highpart[3];
+  split_double_mode (<DWI>mode, &operands[0], 3, lowpart, highpart);

-  split_double_mode (<DWI>mode, &operands[0], 3, &operands[0], &operands[3]);
-
-  if (operands[2] == const0_rtx)
-    emit_move_insn (operands[0], const0_rtx);
-  else if (operands[2] == constm1_rtx)
+  if (lowpart[2] == const0_rtx)
+    emit_move_insn (lowpart[0], const0_rtx);
+  else if (lowpart[2] == constm1_rtx)
      {
-      if (!rtx_equal_p (operands[0], operands[1]))
-       emit_move_insn (operands[0], operands[1]);
+      if (!rtx_equal_p (lowpart[0], lowpart[1]))
+       emit_move_insn (lowpart[0], lowpart[1]);
        else
         emit_insn_deleted_note_p = true;
      }
    else
-    ix86_expand_binary_operator (AND, <MODE>mode, &operands[0], 
TARGET_APX_NDD);
+    ix86_expand_binary_operator (AND, <MODE>mode, &lowpart[0], TARGET_APX_NDD);

-  if (operands[5] == const0_rtx)
-    emit_move_insn (operands[3], const0_rtx);
-  else if (operands[5] == constm1_rtx)
+  if (highpart[2] == const0_rtx)
+    emit_move_insn (highpart[0], const0_rtx);
+  else if (highpart[2] == constm1_rtx)
      {
-      if (!rtx_equal_p (operands[3], operands[4]))
-       emit_move_insn (operands[3], operands[4]);
+      if (!rtx_equal_p (highpart[0], highpart[1]))
+       emit_move_insn (highpart[0], highpart[1]);
        else if (emit_insn_deleted_note_p)
         emit_note (NOTE_INSN_DELETED);
      }
    else
-    ix86_expand_binary_operator (AND, <MODE>mode, &operands[3], 
TARGET_APX_NDD);
+    ix86_expand_binary_operator (AND, <MODE>mode, &highpart[0], 
TARGET_APX_NDD);

    DONE;
  }
@@ -14388,45 +14390,42 @@
       and operands[0] == operands[1], which requires a NOTE_INSN_DELETED.  */
    bool emit_insn_deleted_note_p = false;

-  split_double_mode (<DWI>mode, &operands[0], 3, &operands[0], &operands[3]);
+  rtx lowpart[3], highpart[3];
+  split_double_mode (<DWI>mode, &operands[0], 3, lowpart, highpart);

-  if (operands[2] == const0_rtx)
+  if (lowpart[2] == const0_rtx)
      {
-      if (!rtx_equal_p (operands[0], operands[1]))
-       emit_move_insn (operands[0], operands[1]);
+      if (!rtx_equal_p (lowpart[0], lowpart[1]))
+       emit_move_insn (lowpart[0], lowpart[1]);
        else
         emit_insn_deleted_note_p = true;
      }
-  else if (operands[2] == constm1_rtx)
+  else if (lowpart[2] == constm1_rtx)
      {
        if (<CODE> == IOR)
-       emit_move_insn (operands[0], constm1_rtx);
+       emit_move_insn (lowpart[0], constm1_rtx);
        else
-       ix86_expand_unary_operator (NOT, <MODE>mode, &operands[0],
-                                   TARGET_APX_NDD);
+       ix86_expand_unary_operator (NOT, <MODE>mode, lowpart, TARGET_APX_NDD);
      }
    else
-    ix86_expand_binary_operator (<CODE>, <MODE>mode, &operands[0],
-                                TARGET_APX_NDD);
+    ix86_expand_binary_operator (<CODE>, <MODE>mode, lowpart, TARGET_APX_NDD);

-  if (operands[5] == const0_rtx)
+  if (highpart[2] == const0_rtx)
      {
-      if (!rtx_equal_p (operands[3], operands[4]))
-       emit_move_insn (operands[3], operands[4]);
+      if (!rtx_equal_p (highpart[0], highpart[1]))
+       emit_move_insn (highpart[0], highpart[1]);
        else if (emit_insn_deleted_note_p)
         emit_note (NOTE_INSN_DELETED);
      }
-  else if (operands[5] == constm1_rtx)
+  else if (highpart[2] == constm1_rtx)
      {
        if (<CODE> == IOR)
-       emit_move_insn (operands[3], constm1_rtx);
+       emit_move_insn (highpart[0], constm1_rtx);
        else
-       ix86_expand_unary_operator (NOT, <MODE>mode, &operands[3],
-                                   TARGET_APX_NDD);
+       ix86_expand_unary_operator (NOT, <MODE>mode, highpart, TARGET_APX_NDD);
      }
    else
-    ix86_expand_binary_operator (<CODE>, <MODE>mode, &operands[3],
-                                TARGET_APX_NDD);
+    ix86_expand_binary_operator (<CODE>, <MODE>mode, highpart, TARGET_APX_NDD);

    DONE;
  }
--
2.54.0


--
BR,
Hongtao

Re: [PATCH v2 5/7] i386: Don't use operands array as storage

Reply via email to