On 11/09/18 17:53, James Greenhalgh wrote:
On Mon, Aug 06, 2018 at 11:14:17AM -0500, Vlad Lazar wrote:
Hi,
The patch adds support for the TARGET_COMPUTE_FRAME_LAYOUT hook on AArch64
and removes unneeded frame layout recalculation.
The removed aarch64_layout_frame calls are unnecessary because the functions in
which
they appear will be called during or after the reload pass in which the
TARGET_COMPUTE_FRAME_LAYOUT
hook is called. The if statement in aarch64_layout_frame had the purpose of
avoiding
the extra work from the calls which have been removed and is now redundant.
I'm not sure I understand, I may be missing something as the frame layout
is complex, but I can't get where I need to be to accept your patch from this
comment.
The check you removed ensures that if we're after reload, and the frame is
laid out, we do no additional work. That part I understand, and that would
mean that any post-reload calls were no-ops. Is the argument that all
users of this code that you eliminate are after reload, and consequently
would have hit this no-op path? Can you talk me through why each case is
safe?
Thanks for taking a look at the patch.
Indeed, all the removed calls are happening during or after reload. I'll go
trough all of them
and try to explain the rationale behind.
aarch64_expand_prologue and aarch64_expand_epilogue are called after the
pro_and_epilogue pass,
which runs after reload where TARGET_COMPUTE_FRAME_LAYOUT is called.
aarch64_use_return_insn_p checks explicitly for reload_completed at the
beginning of the function
and returns false if reload has not run. So it's safe to remove the call as the
frame layout is
computed by the time it reaches that point.
aarch64_get_separate_components implements the
TARGET_SHRINK_WRAP_GET_SEPARATE_COMPONENTS hook.
This hook only seems to be used int shrink_wrap.c:try_shrink_wrapping_separate.
The actual origin
of this hook call can be traced back to the pro_and_epilogue pass:
shrink_wrap.c:try_shrink_wrapping_separate <-
function.c:thread_prologue_and_epilogue_insns <-
function.c:rest_of_handle_thread_prologue_and_epilogue (pro_and_epilogue pass
entry point).
Therefore, aarch64_get_separate_components only gets called post reload.
aarch64_get_separate_components implements the INITIAL_ELIMINATION_OFFSET hook,
which is used in:
1. rtlanal.c:get_initial_register_offset: Before using the hook it
checks that reload has
been completed.
2. reload1.c:get_initial_register_offset and
reload1.c:set_initial_elim_offsets: These functions
explicitly call TARGET_COMPUTE_FRAME_LAYOUT before using the hook.
3. lra-eliminitations.c:update_reg_eliminate: The
TARGET_COMPUTE_FRAME_LAYOUT is, again, called
before the INITIAL_ELIMINATION_OFFSET hook is used.
I hope this helps make things a bit clearer.
Thanks,
Vlad
Thanks,
James
gcc/
2018-08-06 Vlad Lazar <[email protected]>
* config/aarch64/aarch64.h (TARGET_COMPUTE_FRAME_LAYOUT): Define.
* config/aarch64/aarch64.c (aarch64_expand_prologue): Remove
aarch64_layout_frame call.
(aarch64_expand_epilogue): Likewise.
(aarch64_initial_elimination_offset): Likewise.
(aarch64_get_separate_components): Likewise.
(aarch64_use_return_insn_p): Likewise.
(aarch64_layout_frame): Remove unneeded check.
