This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFC, tentative patch] Adjust cost for conversion expression


On Mon, 28 Nov 2016, Pitchumani Sivanupandi wrote:

> On Thursday 24 November 2016 04:54 PM, Richard Biener wrote:
> > On Thu, 24 Nov 2016, Pitchumani Sivanupandi wrote:
> > 
> > > GCC inlines small functions if the code size after expansion is not
> > > excedded.
> > > For test case (inline.c, avr-gcc -Os -S inline.c) code size become higher
> > > if
> > > 'func2' is inlined. It happens because the CONVERT_EXPR/ NOP_EXPR are
> > > considered
> > > as zero cost expression.
> > > 
> > > Few conversions will cost additional instructions. For targets like AVR
> > > it will cost considerably as it's register size is just one byte.
> > > 
> > > Attached the tentative patch that changes the CONVERT_EXPR/ NOP_EXPR cost
> > > to 1 if the LHS is bigger than RHS and target's word_mode.
> > > 
> > > Is this Ok?
> > > 
> > > Would it be reasonable if cost evaluated as below instead of constant 1?
> > >    if (LHS PRECISION > RHS PRECISION)
> > >      cost = LHS_PRECISION / word_mode - 1
> > >    else
> > >      cost = 0
> > > 
> > > Built GCC for native with bootstrap enabled. No issues.
> > I believe a better check would be tree_nop_conversion_p ().  Thus
> > 
> >    CASE_CONVERT:
> >      return tree_nop_conversion_p (type, TREE_TYPE (op0)) ? 0 : 1;
> > 
> > note that
> > 
> > +      rhs_code = gimple_assign_rhs_code (stmt);
> > +      if ((rhs_code == NOP_EXPR) || (rhs_code == CONVERT_EXPR))
> > +      {
> > +        cost += estimate_operator_cost (rhs_code, weights,
> > +                     gimple_assign_lhs (stmt),
> > +                     gimple_assign_rhs1 (stmt));
> > +      }
> > 
> > is super-ugly - please simply add the type of the expression as an
> > additional argument (see gimple_expr_type (), but the type of the
> > LHS would do as well).
> > 
> > Note that doing this change might require some re-tuning of
> > inliner params, but otherwise it's totally sensible.
> 
> Thanks. Attached the revised patch.
> 
> When reg-tested for x86_64 found following failures.
> FAIL: gcc.dg/uninit-19.c
> FAIL: gcc.dg/vect/vect-104.c
> 
> For uninit-19.c, index to dereference float array is converted to
> long unsigned int and that is not tree_nop_conversion_p. This caused
> function cost to increase and auto inline is rejected.

Hum, yeah...  so the simple tree_nop_conversion_p is too coarse...
(the conversions do result in code generation though).

> I think, this may be huge penalty for target like x86_64 which has rich ISA.
> Any suggestions to avoid hitting such targets?

No good one.  estimate_num_insns is really just a wild guess ...
but one with a huge impact downstream.

Note we're not trying to be clever anywhere but we even do not handle
"gross" mistakes like counting vector additions of 2048 element vectors
(later split to HW supportable sizes) as one.

Yes, we're trying to be clever at call stmts, but that's it.

Richard.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]