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