Hello, Any comments? (patch is here: http://gcc.gnu.org/ml/gcc-patches/2013-07/msg01315.html)
Cheers, Oleg On Sat, 2013-07-27 at 14:52 +0200, Oleg Endo wrote: > Hi, > > On Fri, 2013-07-26 at 08:51 +0200, Uros Bizjak wrote: > > > BTW: I am not c++ expert, but doesn't c++ offer some sort of > > abstraction to get rid of > > > > + union { > > + rtx (*argc0) (void); > > + rtx (*argc1) (rtx); > > + rtx (*argc2) (rtx, rtx); > > + rtx (*argc3) (rtx, rtx, rtx); > > + rtx (*argc4) (rtx, rtx, rtx, rtx); > > + rtx (*argc5) (rtx, rtx, rtx, rtx, rtx); > > + rtx (*argc6) (rtx, rtx, rtx, rtx, rtx, rtx); > > + rtx (*argc7) (rtx, rtx, rtx, rtx, rtx, rtx, rtx); > > + rtx (*argc8) (rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx); > > + rtx (*argc9) (rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx); > > + rtx (*argc10) (rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx); > > + rtx (*argc11) (rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx, rtx); > > + } genfun; > > > > Variadic templates maybe, but we can't use them since that requires C > ++11. > > Anyway, I think it's better to turn the insn_gen_fn typedef in recog.h > into a functor. The change is quite transparent to the users, as the > attached patch shows. There really is no need for things like GEN_FCN?, > nor do we need to fixup all the backends etc. > > I've tested the attached patch on my SH cross setup with 'make all-gcc' > and it can still produce a valid 'hello world'. Not sure whether it's > sufficient. > > Some notes regarding the patch: > > * The whole arg list thing could probably be folded with x-macros or > something like that, but I don't think it's worth doing it for this > single case. If we can use C++11 in GCC at some point in time in the > future, the insn_gen_fn implementation can be folded with variadic > templates without touching all the code that uses it. > > * I had to extend the number of max. args to 16, otherwise the SH > backend's sync.md code wouldn't compile. > > * I don't know whether it's really needed to properly format the code of > class insn_gen_fn. After reading the first two or three overloads > (which do fit into 80 columns) one gets the idea and so I guess nobody > is going to read that stuff completely anyway. > > * The class insn_gen_fn is a POD, so it can be passed by value without > any overhead, just like a normal function pointer. Same goes for the > function call through the wrapper class. > > * Initially I had overloaded constructors in class insn_gen_fn which > worked and didn't require the cast in genoutput.c. However, it > introduced static initializer functions and defeated the purpose of the > generated const struct insn_data_d insn_data[]. This is worked around > by casting the function pointer to insn_gen_fn::stored_funcptr. (Since > it's C++ and not C it won't implicitly cast to void*). > > * The whole thing will fall apart if the stored pointer to a function > 'rtx (*)(void)' is different from a stored pointer to e.g. 'rtx > (*)(rtx)'. But I guess this is not likely to happen. > > Cheers, > Oleg > > gcc/ChangeLog: > * recog.h (rtx (*insn_gen_fn) (rtx, ...)): Replace typedef with > new class insn_gen_fn. > * expr.c (move_by_pieces_1, store_by_pieces_2): Replace > argument rtx (*) (rtx, ...) with insn_gen_fn. > * genoutput.c (output_insn_data): Cast gen_? function pointers > to insn_gen_fn::stored_funcptr. Add initializer braces.