> Here is an attempt to make the check more complete (e.g. > the change wouldn't see overlap if addr was PLUS of two REGs, > where one of the REGs was based on internal_arg_pointer, etc.) > and less pessimistic. As tree-tailcall.c doesn't allow tail calls > from functions that have address of any of the caller's parameters > taken, IMHO it is enough to look for internal_arg_pointer based > pseudos initialized in the tail call sequence. > This patch scans the tail call sequence and notes which pseudos > are based on internal_arg_pointer (and what offset from > that pointer they have) and uses that in > mem_overlaps_already_clobbered_arg_p.
This looks reasonable, but the logic is a bit hard to follow, especially the double usage of internal_arg_pointer_based_reg depending on SCAN's value. Would it be possible to split it into 2 functions that recursively call each other? You should also make it clearer that internal_arg_pointer_seq_start is part of the caching mechanism (I wondered for some minutes whether it has anything to do with RTL sequences), maybe: /* Last insn that has been scanned by internal_arg_pointer_based_reg, or NULL_RTX if none has been scanned yet. */ static rtx internal_arg_pointer_insn_cache; /* Vector indexed by REGNO - FIRST_PSEUDO_REGISTER, recording if a pseudo is based on crtl->args.internal_arg_pointer. The element is NULL_RTX if the pseudo isn't based on it, a CONST_INT offset if the pseudo is based on it with fixed offset, or PC if this is with variable or unknown offset. */ static VEC(rtx, heap) *internal_arg_pointer_pseudo_cache; > +/* Helper function for internal_arg_pointer_based_reg, called through > + for_each_rtx. Return 1 if a crtl->args.internal_arg_pointer based > + register is seen anywhere. */ > + > +static int > +internal_arg_pointer_based_reg_1 (rtx *loc, void *data ATTRIBUTE_UNUSED) > +{ > + if (REG_P (*loc) && internal_arg_pointer_based_reg (*loc, false) != > NULL_RTX) + return 1; > + if (MEM_P (*loc)) > + return -1; > + return 0; > +} Missing comment for the MEM_P case. > +/* If REG is based on crtl->args.internal_arg_pointer, return either > + a CONST_INT offset from crtl->args.internal_arg_pointer if > + offset from it is known constant, or PC if the offset is unknown. > + Return NULL_RTX if REG isn't based on crtl->args.internal_arg_pointer. > */ + > +static rtx > +internal_arg_pointer_based_reg (rtx reg, bool scan) > +{ > + rtx insn; > + > + if (CONSTANT_P (reg)) > + return NULL_RTX; > + > + if (reg == crtl->args.internal_arg_pointer) > + return const0_rtx; > + > + if (REG_P (reg) && REGNO (reg) < FIRST_PSEUDO_REGISTER) > + return NULL_RTX; if (REG_P (reg) && HARD_REGISTER_P (reg)) return NULL_RTX; -- Eric Botcazou