Re: fuse-caller-save - hook format
On 22/04/14 18:13, Tom de Vries wrote: On 22-04-14 18:18, Richard Sandiford wrote: Tom de Vries tom_devr...@mentor.com writes: On 22-04-14 17:27, Richard Sandiford wrote: Tom de Vries tom_devr...@mentor.com writes: 2. post_expand_call_insn. A utility hook to facilitate adding the clobbers to CALL_INSN_FUNCTION_USAGE. Why is this needed though? Like I say, I think targets should update CALL_INSN_FUNCTION_USAGE when emitting calls as part of the call expander. Splitting the functionality of the call expanders across the define_expand and a new hook just makes things unnecessarily complicated IMO. Richard, It is not needed, but it is convenient. There are targets where the define_expands for calls use the rtl template. Having to add clobbers to the CALL_INSN_FUNCTION_USAGE for such a target means you cannot use the rtl template any more and instead need to generate all needed RTL insns in C code. This hook means that you can keep using the rtl template, which is less intrusive for those targets. [ switching order of questions ] Which target do you have in mind? Aarch64. But if the target is simple enough to use a single call pattern for call cases, wouldn't it be possible to add the clobber directly to the call pattern? I think that can be done, but that feels intrusive as well. I thought the reason that we added these clobbers to CALL_INSN_FUNCTION_USAGE was exactly because we did not want to add them to the rtl patterns? But, if the maintainer is fine with that, so am I. Richard Earnshaw, are you ok with adding the IP0_REGNUM/IP1_REGNUM clobbers to all the call patterns in the Aarch64 target? The alternatives are: - rewrite the call expansions not to use the rtl templates, and add the clobbers there to CALL_INSN_FUNCTION_USAGE - get the post_expand_call_insn hook approved and use that to add the clobbers to CALL_INSN_FUNCTION_USAGE. what is your preference? It seems undesirable to me to be hard-coding ABI constraints directly into the MD file. It's not a major problem while there is one ABI that's common to all targets; but it's quite possible this sort of detail would change from platform to platform. That sort of churn is best kept out of the MD file itself, if at all possible. R. Thanks, - Tom
Re: fuse-caller-save - hook format
On 17-04-14 18:49, Vladimir Makarov wrote: I see. I guess your proposed solution is ok then. Vladimir, Richard, I've updated the fuse-caller-save patch series to model non-callee call clobbers in CALL_INSN_FUNCTION_USAGE. There are 2 new hooks: 1. call_fusage_contains_non_callee_clobbers. A hook to indicate whether a target has added the non-callee call clobbers to CALL_INSN_FUNCTION_USAGE, meaning it's safe to do the fuse-caller-save optimization. 2. post_expand_call_insn. A utility hook to facilitate adding the clobbers to CALL_INSN_FUNCTION_USAGE. I've bootstrapped and reg-tested on x86_64, and I've build and reg-tested on MIPS. The series now looks like: 1 -fuse-caller-save - Add command line option 2 -fuse-caller-save - Add new reg-note REG_CALL_DECL 3 Add implicit parameter to find_all_hard_reg_sets 4 Register CALL_INSN_FUNCTION_USAGE in find_all_hard_reg_sets 5 Add call_fusage_contains_non_callee_clobbers hook 6 -fuse-caller-save - Collect register usage information 7 -fuse-caller-save - Use collected register usage information 8 -fuse-caller-save - Enable by default at O2 and higher 9 -fuse-caller-save - Add documentation 10 -fuse-caller-save - Add test-case 11 Add post_expand_call_insn hook 12 Add clobber_reg 13 -fuse-caller-save - Enable for MIPS 14 -fuse-caller-save - Enable for ARM 15 -fuse-caller-save - Enable for AArch64 16 -fuse-caller-save - Support in lra 17 -fuse-caller-save - Enable for i386 The submission/approval status is: 1-3, 7-10, 16: approved 4: submitted, pinged Eric Botcazou 16-04-2014 5, 11: new hook, need to submit 6, 14-15: approved earlier, but need to resubmit due to updated hook 12: new utility patch, need to submit 13: need to resubmit due to updated hook 17: need to submit I'll post the patches that need (re)submitting. Thanks, - Tom
Re: fuse-caller-save - hook format
Tom de Vries tom_devr...@mentor.com writes: 2. post_expand_call_insn. A utility hook to facilitate adding the clobbers to CALL_INSN_FUNCTION_USAGE. Why is this needed though? Like I say, I think targets should update CALL_INSN_FUNCTION_USAGE when emitting calls as part of the call expander. Splitting the functionality of the call expanders across the define_expand and a new hook just makes things unnecessarily complicated IMO. Thanks, Richard
Re: fuse-caller-save - hook format
On 22-04-14 17:27, Richard Sandiford wrote: Tom de Vries tom_devr...@mentor.com writes: 2. post_expand_call_insn. A utility hook to facilitate adding the clobbers to CALL_INSN_FUNCTION_USAGE. Why is this needed though? Like I say, I think targets should update CALL_INSN_FUNCTION_USAGE when emitting calls as part of the call expander. Splitting the functionality of the call expanders across the define_expand and a new hook just makes things unnecessarily complicated IMO. Richard, It is not needed, but it is convenient. There are targets where the define_expands for calls use the rtl template. Having to add clobbers to the CALL_INSN_FUNCTION_USAGE for such a target means you cannot use the rtl template any more and instead need to generate all needed RTL insns in C code. This hook means that you can keep using the rtl template, which is less intrusive for those targets. Thanks, - Tom Thanks, Richard
Re: fuse-caller-save - hook format
Tom de Vries tom_devr...@mentor.com writes: On 22-04-14 17:27, Richard Sandiford wrote: Tom de Vries tom_devr...@mentor.com writes: 2. post_expand_call_insn. A utility hook to facilitate adding the clobbers to CALL_INSN_FUNCTION_USAGE. Why is this needed though? Like I say, I think targets should update CALL_INSN_FUNCTION_USAGE when emitting calls as part of the call expander. Splitting the functionality of the call expanders across the define_expand and a new hook just makes things unnecessarily complicated IMO. Richard, It is not needed, but it is convenient. There are targets where the define_expands for calls use the rtl template. Having to add clobbers to the CALL_INSN_FUNCTION_USAGE for such a target means you cannot use the rtl template any more and instead need to generate all needed RTL insns in C code. This hook means that you can keep using the rtl template, which is less intrusive for those targets. But if the target is simple enough to use a single call pattern for call cases, wouldn't it be possible to add the clobber directly to the call pattern? Which target do you have in mind? Thanks, Richard
Re: fuse-caller-save - hook format
On 22-04-14 18:18, Richard Sandiford wrote: Tom de Vries tom_devr...@mentor.com writes: On 22-04-14 17:27, Richard Sandiford wrote: Tom de Vries tom_devr...@mentor.com writes: 2. post_expand_call_insn. A utility hook to facilitate adding the clobbers to CALL_INSN_FUNCTION_USAGE. Why is this needed though? Like I say, I think targets should update CALL_INSN_FUNCTION_USAGE when emitting calls as part of the call expander. Splitting the functionality of the call expanders across the define_expand and a new hook just makes things unnecessarily complicated IMO. Richard, It is not needed, but it is convenient. There are targets where the define_expands for calls use the rtl template. Having to add clobbers to the CALL_INSN_FUNCTION_USAGE for such a target means you cannot use the rtl template any more and instead need to generate all needed RTL insns in C code. This hook means that you can keep using the rtl template, which is less intrusive for those targets. [ switching order of questions ] Which target do you have in mind? Aarch64. But if the target is simple enough to use a single call pattern for call cases, wouldn't it be possible to add the clobber directly to the call pattern? I think that can be done, but that feels intrusive as well. I thought the reason that we added these clobbers to CALL_INSN_FUNCTION_USAGE was exactly because we did not want to add them to the rtl patterns? But, if the maintainer is fine with that, so am I. Richard Earnshaw, are you ok with adding the IP0_REGNUM/IP1_REGNUM clobbers to all the call patterns in the Aarch64 target? The alternatives are: - rewrite the call expansions not to use the rtl templates, and add the clobbers there to CALL_INSN_FUNCTION_USAGE - get the post_expand_call_insn hook approved and use that to add the clobbers to CALL_INSN_FUNCTION_USAGE. what is your preference? Thanks, - Tom
Re: fuse-caller-save - hook format
On 2014-04-16, 3:19 PM, Tom de Vries wrote:
Vladimir,
All patches for the fuse-caller-save optimization have been ok-ed. The only
part
not approved is the MIPS-specific part.
The objection of Richard S. is not so much the patch itself, but more the idea
of the hook fn_other_hard_reg_usage.
For clarity, I'm restating the current hook definition here:
...
+@deftypefn {Target Hook} bool TARGET_FN_OTHER_HARD_REG_USAGE (struct
hard_reg_set_container *@var{regs})
Add any hard registers to @var{regs} that are set or clobbered by a call to
the
function. This hook only needs to add registers that cannot be found by
examination of the final RTL representation of a function. This hook returns
true if it managed to determine which registers need to be added. The default
version of this hook returns false.
...
Richard prefers to, rather than having a hook specifying what registers are
implicitly clobbered, adding those clobbers to CALL_INSN_FUNCTION_USAGE.
I can see these possibilities (and perhaps there are more):
1. We go with Richards proposal: we make each target responsible for adding
these clobbers in CALL_INSN_FUNCTION_USAGE, and use a hook called f.i.
targetm.fuse_caller_save_p or targetm.implicit_call_clobbers_in_fusage_p, to
indicate whether a target has taken care of that, meaning it's safe to do the
fuse-caller-save optimization.
2. A mixed solution: we make each target responsible for specifying which
clobbers need to be added in CALL_INSN_FUNCTION_USAGE, using a hook called
f.i.
targetm.call_clobbered_regs, and add generic code to add those clobbers to
CALL_INSN_FUNCTION_USAGE.
3. We stick with the current, approved hook format, and try to convince
Richard
to live with it.
Since you are a register allocator maintainer, familiar with the
fuse-caller-save optimization, and have approved the original hook, I would
like
to ask you to make a decision on how to proceed from here.
I have no preferences and it is a matter of taste. Each solution has
own advantages and disadvantages. Putting this info into
CALL_INSN_FUNCTION_USAGE helps GCC developing a lot but it has a big
drawback in RTL memory footprint (especially for some targets which have
a lot of regs like AM29k or IA64). On the order hand analogous approach
is already used in DF-infrastructure (which would be nice to fix it imho).
Still between GCC users and GCC developers, I'd prefer solution (even
the effect on amount of resources used by GCC is quite insignificant)
for users as their number is in a few magnitudes more then the developers.
But I can live with any solution. So it is up to you. I am flexible.
Re: fuse-caller-save - hook format
Vladimir Makarov vmaka...@redhat.com writes:
On 2014-04-16, 3:19 PM, Tom de Vries wrote:
Vladimir,
All patches for the fuse-caller-save optimization have been ok-ed. The
only part
not approved is the MIPS-specific part.
The objection of Richard S. is not so much the patch itself, but more the
idea
of the hook fn_other_hard_reg_usage.
For clarity, I'm restating the current hook definition here:
...
+@deftypefn {Target Hook} bool TARGET_FN_OTHER_HARD_REG_USAGE (struct
hard_reg_set_container *@var{regs})
Add any hard registers to @var{regs} that are set or clobbered by a
call to the
function. This hook only needs to add registers that cannot be found by
examination of the final RTL representation of a function. This hook returns
true if it managed to determine which registers need to be added. The
default
version of this hook returns false.
...
Richard prefers to, rather than having a hook specifying what registers are
implicitly clobbered, adding those clobbers to CALL_INSN_FUNCTION_USAGE.
I can see these possibilities (and perhaps there are more):
1. We go with Richards proposal: we make each target responsible for adding
these clobbers in CALL_INSN_FUNCTION_USAGE, and use a hook called f.i.
targetm.fuse_caller_save_p or targetm.implicit_call_clobbers_in_fusage_p, to
indicate whether a target has taken care of that, meaning it's safe to do the
fuse-caller-save optimization.
2. A mixed solution: we make each target responsible for specifying which
clobbers need to be added in CALL_INSN_FUNCTION_USAGE, using a hook
called f.i.
targetm.call_clobbered_regs, and add generic code to add those clobbers to
CALL_INSN_FUNCTION_USAGE.
3. We stick with the current, approved hook format, and try to
convince Richard
to live with it.
Since you are a register allocator maintainer, familiar with the
fuse-caller-save optimization, and have approved the original hook, I
would like
to ask you to make a decision on how to proceed from here.
I have no preferences and it is a matter of taste. Each solution has
own advantages and disadvantages. Putting this info into
CALL_INSN_FUNCTION_USAGE helps GCC developing a lot but it has a big
drawback in RTL memory footprint (especially for some targets which have
a lot of regs like AM29k or IA64). On the order hand analogous approach
is already used in DF-infrastructure (which would be nice to fix it imho).
Still between GCC users and GCC developers, I'd prefer solution (even
the effect on amount of resources used by GCC is quite insignificant)
for users as their number is in a few magnitudes more then the developers.
Hmm, but you're talking like there are going to be a lot of these registers.
This isn't about which registers are call-clobbered or call-saved according
to the ABI (that's already available in other places). All we want here
are the set of registers that are clobbered _in the caller_ before reaching
the callee or after the callee has returned.
So although IA-64 has lots of registers, the caller doesn't AFAIK use
lots of registers in the process of making the call.
On all targets we should be talking about one or two registers here.
Thanks,
Richard
Re: fuse-caller-save - hook format
On 2014-04-17, 11:29 AM, Richard Sandiford wrote:
Vladimir Makarov vmaka...@redhat.com writes:
On 2014-04-16, 3:19 PM, Tom de Vries wrote:
Vladimir,
All patches for the fuse-caller-save optimization have been ok-ed. The
only part
not approved is the MIPS-specific part.
The objection of Richard S. is not so much the patch itself, but more the idea
of the hook fn_other_hard_reg_usage.
For clarity, I'm restating the current hook definition here:
...
+@deftypefn {Target Hook} bool TARGET_FN_OTHER_HARD_REG_USAGE (struct
hard_reg_set_container *@var{regs})
Add any hard registers to @var{regs} that are set or clobbered by a
call to the
function. This hook only needs to add registers that cannot be found by
examination of the final RTL representation of a function. This hook returns
true if it managed to determine which registers need to be added. The default
version of this hook returns false.
...
Richard prefers to, rather than having a hook specifying what registers are
implicitly clobbered, adding those clobbers to CALL_INSN_FUNCTION_USAGE.
I can see these possibilities (and perhaps there are more):
1. We go with Richards proposal: we make each target responsible for adding
these clobbers in CALL_INSN_FUNCTION_USAGE, and use a hook called f.i.
targetm.fuse_caller_save_p or targetm.implicit_call_clobbers_in_fusage_p, to
indicate whether a target has taken care of that, meaning it's safe to do the
fuse-caller-save optimization.
2. A mixed solution: we make each target responsible for specifying which
clobbers need to be added in CALL_INSN_FUNCTION_USAGE, using a hook
called f.i.
targetm.call_clobbered_regs, and add generic code to add those clobbers to
CALL_INSN_FUNCTION_USAGE.
3. We stick with the current, approved hook format, and try to
convince Richard
to live with it.
Since you are a register allocator maintainer, familiar with the
fuse-caller-save optimization, and have approved the original hook, I
would like
to ask you to make a decision on how to proceed from here.
I have no preferences and it is a matter of taste. Each solution has
own advantages and disadvantages. Putting this info into
CALL_INSN_FUNCTION_USAGE helps GCC developing a lot but it has a big
drawback in RTL memory footprint (especially for some targets which have
a lot of regs like AM29k or IA64). On the order hand analogous approach
is already used in DF-infrastructure (which would be nice to fix it imho).
Still between GCC users and GCC developers, I'd prefer solution (even
the effect on amount of resources used by GCC is quite insignificant)
for users as their number is in a few magnitudes more then the developers.
Hmm, but you're talking like there are going to be a lot of these registers.
Yes, you are right. That is what I thought. I should have read Tom's
email with more attention.
This isn't about which registers are call-clobbered or call-saved according
to the ABI (that's already available in other places). All we want here
are the set of registers that are clobbered _in the caller_ before reaching
the callee or after the callee has returned.
So although IA-64 has lots of registers, the caller doesn't AFAIK use
lots of registers in the process of making the call.
On all targets we should be talking about one or two registers here.
I see. I guess your proposed solution is ok then.
Re: fuse-caller-save - hook format
Tom de Vries tom_devr...@mentor.com writes:
Vladimir,
All patches for the fuse-caller-save optimization have been ok-ed. The only
part
not approved is the MIPS-specific part.
The objection of Richard S. is not so much the patch itself, but more the idea
of the hook fn_other_hard_reg_usage.
For clarity, I'm restating the current hook definition here:
...
+@deftypefn {Target Hook} bool TARGET_FN_OTHER_HARD_REG_USAGE (struct
hard_reg_set_container *@var{regs})
Add any hard registers to @var{regs} that are set or clobbered by a call to
the
function. This hook only needs to add registers that cannot be found by
examination of the final RTL representation of a function. This hook returns
true if it managed to determine which registers need to be added. The default
version of this hook returns false.
...
Just for the record, I think this hook was defined as applying during final
to a potential callee function, rather than applying to a particular call.
I.e., after calculating which registers a function uses, the code would
add the registers returned by this hook to the set.
My objection to that was that the set of registers clobbered while making
a call depends on the caller.
With that proviso...
Richard prefers to, rather than having a hook specifying what registers are
implicitly clobbered, adding those clobbers to CALL_INSN_FUNCTION_USAGE.
...I agree this is a fair summary.
I can see these possibilities (and perhaps there are more):
1. We go with Richards proposal: we make each target responsible for adding
these clobbers in CALL_INSN_FUNCTION_USAGE, and use a hook called f.i.
targetm.fuse_caller_save_p or targetm.implicit_call_clobbers_in_fusage_p, to
indicate whether a target has taken care of that, meaning it's safe to do the
fuse-caller-save optimization.
2. A mixed solution: we make each target responsible for specifying which
clobbers need to be added in CALL_INSN_FUNCTION_USAGE, using a hook called
f.i.
targetm.call_clobbered_regs, and add generic code to add those clobbers to
CALL_INSN_FUNCTION_USAGE.
3. We stick with the current, approved hook format, and try to convince
Richard
to live with it.
The reason I don't like (2) is that, on targets like MIPS where the
different call cases are quite complicated, the implementation of
the hook would need to follow the same logic as the call expander
to figure out which case applies. It just seems more elegant to me
to add the clobbers when emitting the call.
IMO CALL_INSN_FUNCTION_USAGE is like a varargs part of the call pattern.
In other words it's a way of allowing the set of uses and clobbers to
vary from call to call without having to define lots of different call
define_insns. If you look at it like that, adding the clobbers when
emitting the insn seems more correct as well.
Thanks,
Richard
Re: fuse-caller-save - hook format
On 04/16/14 13:41, Richard Sandiford wrote: IMO CALL_INSN_FUNCTION_USAGE is like a varargs part of the call pattern. In other words it's a way of allowing the set of uses and clobbers to vary from call to call without having to define lots of different call define_insns. If you look at it like that, adding the clobbers when emitting the insn seems more correct as well. This seems like a better direction to me as well. There's just something clean and elegant about attaching this stuff to the CALL_INSN. jeff
