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.
So you actually ran into a testcase that expected the warning?
Several of them if I remember correctly...
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.
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?
--
Marc Glisse
