https://gcc.gnu.org/bugzilla/show_bug.cgi?id=124165
--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
I don't quite understand what this function does. Why is only dominating SET
insns relevant? Possibly the idea is to only check defs from which BB is
reachable? But that's not dominance.
So what does the callers loop try to do (it lacks a comment)? In
stack_slot_access we have collected all regs that may contain stack
slot addresses, then we walk all of those reg uses, and for each
use we walk all defs of the reg (again?) and see whether this def
(set of a stack address) requires a stack frame to be set up.
I'll note that
a) only checking "dominating" defs looks like premature optimization,
in particular this requires to do this for each use of a reg instead
of once for a reg
b) we likely do not have to check all of the stack pointer regs but only
those used for actual stack accesses, this check is defered to the
note_stores call
so I'd say doing ix86_access_stack_p only once, like with the following should
work, given the dominance check isn't correct in the first place. But all
of this looks a bit awkward and I really wonder why we cannot keep the
required stack alignment up-to-date when introducing stack objects (we
do that during RTL expansion, right?).
diff --git a/gcc/config/i386/i386.cc b/gcc/config/i386/i386.cc
index 52f82185e32..6e2db8cbb45 100644
--- a/gcc/config/i386/i386.cc
+++ b/gcc/config/i386/i386.cc
@@ -8715,7 +8715,7 @@ ix86_find_all_reg_uses (HARD_REG_SET ®set,
defined in a basic block that dominates the block where it is used. */
static bool
-ix86_access_stack_p (unsigned int regno, basic_block bb,
+ix86_access_stack_p (unsigned int regno, basic_block,
HARD_REG_SET &set_up_by_prologue,
HARD_REG_SET &prologue_used)
{
@@ -8728,15 +8728,11 @@ ix86_access_stack_p (unsigned int regno, basic_block
bb,
&& !DF_REF_FLAGS_IS_SET (def, DF_REF_MAY_CLOBBER)
&& !DF_REF_FLAGS_IS_SET (def, DF_REF_MUST_CLOBBER))
{
- basic_block set_bb = DF_REF_BB (def);
- if (dominated_by_p (CDI_DOMINATORS, bb, set_bb))
- {
- rtx_insn *insn = DF_REF_INSN (def);
- /* Return true if INSN requires stack. */
- if (requires_stack_frame_p (insn, prologue_used,
- set_up_by_prologue))
- return true;
- }
+ rtx_insn *insn = DF_REF_INSN (def);
+ /* Return true if INSN requires stack. */
+ if (requires_stack_frame_p (insn, prologue_used,
+ set_up_by_prologue))
+ return true;
}
return false;
@@ -8837,6 +8833,7 @@ ix86_find_max_used_stack_alignment (unsigned int
&stack_alignment,
|| ix86_access_stack_p (regno, BLOCK_FOR_INSN (insn),
set_up_by_prologue, prologue_used))
{
+ add_to_hard_reg_set (&hard_stack_slot_access, Pmode, regno);
/* Update stack alignment if REGNO is used for stack
access. */
data.reg = DF_REF_REG (ref);