On Wed, Feb 12, 2025 at 5:28 PM Uros Bizjak <ubiz...@gmail.com> wrote:
>
> On Wed, Feb 12, 2025 at 6:25 AM H.J. Lu <hjl.to...@gmail.com> wrote:
> >
> > Don't assume that stack slots can only be accessed by stack or frame
> > registers. We first find all registers defined by stack or frame
> > registers. Then check memory accesses by such registers, including
> > stack and frame registers.
> >
> > gcc/
> >
> > PR target/109780
> > PR target/109093
> > * config/i386/i386.cc (ix86_update_stack_alignment): New.
> > (ix86_find_all_reg_use): Likewise.
> > (ix86_find_max_used_stack_alignment): Also check memory accesses
> > from registers defined by stack or frame registers.
> >
> > gcc/testsuite/
> >
> > PR target/109780
> > PR target/109093
> > * g++.target/i386/pr109780-1.C: New test.
> > * gcc.target/i386/pr109093-1.c: Likewise.
> > * gcc.target/i386/pr109780-1.c: Likewise.
> > * gcc.target/i386/pr109780-2.c: Likewise.
>
> > +/* Find all registers defined with REG. */
> > +
> > +static void
> > +ix86_find_all_reg_use (HARD_REG_SET &stack_slot_access,
> > + unsigned int reg, auto_bitmap &worklist)
> > +{
> > + for (df_ref ref = DF_REG_USE_CHAIN (reg);
> > + ref != NULL;
> > + ref = DF_REF_NEXT_REG (ref))
> > + {
> > + if (DF_REF_IS_ARTIFICIAL (ref))
> > + continue;
> > +
> > + rtx_insn *insn = DF_REF_INSN (ref);
> > + if (!NONDEBUG_INSN_P (insn))
> > + continue;
> > +
> > + rtx set = single_set (insn);
> > + if (!set)
> > + continue;
> > +
>
> Isn't the above condition a bit too limiting? We can have insn with
> multiple sets in the chain.
>
> The issue at hand is the correctness issue (the program will segfault
> if registers are not tracked correctly), not some missing
> optimization. I'd suggest to stay on the safe side and also process
> PARALLELs. Something similar to e.g. store_data_bypass_p from
> recog.cc:
>
> --cut here--
> rtx set = single_set (insn);
> if (set)
> ix86_find_all_reg_use_1(...);
>
> rtx pat = PATTERN (insn);
> if (GET_CODE (pat) != PARALLEL)
> return false;
>
> for (int i = 0; i < XVECLEN (pat, 0); i++)
> {
> rtx exp = XVECEXP (pat, 0, i);
>
> if (GET_CODE (exp) == CLOBBER || GET_CODE (exp) == USE)
> continue;
>
> gcc_assert (GET_CODE (exp) == SET);
>
> ix86_find_all_reg_use_1(...);
> }
> --cut here--
>
> The above will make ix86_find_all_reg_use significantly more robust.
>
> Uros.
Like this?
/* Helper function for ix86_find_all_reg_use. */
static void
ix86_find_all_reg_use_1 (rtx set, HARD_REG_SET &stack_slot_access,
auto_bitmap &worklist)
{
rtx src = SET_SRC (set);
if (MEM_P (src))
return;
rtx dest = SET_DEST (set);
if (!REG_P (dest))
return;
if (TEST_HARD_REG_BIT (stack_slot_access, REGNO (dest)))
return;
/* Add this register to stack_slot_access. */
add_to_hard_reg_set (&stack_slot_access, Pmode, REGNO (dest));
bitmap_set_bit (worklist, REGNO (dest));
}
/* Find all registers defined with REG. */
static void
ix86_find_all_reg_use (HARD_REG_SET &stack_slot_access,
unsigned int reg, auto_bitmap &worklist)
{
for (df_ref ref = DF_REG_USE_CHAIN (reg);
ref != NULL;
ref = DF_REF_NEXT_REG (ref))
{
if (DF_REF_IS_ARTIFICIAL (ref))
continue;
rtx_insn *insn = DF_REF_INSN (ref);
if (!NONDEBUG_INSN_P (insn))
continue;
rtx set = single_set (insn);
if (set)
ix86_find_all_reg_use_1 (set, stack_slot_access, worklist);
rtx pat = PATTERN (insn);
if (GET_CODE (pat) != PARALLEL)
continue;
for (int i = 0; i < XVECLEN (pat, 0); i++)
{
rtx exp = XVECEXP (pat, 0, i);
if (GET_CODE (exp) == CLOBBER || GET_CODE (exp) == USE)
continue;
gcc_assert (GET_CODE (exp) == SET);
ix86_find_all_reg_use_1 (exp, stack_slot_access, worklist);
}
}
}
--
H.J.
