diff mbox

Fix finding reg-sets of call insn in collect_fn_hard_reg_usage

Message ID 53A30AB7.1060800@mentor.com
State New
Headers show

Commit Message

Tom de Vries June 19, 2014, 4:07 p.m. UTC
Richard,

atm the moment, when processing a call in collect_fn_hard_reg_usage, we get the 
used regs from the callee, but forget to register the regs in the call insn 
itself (ouch).  This patch fixes this by introducing an extra IOR_HARD_REG_SET.

We also switch the order of find_all_hard_reg_sets and get_call_reg_set_usage. 
There's no point in doing find_all_hard_reg_sets on a call if 
get_call_reg_set_usage returns false.

OK for trunk if bootstrap and reg-test on x86_64 is ok ?

Thanks,
- Tom

Comments

Richard Henderson June 19, 2014, 4:40 p.m. UTC | #1
On 06/19/2014 09:07 AM, Tom de Vries wrote:
> 
> 2014-06-19  Tom de Vries  <tom@codesourcery.com>
> 
> 	* final.c (collect_fn_hard_reg_usage): Add separate IOR_HARD_REG_SET for
> 	get_call_reg_set_usage.

Ok, as far as it goes, but...

It seems like there should be quite a bit of overlap with regs_ever_live here.
 How much of that previous computation can we leverage?

It appears that regs_ever_live includes any register mentioned explicitly, and
thus the only registers it doesn't contain are those killed by the callees.
That should be an easier scan than the rtl, since we have those already
collected in the cgraph.

Sorry I wasn't paying much attention earlier when this was first posted, when
questions like this may have been answered.


r~
Richard Henderson June 19, 2014, 4:47 p.m. UTC | #2
On 06/19/2014 09:40 AM, Richard Henderson wrote:
> It appears that regs_ever_live includes any register mentioned explicitly, and
> thus the only registers it doesn't contain are those killed by the callees.
> That should be an easier scan than the rtl, since we have those already
> collected in the cgraph.

And I forgot to mention it might be worth while to notice simple recursion.
Avoid the early exit path if caller == callee, despite the caller-save info not
being valid.


r~
Tom de Vries June 27, 2014, 9:21 a.m. UTC | #3
On 19-06-14 18:40, Richard Henderson wrote:
> On 06/19/2014 09:07 AM, Tom de Vries wrote:
>>
>> 2014-06-19  Tom de Vries  <tom@codesourcery.com>
>>
>> 	* final.c (collect_fn_hard_reg_usage): Add separate IOR_HARD_REG_SET for
>> 	get_call_reg_set_usage.
>
> Ok, as far as it goes, but...
>
> It seems like there should be quite a bit of overlap with regs_ever_live here.
>   How much of that previous computation can we leverage?
>
> It appears that regs_ever_live includes any register mentioned explicitly, and
> thus the only registers it doesn't contain are those killed by the callees.
> That should be an easier scan than the rtl, since we have those already
> collected in the cgraph.
>
> Sorry I wasn't paying much attention earlier when this was first posted, when
> questions like this may have been answered.
>

Richard,

At the moment, collect_fn_hard_reg_usage is run in pass_final, after final (), 
that is, after the final splitting of insns. The idea is that we use the most 
final representation available, to be on the safe side.

AFAIU, the regs_ever_live information is computed using the df infrastructure, 
which requires the cfg, which is available only until pass_free_cfg for all 
targets (more details in this discussion: 
https://gcc.gnu.org/ml/gcc-patches/2013-05/msg01060.html ). I don't think 
regs_ever_live is guaranteed to be up to date afterwards.

So in order to known whether it's safe and optimal to use regs_ever_live 
instead, the question is whether the passes after pass_free_cfg (are allowed to) 
add or remove sets or clobbers of call_really_used_regs. I don't know the full 
answer there.

Eric, can you comment?

Thanks,
- Tom
Eric Botcazou July 2, 2014, 8:32 a.m. UTC | #4
> So in order to known whether it's safe and optimal to use regs_ever_live
> instead, the question is whether the passes after pass_free_cfg (are allowed
> to) add or remove sets or clobbers of call_really_used_regs. I don't know
> the full answer there.

pass_machine_reorg is run after pass_free_cfg and it can do pretty much what 
it wants so I'd think that the answer is yes.
Jeff Law July 2, 2014, 3:55 p.m. UTC | #5
On 07/02/14 02:32, Eric Botcazou wrote:
>> So in order to known whether it's safe and optimal to use regs_ever_live
>> instead, the question is whether the passes after pass_free_cfg (are allowed
>> to) add or remove sets or clobbers of call_really_used_regs. I don't know
>> the full answer there.
>
> pass_machine_reorg is run after pass_free_cfg and it can do pretty much what
> it wants so I'd think that the answer is yes.
Agreed -- until such time as we decide to place some limits on the 
machine dependent reorg pass.  Right now I think the only limit is the 
result of that pass must be valid RTL.  Pretty weak :(

Jeff
diff mbox

Patch

2014-06-19  Tom de Vries  <tom@codesourcery.com>

	* final.c (collect_fn_hard_reg_usage): Add separate IOR_HARD_REG_SET for
	get_call_reg_set_usage.

diff --git a/gcc/final.c b/gcc/final.c
index e67e84b..bbeb50d 100644
--- a/gcc/final.c
+++ b/gcc/final.c
@@ -4775,12 +4775,16 @@  collect_fn_hard_reg_usage (void)
       if (!NONDEBUG_INSN_P (insn))
 	continue;
 
-      find_all_hard_reg_sets (insn, &insn_used_regs, false);
+      if (CALL_P (insn))
+	{
+	  if (!get_call_reg_set_usage (insn, &insn_used_regs,
+				       call_used_reg_set))
+	    return;
 
-      if (CALL_P (insn)
-	  && !get_call_reg_set_usage (insn, &insn_used_regs, call_used_reg_set))
-	return;
+	  IOR_HARD_REG_SET (function_used_regs, insn_used_regs);
+	}
 
+      find_all_hard_reg_sets (insn, &insn_used_regs, false);
       IOR_HARD_REG_SET (function_used_regs, insn_used_regs);
     }
 
-- 
1.9.1