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: PPC64 HTM support (was [buildbot] r201513: Invalid cast)


On Tue, 2013-08-06 at 22:01 -0500, Peter Bergner wrote:
> Oleg Endo wrote:
> > Speaking of GEN_FCN usage in rs6000.c.  The recently added HTM builtin
> > code has one interesting piece:
> > 
> > static rtx
> > htm_expand_builtin (tree exp, rtx target, bool * expandedp)
> > {
> > ...
> > switch (nopnds)
> >  {
> >  case 0:
> >    pat = GEN_FCN (icode) (NULL_RTX);
> >    break;
> >  case 1:
> >    pat = GEN_FCN (icode) (op[0]);
> >    break;
> > 
> > The 'case 0' looks suspicious.
> 
> Yes, the "case 0:" is not needed.  It was needed at one point
> when I originally modeled this code after Andreas's s390 changes.
> The code then passed in the target rtx as a separate parameter
> from the operands, so nopnds was actually zero in some cases.
> I realized I could simplify the code a lot by placing the
> target rtx into the op[] array along with the source operands
> and at that point, the "case 0:" statements became dead code
> as you discovered.  I'll bootstrap and regtest the change
> below and commit it as an obvious fix when that is done.
> 
> Thanks for spotting this.

No problem.  Thanks for the explanation.
BTW please don't forget to add documentation of the builtins.
Being a non-PPC person, I had to dig and hack in the backend to get the
HTM builtins to work for the test.  I think the binutils version that I
used (2.23.1) is too old or something.  Even though I specified the
"-mhtm" option it would complain about that I need to specify the
"-mhtm" option to use the builtins...

Cheers,
Oleg


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