On Tue, Apr 26, 2016 at 10:28 PM, Marc Glisse <marc.gli...@inria.fr> wrote: > On Tue, 26 Apr 2016, Richard Biener wrote: > >> On Sun, Apr 24, 2016 at 7:14 PM, Marc Glisse <marc.gli...@inria.fr> wrote: >>> >>> Hello, >>> >>> trying to move a first pattern from fold_comparison. >>> >>> I first tried without single_use. It brought the number of 'free' in >>> g++.dg/tree-ssa/pr61034.C down to 11, changed gcc.dg/sms-6.c to only 2 >>> SMS >>> (I don't think the generated code was worse, maybe even better, but I >>> don't >>> know ppc asm), broke Wstrict-overflow-18.c (the optimization moved from >>> VRP >>> to CCP if I remember correctly), and caused IVOPTS to make a mess in >>> guality/pr54693-2.c (much longer code, and many <optimized> debug >>> variables). If someone wants to drop the single_use, they can work on >>> that >>> after this patch is in. >>> >>> The conditions do not exactly match the ones in fold-const.c, but I guess >>> they are close. The warning in the constant case was missing in >>> fold_comparison, but present in VRP, so I had to add it not to regress. >>> >>> I don't think we were warning much from match.pd. I can't say that I am a >>> big fan of those strict overflow warnings, but I expect people would >>> complain if we just dropped the existing ones when moving the transforms >>> to >>> match.pd? >> >> >> I just dropped them for patterns I moved (luckily we didn't have >> testcases sofar ;)) >> >> If we really want to warn from match.pd then you should do the >> defer/undefer >> stuff in fold_stmt itself (same condition I guess) and defer/undefer >> without >> warning in gimple_fold_stmt_to_constant_1. > > > Moving it to fold_stmt_1 seems like a good idea, much better than putting it > in forwprop. Looking at gimple_fold_stmt_to_constant_1 on the other hand, I > have some concerns. If we do not warn for gimple_fold_stmt_to_constant_1, we > are going to miss some warnings (I believe there is at least one testcase > that will break, where VRP currently warns but CCP will come first). On the > other hand, gimple_fold_stmt_to_constant_1 doesn't do much validation on its > return value, each caller has their own idea of which results are > acceptable, so it is only in the (many) callers that we can defer/undefer, > or we might warn for transformations that we don't actually perform. CCP > already has the defer/undefer calls around ccp_fold and thus > gimple_fold_stmt_to_constant_1.
Yeah, the issue with gimple_fold_stmt_to_constant_1 is that it's usually used in an iteration scheme and thus we may warn multiple times and for transforms that don't end up being used... >> So you actually ran into a testcase that expected the warning? > > > Several of them if I remember correctly... Ugh. >>> I wanted to restrict the equality case to TYPE_OVERFLOW_WRAPS || >>> TYPE_OVERFLOW_UNDEFINED, but that broke 20041114-1.c at -O1 (no strict >>> overflow), so I went with some kind of complement we use elsewhere. >> >> >> I think I prefer to move things 1:1 (unless sth regresses) and fix bugs in >> the >> fold-const.c variant as followup (possibly also adding testcases). > > > Well, yes, but things do indeed regress quite regularly when moving things > 1:1 :-( At least unless we add a big (if (GENERIC) ...) around the > transformation. Sure, just wanted clarification on what changes are just (obvious) bugfixes and what are needed to not regress in the testsuite. >> IMHO -fno-strict-overflow needs to go. It has very wary designed >> semantics >> (ops neither wrap nor have undefined overflow). > > > Maybe make it an alias for -fwrapv? Yes, for example. Richard. > > -- > Marc Glisse
