Hello, int f(double a,double b){ if(a>b) if(a<b) return 1; return 0; } gets simplified by tree-ssa-ifcombine to just return 0; which is wrong if the comparison would trap. This is caused by combine_comparisons: bool trap = (compcode & COMPCODE_UNORD) == 0 && (compcode != COMPCODE_EQ) && (compcode != COMPCODE_ORD); which is missing: && (compcode != COMPCODE_FALSE) (this isn't needed for ltrap and rtrap)
We do not try to preserve traps instead we only try to not produce new ones.
(In reply to comment #1) > We do not try to preserve traps instead we only try to not produce new ones. That would make a lot of sense, and assuming it is the official policy I am happy to learn that, but then why don't we optimize this testcase: int f(double a,double b){ if(a>=b) if(a<=b) return 1; return 0; } to if(a==b) return 1; ? The test in combine_comparisons seems to deliberately check for any change of the trapping condition, not just false->true. I am happy to relabel this bug (or file a new one if you prefer) as a missed optimization.
(In reply to comment #2) > (In reply to comment #1) > > We do not try to preserve traps instead we only try to not produce new ones. > > That would make a lot of sense, and assuming it is the official policy I am > happy to learn that, but then why don't we optimize this testcase: > > int f(double a,double b){ > if(a>=b) if(a<=b) return 1; > return 0; > } > > to > if(a==b) return 1; > ? Good question. I suppose that's a missed optimization then. > The test in combine_comparisons seems to deliberately check for any change of > the trapping condition, not just false->true. > > I am happy to relabel this bug (or file a new one if you prefer) as a missed > optimization. I'd say open a new one.
On Fri, 29 Jun 2012, rguenth at gcc dot gnu.org wrote: > We do not try to preserve traps instead we only try to not produce new ones. That's a bug. -ftrapping-math should do both (as regards the set of exceptions raised between function calls or asms that might test them, not necessarily preserving the exact positive number of times an exception is raised). If that's thought to be a performance issue maybe it should be split into separate options for the two cases.
(In reply to comment #4) > On Fri, 29 Jun 2012, rguenth at gcc dot gnu.org wrote: > > > We do not try to preserve traps instead we only try to not produce new ones. > > That's a bug. -ftrapping-math should do both (as regards the set of > exceptions raised between function calls or asms that might test them, not > necessarily preserving the exact positive number of times an exception is > raised). If that's thought to be a performance issue maybe it should be > split into separate options for the two cases. We happily remove dead trapping statements: void foo(double x, int y) { 1 / x; 1 / y; } Eric recently (for the sake of Java) added fdelete-dead-exceptions (in Ada, with -fnon-call-exceptions, you are allowed to remove the above statements). We similarly happily remove dead indirect loads (that might trap). I think it is preferable that we do all this, we can't always reasonably prove that a path in the CFG is never reached.
On Fri, 29 Jun 2012, rguenth at gcc dot gnu.org wrote: > We happily remove dead trapping statements: > > void foo(double x, int y) > { > 1 / x; Bug with -ftrapping-math. Maybe a known bug - there are plenty of open bugs for ways in which -ftrapping-math -frounding-math fail to implement standard C semantics - but a bug. > 1 / y; Arguably a bug with -ftrapv (again, the option could reasonably be split into different cases), but not otherwise.
(In reply to comment #6) > On Fri, 29 Jun 2012, rguenth at gcc dot gnu.org wrote: > > > We happily remove dead trapping statements: > > > > void foo(double x, int y) > > { > > 1 / x; > > Bug with -ftrapping-math. Maybe a known bug - there are plenty of open > bugs for ways in which -ftrapping-math -frounding-math fail to implement > standard C semantics - but a bug. > > > 1 / y; > > Arguably a bug with -ftrapv (again, the option could reasonably be split > into different cases), but not otherwise. All CPUs I know trap for 1 / 0, even in the integer division case. -ftrapv is a completely different story - for it we'd even have to preserve void foo (int x, int y) { x + y; }
Integer 1 / 0 is undefined behavior, so it is fine to optimize that away. But floating point traps with -ftrapping-math aren't undefined behavior.
Suspicious code: combine_comparisons has special code that depends on which comparison comes first, but maybe_fold_and_comparisons calls and_comparisons_1 (and thus combine_comparisons) with arguments in both orders. I guess this would give a wrong optimization for ORDERED_EXPR && LT_EXPR, but I am having a hard time creating an ORDERED_EXPR. It looks like invert_tree_comparison shouldn't return ERROR_MARK for (UN)ORDERED_EXPR.
Author: glisse Date: Thu Aug 2 19:54:43 2012 New Revision: 190100 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=190100 Log: 2012-08-02 Marc Glisse <marc.glisse@inria.fr> PR tree-optimization/53805 * gcc/fold-const.c (invert_tree_comparison): Invert ORDERED_EXPR and UNORDERED_EXPR even for trapping floating point. * gcc/testsuite/gcc.dg/fold-notunord.c: New testcase. Added: trunk/gcc/testsuite/gcc.dg/fold-notunord.c (with props) Modified: trunk/gcc/ChangeLog trunk/gcc/fold-const.c trunk/gcc/testsuite/ChangeLog Propchange: trunk/gcc/testsuite/gcc.dg/fold-notunord.c ('svn:eol-style' added) Propchange: trunk/gcc/testsuite/gcc.dg/fold-notunord.c ('svn:keywords' added)
(In reply to Richard Biener from comment #3) > > I am happy to relabel this bug (or file a new one if you prefer) as a missed > > optimization. > > I'd say open a new one. For reference, that's PR 53806.