| Message ID | 37378DC5BCD0EE48BA4B082E0B55DFAA429D6689@XAP-PVEXMBX02.xlnx.xilinx.com |
|---|---|
| State | New |
| Headers | show |
On 12/16/2015 12:54 PM, Ajit Kumar Agarwal wrote: > > The estimate on target_clobbered_registers based on the call_used arrays is not correct. This is the worst case > heuristics on the estimate on target_clobbered_registers. This disables many of the loop Invariant code motion > opportunities in presence of call. Instead of considering the spill cost we consider only the target_reg_cost > aggressively. Right now we are in stage 3, and not accepting anything but bug fixes. Please resubmit once stage 1 reopens. > > diff --git a/gcc/cfgloopanal.c b/gcc/cfgloopanal.c > --- a/gcc/cfgloopanal.c > +++ b/gcc/cfgloopanal.c Patch submissions should include ChangeLog entries. > @@ -373,15 +373,23 @@ estimate_reg_pressure_cost (unsigned n_new, unsigned n_old, bool speed, > > /* If there is a call in the loop body, the call-clobbered registers > are not available for loop invariants. */ > + > if (call_p) > available_regs = available_regs - target_clobbered_regs; > - > + Spurious whitespace change. > /* If we have enough registers, we should use them and not restrict > the transformations unnecessarily. */ > if (regs_needed + target_res_regs <= available_regs) > return 0; > > - if (regs_needed <= available_regs) > + /* Estimation of target_clobbered register is based on the call_used > + arrays which is not the right estimate for the clobbered register > + used in called function. Instead of considering the spill cost we > + consider only the reg_cost aggressively. */ > + > + if ((regs_needed <= available_regs) > + || (call_p && (regs_needed <= > + (available_regs + target_clobbered_regs)))) Formatting issues - unnecessary parens, and bad line split. This would be written as if (regs_needed <= available_regs || (call_p && regs_needed <= available_regs + target_clobbered_regs)) But then, I think the whole thing could be simplified by just keeping the original available_regs value around (before subtracting target_clobbered_regs) and have just one comparison here. Once again I find myself unsure whether this change is actually better or just different. The existing code kind of makes sense to me - if a reg is call-clobbered and there's a call in the loop, it can't hold an invariant. In any case, this is a question for stage 1. Bernd
On 12/16/2015 12:54 PM, Ajit Kumar Agarwal wrote: > /* If there is a call in the loop body, the call-clobbered registers > are not available for loop invariants. */ > + > if (call_p) > available_regs = available_regs - target_clobbered_regs; > - > + > /* If we have enough registers, we should use them and not restrict > the transformations unnecessarily. */ > if (regs_needed + target_res_regs <= available_regs) > return 0; Just a thought, one thing that might make sense here is counting some of the target_res_regs among the clobbered ones. This would become int res_regs = target_res_regs; if (call_p) { available_regs = available_regs - target_clobbered_regs; res_regs /= 2; } /* If we have enough registers, we should use them and not restrict the transformations unnecessarily. */ if (regs_needed + res_regs <= available_regs) return 0; It's all a bit crude, before and after, but such a change might be justifiable. Bernd
diff --git a/gcc/cfgloopanal.c b/gcc/cfgloopanal.c --- a/gcc/cfgloopanal.c +++ b/gcc/cfgloopanal.c @@ -373,15 +373,23 @@ estimate_reg_pressure_cost (unsigned n_new, unsigned n_old, bool speed, /* If there is a call in the loop body, the call-clobbered registers are not available for loop invariants. */ + if (call_p) available_regs = available_regs - target_clobbered_regs; - + /* If we have enough registers, we should use them and not restrict the transformations unnecessarily. */ if (regs_needed + target_res_regs <= available_regs) return 0; - if (regs_needed <= available_regs) + /* Estimation of target_clobbered register is based on the call_used + arrays which is not the right estimate for the clobbered register + used in called function. Instead of considering the spill cost we + consider only the reg_cost aggressively. */ + + if ((regs_needed <= available_regs) + || (call_p && (regs_needed <= + (available_regs + target_clobbered_regs)))) /* If we are close to running out of registers, try to preserve them. */ cost = target_reg_cost [speed] * n_new;