[Bug target/128483] New: x86_cse hoists a vector splat from cold early-exit vector loops into the hot outer-loop header

[Reshma.Roy at amd dot com via Gcc-bugs]

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

            Bug ID: 128483
           Summary: x86_cse hoists a vector splat from cold early-exit
                    vector loops into the hot outer-loop header
           Product: gcc
           Version: 17.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: Reshma.Roy at amd dot com
  Target Milestone: ---

Created attachment 66093
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=66093&action=edit
The preprocessed input file to be used for the command mentioned in the
description

Component: target   
Target: x86_64-pc-linux-gnu
Version: 17.0.0 20261012 (experimental)

Testcase (bcast.c), reduced from spec2026 777.zstd_r: ZSTD_RowFindBestMatch's
candidate loop with ZSTD_count force-inlined on both sides of an if/else. After
inlining, find_best contains no calls.

Snippet of the reduced test case:

/* zstd_compress_internal.h:ZSTD_count.  */
static inline __attribute__ ((always_inline)) size_t
count (const BYTE *pIn, const BYTE *pMatch, const BYTE *const pInLimit)
{
  const BYTE *const pStart = pIn;
  const BYTE *const pInLoopLimit = pInLimit - (sizeof (size_t) - 1);

  if (pIn < pInLoopLimit)
    {
      {
        size_t const diff = read64 (pMatch) ^ read64 (pIn);
        if (diff)
          return __builtin_ctzll (diff) >> 3;
      }
      pIn += sizeof (size_t);
      pMatch += sizeof (size_t);
      while (pIn < pInLoopLimit)
        {
          size_t const diff = read64 (pMatch) ^ read64 (pIn);
          if (!diff)
            {
              pIn += sizeof (size_t);
              pMatch += sizeof (size_t);
              continue;
            }
          pIn += __builtin_ctzll (diff) >> 3;
          return (size_t) (pIn - pStart);
        }
    }
  if (pIn < pInLimit - 3 && read32 (pMatch) == read32 (pIn))
    {
      pIn += 4;
      pMatch += 4;
    }
  if (pIn < pInLimit && *pMatch == *pIn)
    pIn++;
  return (size_t) (pIn - pStart);
}

/* zstd_lazy.c:ZSTD_RowFindBestMatch, candidate loop (lines 1254-1271).  */
size_t
find_best (const BYTE *ip, const BYTE *iLimit, const BYTE *base,
           const BYTE *dictBase, const unsigned *matchBuffer,
           size_t numMatches, unsigned dictLimit, int extDict, size_t ml,
           size_t *offset)
{
  for (size_t currMatch = 0; currMatch < numMatches; ++currMatch)
    {
      unsigned const matchIndex = matchBuffer[currMatch];
      size_t currentMl = 0;

      if (!extDict || matchIndex >= dictLimit)
        {
          const BYTE *const match = base + matchIndex;
          if (read32 (match + ml - 3) == read32 (ip + ml - 3))
            currentMl = count (ip, match, iLimit);
        }
      else
        {
          const BYTE *const match = dictBase + matchIndex;
          if (read32 (match) == read32 (ip))
            currentMl = count (ip + 4, match + 4, iLimit) + 4;
        }

      if (currentMl > ml)
        {
          ml = currentMl;
          *offset = currMatch;
          if (ip + currentMl == iLimit)
            break;
        }
    }
  return ml;
}

gcc -O3 -march=x86-64-v4 -S -fdump-rtl-x86_cse-details bcast.i

The early-exit while loop of count is vectorized in both inlined copies in gcc
whereas it's not vectorized in llvm. At each vector-loop exit, the live-out
pMatch is computed as vect_vec_iv + {8,8,8,8,8,8,8,8} (V8DI). Before x86_cse
these two splats are in bb 19 (if arm) and bb 74 (else arm) of find_best. Both
blocks are reached only after the 4-byte read32 check passes and the vector
loop has run.

x86_cse replaces both and places one broadcast at the nearest common
dominator, which is the header of the candidate loop (bb 4, loop 1
header 4) i.e., the loop in the find_best:

  Replace:

  (insn 159 160 162 19 (set (reg:V8DI 286)
          (vec_duplicate:V8DI (reg:DI 287))) "bcast.c":44:15 discrim 100352
9514 {*avx512f_vec_dup_gprv8di}
       (expr_list:REG_DEAD (reg:DI 287)
          (expr_list:REG_EQUAL (const_vector:V8DI [
                      (const_int 8 [0x8]) repeated x8
                  ])
              (nil))))

  with:

  (insn 159 160 162 19 (set (reg:V8DI 286)
          (reg:V8DI 362)) "bcast.c":44:15 discrim 100352 2559
{movv8di_internal}
       ...)
:
(similarly the other splat)
:
  Place:
  (insn 748 66 67 4 (set (reg:V8DI 362)
          (vec_duplicate:V8DI (reg:DI 363))) -1
       (nil))
  after:
  (note 66 635 748 4 [bb 4] NOTE_INSN_BASIC_BLOCK)
  Add:
  (insn 749 66 748 4 (set (reg:DI 363)
          (const_int 8 [0x8])) -1
       (nil))

So, in this test case the vector path is not taken but irrespective of that the
broadcast which is moved to the outer loop header will get executed. This is an
overhead.

For X86_CSE_VEC_DUP and X86_CSE_CONST_VECTOR the x86-cse pass stops at the
nearest common dominator "to avoid extra spills", but it does not check whether
that block runs more often than the blocks it replaces, and no profile
information is read. 

So, can we use profile information also in the x86_cse pass to make the
decision?
The preprocessed test case is also added in the ticket