On Thu, 26 Nov 2015, Jakub Jelinek wrote: > 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.
Btw, I tried to remove that fold () at some point but it spectacularly regressed (though before the C++ early folding work) in constexpr cases. > 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). I'm currently testing the genmatch.c patch ... (which might make this situation even worse). I don't think we have the issue on the 5 branch so much (because of way less patterns in match.pd). Richard.