Hi Jeff,

> It's not clear we really need an sp-only form for the epilogue
> barrier.  My recollection is the issues the stack tie is trying
> to fix are limited to cases where we have a frame pointer.  But
> I don't think having it is necessarily bad, just perhaps not
> strictly necessary.

For restores that directly use SP-relative addresses, I agree that
the barrier may not be strictly required.  However, the current
no-frame-pointer epilogue already emits stack_tie (sp, s0).  The
problem is that it refers to an unsaved s0.

Removing the no-frame-pointer tie entirely would change existing
scheduling behavior.  It would also require changing
riscv_sr_match_epilogue to match an epilogue without a stack tie,
while still requiring the tie for a frame-pointer epilogue.  This
broadens the change and may introduce unintended effects.

Therefore, I think a separate SP-only pattern is the simplest and
lowest-risk solution.  It replaces stack_tie (sp, s0) with
stack_tie_sp (sp), preserves the existing barrier and epilogue
shape, and removes only the incorrect s0 reference.  The matcher
only needs to recognize one additional instruction code.

> Presumably you mean it removes the CFI note that tells the
> unwinder to find the frame in t3 when we're in a probing loop?

What can be removed here is the t3 = sp move, not the CFI note.
The commit message did not make this distinction clear.  Sorry that
this was not described clearly.

stack_tie (sp, sp) refers to another approach I considered:
removing the following condition from the existing two-operand
stack_tie pattern so it could also be used by no-frame-pointer
epilogues:

  !rtx_equal_p (operands[0], operands[1])

In the probing path, the REG_CFA_DEF_CFA note is attached to the
frame-related instruction before t3 = sp.  The stack_tie (sp, t3)
is the normal RTL use that keeps t3 = sp live.  If equal operands
are allowed, copy propagation can turn it into stack_tie (sp, sp).
This makes t3 = sp dead and allows it to be deleted, while the CFA
note on the earlier instruction still names t3 as the CFA register.

The current approach keeps the inequality condition and adds a
separate one-operand stack_tie_sp pattern, so the probing path is
unaffected.

> In general HOST_WIDE_INT_1U rather than 1U; however, in this
> case 1U is correct because the mask field is just an unsigned int.

Yes.  frame.mask has type unsigned int, and
HARD_FRAME_POINTER_REGNUM is 8.  Therefore, 1U << 8 has the
correct type and width, and HOST_WIDE_INT_1U is unnecessary.

Any other questions or suggestions?

BR,
Jin Ma

Reply via email to