This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: PPC64 HTM support (was [buildbot] r201513: Invalid cast)
- From: Oleg Endo <oleg dot endo at t-online dot de>
- To: Peter Bergner <bergner at vnet dot ibm dot com>
- Cc: David Edelsohn <dje dot gcc at gmail dot com>, Jan-Benedict Glaw <jbglaw at lug-owl dot de>, GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Wed, 07 Aug 2013 21:32:28 +0200
- Subject: Re: PPC64 HTM support (was [buildbot] r201513: Invalid cast)
- References: <CAGWvny=8Ry_WsGuCrY5yZScVf5na0v4nO1vOk2zYjd21-TKFXw at mail dot gmail dot com> <1375844473 dot 5240 dot 111 dot camel at otta>
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