> Should UNITS_PER_WORD here be POINTER_SIZE/BITS_PER_UNIT right? Hmm, yes, presumably, it's the size of the static chain and so a pointer.
> Another UNITS_PER_WORD that I think ought to be > POINTER_SIZE/BITS_PER_UNIT. Probably worth a pass over the patch to > look for this throughout. Yes, it was very likely enabled only on platforms with word-sized pointers. > > +ftrampolines > > +Common Report Var(flag_trampolines) Init(0) > > +Always generate trampolines for pointers to nested functions > > Even for targets that use procedure descriptors natively? I'm not sure > what's the best choice for them -- I could argue either way on this issue. For targets that use procedure descriptors natively, it's a no-op because the entire patch is supposed to be a no-op for them. I'm going to rephrase. > I'm probably not well versed enough in this aspect of the various > targets's ABIs to review the target stuff. We'll want/need the target > maintainers to chime in. That might be easier if the target bits were > split out into their own patches. Good idea, I'll do that. > "number of bytes", don't you mean "number of bits" in the last paragraph? Yes, that's confused, I'll rephrase too. > > Index: tree-nested.c > > =================================================================== > > --- tree-nested.c (revision 237789) > > +++ tree-nested.c (working copy) > > @@ -21,6 +21,7 @@ > > > > #include "system.h" > > #include "coretypes.h" > > #include "backend.h" > > > > +#include "target.h" > > Not real happy to see target stuff getting into tree-*. Is this really > needed? It's the target hook mentioned earlier, e.g. for targets that use procedure descriptors natively we don't want to create generic descriptors but use the regular "degenerated" trampolines. > I think the biggest issues are the ABI concerns and getting the target > maintainers to take a looksie. Everything else looks pretty good. Thanks for the review. -- Eric Botcazou
