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: [PATCH] Inline handled_component_p


On Thu, 16 Apr 2009, Martin Jambor wrote:

> Hi,
> 
> On Thu, Apr 16, 2009 at 12:59:53PM +0200, Richard Guenther wrote:
> > On Thu, 16 Apr 2009, Paolo Bonzini wrote:
> > 
> > > 
> > > > ! static inline bool
> > > > ! handled_component_p (const_tree t)
> > > > ! {
> > > > !   switch (TREE_CODE (t))
> > > > !     {
> > > > !     case BIT_FIELD_REF:
> > > > !     case COMPONENT_REF:
> > > > !     case ARRAY_REF:
> > > > !     case ARRAY_RANGE_REF:
> > > > !     case VIEW_CONVERT_EXPR:
> > > > !     case REALPART_EXPR:
> > > > !     case IMAGPART_EXPR:
> > > > !       return true;
> > > > ! 
> > > > !     default:
> > > > !       return false;
> > > > !     }
> > > > ! }
> > > 
> > > Given this, why not use IN_RANGE?
> > 
> > That would make ordering in tree.def a correctness issue.  Why not
> > let the compiler optimize this?
> > 
> > But I see we switch-convert the above.  Ugh.  Martin, can you
> > investigate?  Testcase:
> > 
> > typedef enum { a = 5, b = 6, c = 7, d = 8, e = 9 } X;
> > 
> > int h1 (X x)
> > {
> >   switch (x) {
> >   case a:
> >   case b:
> >   case c:
> >   case d:
> >   case e:
> >     return 1;
> >   default:
> >     return 0;
> >     }
> > }
> > 
> > switch conversion should see that the constants are all 1 and simply
> > substitute that, not generating the lookup table (CCP isn't good
> > enough to constant propagate from the all-ones array from a variable
> > index load).
> 
> Hm, I guess that when the case statement has only two branches, switch
> conversion should not  trigger and IIRC there is no  such check in the
> pass.  Is that what you mean?

No, it would be nice if switch conversion instead of a load from
the constant table would set the result to the other single
non-constant value.  Nothing else does this optimization.

> I am not sure what is the minimum reasonable number of branches.  With
> three branches we  trade a branch for  a load so it at  least does not
> make things evidently worse...
> 
> I'll prepare a patch avoiding the two branch scenario shortly,

Please instead make the optimization work ;)  Thus, instead of

h1 (X x)
{
  X csui.2;
  X D.2683;
  <unnamed-signed:64> csti.0;
  int D.1606;

<bb 2>:
  D.2683_4 = x_2(D) + 4294967291;
  csui.2_6 = x_2(D);
  csui.2_7 = csui.2_6 + 4294967291;
  if (csui.2_7 <= 4)
    goto <bb 6> (<L9>);
  else
    goto <bb 5> (<L8>);

<L8>:
  D.1606_5 = 0;
  goto <bb 4> (<L10>);

<L9>:
  csti.0_8 = (<unnamed-signed:64>) D.2683_4;
  D.1606_3 = CSWTCH.1[csti.0_8];

  # D.1606_1 = PHI <D.1606_3(6), D.1606_5(5)>
<L10>:
<L7>:
  return D.1606_1;

}

emit the same with

  D.1606_3 = 1;

instead.

Thanks,
Richard.


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