On Thu, Nov 26, 2015 at 12:15:48PM +0100, Marek Polacek wrote: > On Wed, Nov 25, 2015 at 03:16:53PM +0000, Joseph Myers wrote: > > > Wonder if we couldn't use some FE specific bit on the SAVE_EXPR to say > > > whether c_fully_fold_internal has already processed it or not, and just > > > get rid of c_save_expr, in c_fully_fold* recurse into SAVE_EXPRs too, but > > > only if that bit is not yet already set, and set it afterwards. > > > > I suppose you could do that, in line with the general principle of > > reducing early folding (as long as you ensure that folding the contents of > > a SAVE_EXPR results in modifying that SAVE_EXPR so that all pointers to it > > stay pointing to the same tree node). > > I had a go at this, but I'm now skeptical about removing c_save_expr. > save_expr calls fold (), so we need to ensure that we don't pass any > C_MAYBE_CONST_EXPRs into it, meaning that we'd need to call c_fully_fold > before > save_expr anyway...
I bet that is too hard for stage3, but IMHO if we want delayed folding, we want to delay it even for SAVE_EXPRs. Supposedly the reason why save_expr calls fold is to determine the cases where there is no point to create the SAVE_EXPR. But perhaps it should just fold for the purpose of testing that and return the original expression if after folding it is simple arithmetics, and wrap the original expression into SAVE_EXPR. Though in this particular case, where save_expr is just an optimization it is perhaps premature optimization and we should not perform that. For stage3, I agree, some other fix is needed (and one usable also for the 5 branch). Jakub