| Message ID | YkvH/Pv559ax5GFE@tucnak |
|---|---|
| State | New |
| Headers | show |
| Series | match.pd: Punt on optimizing sqrt with incorrect arg type [PR105150] | expand |
On Tue, 5 Apr 2022, Jakub Jelinek wrote: > Hi! > > In the following testcase sqrt is declared as unprototyped function > and so it depends on what type has the argument passed to it. > If an argument of incorrect type is passed, the sqrt comparison > simplification can create invalid GIMPLE. > > The patch fixes it by punting if there is a mismatch of types. > As I didn't want to reindent 133 lines, the first hunk contains an > ugly hack with if (false). If you prefer reindentation, I can do that > too. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? So why does this get pas gimple_call_combined_fn ()? Are those internal functions at the point we match them? Otherwise we invoke gimple_builtin_call_types_compatible_p on them. If they are internal functions why did we rewrite the calls to them when the types do not match? If gimple_builtin_call_types_compatible_p is fine then why did the frontend assign BUILT_IN_SQRT to the function decls? Richard. > 2022-04-05 Jakub Jelinek <jakub@redhat.com> > > PR tree-optimization/105150 > * match.pd (sqrt (x) cmp real_cst, sqrt (x) cmp sqrt (y)): Punt > if sqrt operand has incompatible types. > > * gcc.dg/pr105150.c: New test. > > --- gcc/match.pd.jj 2022-03-18 18:32:36.000000000 +0100 > +++ gcc/match.pd 2022-04-04 19:49:28.621934784 +0200 > @@ -4927,6 +4927,10 @@ (define_operator_list SYNC_FETCH_AND_AND > (simplify > (cmp (sq @0) REAL_CST@1) > (switch > + (if (!types_match (TREE_TYPE (@0), TREE_TYPE (@1))) > + /* Punt if there is a type mismatch. */ > + (if (false) > + @1)) > (if (REAL_VALUE_NEGATIVE (TREE_REAL_CST (@1))) > (switch > /* sqrt(x) < y is always false, if y is negative. */ > @@ -5062,7 +5066,7 @@ (define_operator_list SYNC_FETCH_AND_AND > /* Transform sqrt(x) cmp sqrt(y) -> x cmp y. */ > (simplify > (cmp (sq @0) (sq @1)) > - (if (! HONOR_NANS (@0)) > + (if (! HONOR_NANS (@0) && types_match (TREE_TYPE (@0), TREE_TYPE (@1))) > (cmp @0 @1)))))) > > /* Optimize various special cases of (FTYPE) N CMP (FTYPE) M. */ > --- gcc/testsuite/gcc.dg/pr105150.c.jj 2022-04-04 19:53:03.597935060 +0200 > +++ gcc/testsuite/gcc.dg/pr105150.c 2022-04-04 19:54:55.734370333 +0200 > @@ -0,0 +1,8 @@ > +/* PR tree-optimization/105150 */ > +/* { dg-options "-w -Ofast" } */ > + > +#define A(name) __typeof (__builtin_##name (0)) name (); \ > + float name##1 () { return !name (1); } \ > + double name##2 () { return name (1.0L); } > +#define B(name) A(name) A(name##l) > +B (sqrt) > > Jakub > >
On Tue, Apr 05, 2022 at 10:50:01AM +0200, Richard Biener wrote: > > In the following testcase sqrt is declared as unprototyped function > > and so it depends on what type has the argument passed to it. > > If an argument of incorrect type is passed, the sqrt comparison > > simplification can create invalid GIMPLE. > > > > The patch fixes it by punting if there is a mismatch of types. > > As I didn't want to reindent 133 lines, the first hunk contains an > > ugly hack with if (false). If you prefer reindentation, I can do that > > too. > > > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > So why does this get pas gimple_call_combined_fn ()? Are those > internal functions at the point we match them? Otherwise we > invoke gimple_builtin_call_types_compatible_p on them. If > they are internal functions why did we rewrite the calls to them > when the types do not match? If gimple_builtin_call_types_compatible_p > is fine then why did the frontend assign BUILT_IN_SQRT to the > function decls? For the unprototyped functions like double sqrt (); they can be called in a valid way or they might not, so I think having DECL_BUILT_IN_CLASS other than BUILT_IN_NORMAL is right. The problem is that we don't do proper checking. In generic-match.cc, we just call get_call_combined_fn which performs no checking of the arguments whatsoever. In gimple-match.cc, we call gimple_call_combined_fn, which performs bad checking of the arguments. When not an internal function (which doesn't need argument checking, the compiler guarantees it is right), the former does just: tree fndecl = get_callee_fndecl (call); if (fndecl && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL)) return as_combined_fn (DECL_FUNCTION_CODE (fndecl)); In GIMPLE, we call: && gimple_builtin_call_types_compatible_p (stmt, fndecl) but that is insufficient, that verifies whether the arguments passed to GIMPLE_CALL match the fndecl argument types. But that fndecl may very well be the user declaration, so doesn't have to match exactly the builtin as predeclared by builtins.def etc. (though, there is the cotcha that say for FILE * we just use void * etc.). So e.g. in tree-ssa-strlen.cc we use additional: tree callee = gimple_call_fndecl (stmt); tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (callee)); if (decl && decl != callee && !gimple_builtin_call_types_compatible_p (stmt, decl)) return false; For this particular testcase, seems the problem is just the generic folding, if I add && GIMPLE to the sqrt comparison foldings, it doesn't ICE anymore, so just adding generic_builtin_call_types_compatible_p (basically copy gimple_builtin_call_types_compatible_p) might be enough. Though a big question is what it should do, using useless_type_conversion_p might be needed so that it can deal e.g. with the FILE * vs. void * differences (and various others), on the other side, what e.g. the exact sqrt comparison folding requires to be much more strict, as it turns (cmp (sq @0) REAL_CST@1) to (cmp @0 @1) and similarly for (cmp (sq @0) (sq @1)) and that requires exact type match, no? So one possibility would be to use useless_type_conversion_p but actually not write (cmp @0 @1) in such cases, but (cmp (convert:op1type @0) @1) or so? Jakub
On Tue, 5 Apr 2022, Jakub Jelinek wrote: > On Tue, Apr 05, 2022 at 10:50:01AM +0200, Richard Biener wrote: > > > In the following testcase sqrt is declared as unprototyped function > > > and so it depends on what type has the argument passed to it. > > > If an argument of incorrect type is passed, the sqrt comparison > > > simplification can create invalid GIMPLE. > > > > > > The patch fixes it by punting if there is a mismatch of types. > > > As I didn't want to reindent 133 lines, the first hunk contains an > > > ugly hack with if (false). If you prefer reindentation, I can do that > > > too. > > > > > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > > > So why does this get pas gimple_call_combined_fn ()? Are those > > internal functions at the point we match them? Otherwise we > > invoke gimple_builtin_call_types_compatible_p on them. If > > they are internal functions why did we rewrite the calls to them > > when the types do not match? If gimple_builtin_call_types_compatible_p > > is fine then why did the frontend assign BUILT_IN_SQRT to the > > function decls? > > For the unprototyped functions like double sqrt (); they can be called in a > valid way or they might not, so I think having DECL_BUILT_IN_CLASS other > than BUILT_IN_NORMAL is right. > > The problem is that we don't do proper checking. > > In generic-match.cc, we just call get_call_combined_fn which performs no > checking of the arguments whatsoever. > > In gimple-match.cc, we call gimple_call_combined_fn, which performs bad > checking of the arguments. > > When not an internal function (which doesn't need argument checking, the > compiler guarantees it is right), the former does just: > tree fndecl = get_callee_fndecl (call); > if (fndecl && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL)) > return as_combined_fn (DECL_FUNCTION_CODE (fndecl)); > In GIMPLE, we call: > && gimple_builtin_call_types_compatible_p (stmt, fndecl) > but that is insufficient, that verifies whether the arguments passed to > GIMPLE_CALL match the fndecl argument types. But that fndecl may very well > be the user declaration, so doesn't have to match exactly the builtin > as predeclared by builtins.def etc. (though, there is the cotcha that say > for FILE * we just use void * etc.). So e.g. in tree-ssa-strlen.cc > we use additional: > tree callee = gimple_call_fndecl (stmt); > tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (callee)); > if (decl > && decl != callee > && !gimple_builtin_call_types_compatible_p (stmt, decl)) > return false; Yeah, I think we should use that (and only that) function decl in get_call_combined_fn and gimple_call_combined_fn until the frontend stops slapping wrong BUILT_IN_* on random decls. > For this particular testcase, seems the problem is just the generic folding, > if I add && GIMPLE to the sqrt comparison foldings, it doesn't ICE anymore, > so just adding generic_builtin_call_types_compatible_p (basically copy > gimple_builtin_call_types_compatible_p) might be enough. > Though a big question is what it should do, using useless_type_conversion_p > might be needed so that it can deal e.g. with the FILE * vs. void * > differences (and various others), on the other side, what e.g. the exact > sqrt comparison folding requires to be much more strict, as it turns > (cmp (sq @0) REAL_CST@1) > to > (cmp @0 @1) > and similarly for > (cmp (sq @0) (sq @1)) > and that requires exact type match, no? Yes, on GENERIC it requires types_match (), so get_call_combined_fn should use a type checking variant that checks that instead of useless_type_conversion_p. > So one possibility would be to use useless_type_conversion_p but actually > not write (cmp @0 @1) in such cases, but (cmp (convert:op1type @0) @1) > or so? No, I think the match.pd patterns are all fine and the fix has to be elsewhere. Either in the generic get_call_combined_fn or in extra plumbing emitted by genmatch (I prefer to fix get_call_combined_fn and friends since we already do some checking for GIMPLE). Well, of course I prefer to have the frontend fixed, but ... Richard.
On Tue, Apr 05, 2022 at 11:28:53AM +0200, Richard Biener wrote: > > In GIMPLE, we call: > > && gimple_builtin_call_types_compatible_p (stmt, fndecl) > > but that is insufficient, that verifies whether the arguments passed to > > GIMPLE_CALL match the fndecl argument types. But that fndecl may very well > > be the user declaration, so doesn't have to match exactly the builtin > > as predeclared by builtins.def etc. (though, there is the cotcha that say > > for FILE * we just use void * etc.). So e.g. in tree-ssa-strlen.cc > > we use additional: > > tree callee = gimple_call_fndecl (stmt); > > tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (callee)); > > if (decl > > && decl != callee > > && !gimple_builtin_call_types_compatible_p (stmt, decl)) > > return false; > > Yeah, I think we should use that (and only that) function decl > in get_call_combined_fn and gimple_call_combined_fn until the > frontend stops slapping wrong BUILT_IN_* on random decls. So, as a preparation step, this patch adjusts gimple_call_builtin_p and gimple_call_combined_fn so that they check argument types against the builtin_decl_explicit TYPE_ARG_TYPES rather than against the actual used fndecl. Bootstrapped/regtested on powerpc64le-linux, ok for trunk? Will work on generic tree_builtin_call_types_compatible_p next. Jakub
On Tue, 5 Apr 2022, Jakub Jelinek wrote: > On Tue, Apr 05, 2022 at 11:28:53AM +0200, Richard Biener wrote: > > > In GIMPLE, we call: > > > && gimple_builtin_call_types_compatible_p (stmt, fndecl) > > > but that is insufficient, that verifies whether the arguments passed to > > > GIMPLE_CALL match the fndecl argument types. But that fndecl may very well > > > be the user declaration, so doesn't have to match exactly the builtin > > > as predeclared by builtins.def etc. (though, there is the cotcha that say > > > for FILE * we just use void * etc.). So e.g. in tree-ssa-strlen.cc > > > we use additional: > > > tree callee = gimple_call_fndecl (stmt); > > > tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (callee)); > > > if (decl > > > && decl != callee > > > && !gimple_builtin_call_types_compatible_p (stmt, decl)) > > > return false; > > > > Yeah, I think we should use that (and only that) function decl > > in get_call_combined_fn and gimple_call_combined_fn until the > > frontend stops slapping wrong BUILT_IN_* on random decls. > > So, as a preparation step, this patch adjusts gimple_call_builtin_p > and gimple_call_combined_fn so that they check argument types against > the builtin_decl_explicit TYPE_ARG_TYPES rather than against the > actual used fndecl. > > Bootstrapped/regtested on powerpc64le-linux, ok for trunk? You forgot to attach the patch ... Richard. > Will work on generic tree_builtin_call_types_compatible_p next. > > Jakub
On Wed, Apr 06, 2022 at 08:13:24AM +0200, Richard Biener wrote: > On Tue, 5 Apr 2022, Jakub Jelinek wrote: > > > On Tue, Apr 05, 2022 at 11:28:53AM +0200, Richard Biener wrote: > > > > In GIMPLE, we call: > > > > && gimple_builtin_call_types_compatible_p (stmt, fndecl) > > > > but that is insufficient, that verifies whether the arguments passed to > > > > GIMPLE_CALL match the fndecl argument types. But that fndecl may very well > > > > be the user declaration, so doesn't have to match exactly the builtin > > > > as predeclared by builtins.def etc. (though, there is the cotcha that say > > > > for FILE * we just use void * etc.). So e.g. in tree-ssa-strlen.cc > > > > we use additional: > > > > tree callee = gimple_call_fndecl (stmt); > > > > tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (callee)); > > > > if (decl > > > > && decl != callee > > > > && !gimple_builtin_call_types_compatible_p (stmt, decl)) > > > > return false; > > > > > > Yeah, I think we should use that (and only that) function decl > > > in get_call_combined_fn and gimple_call_combined_fn until the > > > frontend stops slapping wrong BUILT_IN_* on random decls. > > > > So, as a preparation step, this patch adjusts gimple_call_builtin_p > > and gimple_call_combined_fn so that they check argument types against > > the builtin_decl_explicit TYPE_ARG_TYPES rather than against the > > actual used fndecl. > > > > Bootstrapped/regtested on powerpc64le-linux, ok for trunk? > > You forgot to attach the patch ... Oops, here it is. Also bootstrapped/regtested on {x86_64,i686}-linux. 2022-04-06 Jakub Jelinek <jakub@redhat.com> PR tree-optimization/105150 * gimple.cc (gimple_call_builtin_p, gimple_call_combined_fn): For BUILT_IN_NORMAL calls, call gimple_builtin_call_types_compatible_p preferrably on builtin_decl_explicit decl rather than fndecl. * tree-ssa-strlen.cc (valid_builtin_call): Don't call gimple_builtin_call_types_compatible_p here. --- gcc/gimple.cc.jj 2022-02-09 15:15:59.436837973 +0100 +++ gcc/gimple.cc 2022-04-05 13:04:58.405586995 +0200 @@ -2841,7 +2841,12 @@ gimple_call_builtin_p (const gimple *stm if (is_gimple_call (stmt) && (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE && DECL_BUILT_IN_CLASS (fndecl) != NOT_BUILT_IN) - return gimple_builtin_call_types_compatible_p (stmt, fndecl); + { + if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL) + if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl))) + fndecl = decl; + return gimple_builtin_call_types_compatible_p (stmt, fndecl); + } return false; } @@ -2854,7 +2859,12 @@ gimple_call_builtin_p (const gimple *stm if (is_gimple_call (stmt) && (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE && DECL_BUILT_IN_CLASS (fndecl) == klass) - return gimple_builtin_call_types_compatible_p (stmt, fndecl); + { + if (klass == BUILT_IN_NORMAL) + if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl))) + fndecl = decl; + return gimple_builtin_call_types_compatible_p (stmt, fndecl); + } return false; } @@ -2867,7 +2877,11 @@ gimple_call_builtin_p (const gimple *stm if (is_gimple_call (stmt) && (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE && fndecl_built_in_p (fndecl, code)) - return gimple_builtin_call_types_compatible_p (stmt, fndecl); + { + if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl))) + fndecl = decl; + return gimple_builtin_call_types_compatible_p (stmt, fndecl); + } return false; } @@ -2884,10 +2898,14 @@ gimple_call_combined_fn (const gimple *s return as_combined_fn (gimple_call_internal_fn (call)); tree fndecl = gimple_call_fndecl (stmt); - if (fndecl - && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL) - && gimple_builtin_call_types_compatible_p (stmt, fndecl)) - return as_combined_fn (DECL_FUNCTION_CODE (fndecl)); + if (fndecl && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL)) + { + tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)); + if (!decl) + decl = fndecl; + if (gimple_builtin_call_types_compatible_p (stmt, decl)) + return as_combined_fn (DECL_FUNCTION_CODE (fndecl)); + } } return CFN_LAST; } --- gcc/tree-ssa-strlen.cc.jj 2022-03-30 09:11:53.310103911 +0200 +++ gcc/tree-ssa-strlen.cc 2022-04-05 12:22:56.023057416 +0200 @@ -1736,12 +1736,6 @@ valid_builtin_call (gimple *stmt) return false; tree callee = gimple_call_fndecl (stmt); - tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (callee)); - if (decl - && decl != callee - && !gimple_builtin_call_types_compatible_p (stmt, decl)) - return false; - switch (DECL_FUNCTION_CODE (callee)) { case BUILT_IN_MEMCMP: Jakub
On Wed, 6 Apr 2022, Jakub Jelinek wrote: > On Wed, Apr 06, 2022 at 08:13:24AM +0200, Richard Biener wrote: > > On Tue, 5 Apr 2022, Jakub Jelinek wrote: > > > > > On Tue, Apr 05, 2022 at 11:28:53AM +0200, Richard Biener wrote: > > > > > In GIMPLE, we call: > > > > > && gimple_builtin_call_types_compatible_p (stmt, fndecl) > > > > > but that is insufficient, that verifies whether the arguments passed to > > > > > GIMPLE_CALL match the fndecl argument types. But that fndecl may very well > > > > > be the user declaration, so doesn't have to match exactly the builtin > > > > > as predeclared by builtins.def etc. (though, there is the cotcha that say > > > > > for FILE * we just use void * etc.). So e.g. in tree-ssa-strlen.cc > > > > > we use additional: > > > > > tree callee = gimple_call_fndecl (stmt); > > > > > tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (callee)); > > > > > if (decl > > > > > && decl != callee > > > > > && !gimple_builtin_call_types_compatible_p (stmt, decl)) > > > > > return false; > > > > > > > > Yeah, I think we should use that (and only that) function decl > > > > in get_call_combined_fn and gimple_call_combined_fn until the > > > > frontend stops slapping wrong BUILT_IN_* on random decls. > > > > > > So, as a preparation step, this patch adjusts gimple_call_builtin_p > > > and gimple_call_combined_fn so that they check argument types against > > > the builtin_decl_explicit TYPE_ARG_TYPES rather than against the > > > actual used fndecl. > > > > > > Bootstrapped/regtested on powerpc64le-linux, ok for trunk? > > > > You forgot to attach the patch ... > > Oops, here it is. Also bootstrapped/regtested on {x86_64,i686}-linux. OK. Thanks, Richard. > 2022-04-06 Jakub Jelinek <jakub@redhat.com> > > PR tree-optimization/105150 > * gimple.cc (gimple_call_builtin_p, gimple_call_combined_fn): > For BUILT_IN_NORMAL calls, call gimple_builtin_call_types_compatible_p > preferrably on builtin_decl_explicit decl rather than fndecl. > * tree-ssa-strlen.cc (valid_builtin_call): Don't call > gimple_builtin_call_types_compatible_p here. > > --- gcc/gimple.cc.jj 2022-02-09 15:15:59.436837973 +0100 > +++ gcc/gimple.cc 2022-04-05 13:04:58.405586995 +0200 > @@ -2841,7 +2841,12 @@ gimple_call_builtin_p (const gimple *stm > if (is_gimple_call (stmt) > && (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE > && DECL_BUILT_IN_CLASS (fndecl) != NOT_BUILT_IN) > - return gimple_builtin_call_types_compatible_p (stmt, fndecl); > + { > + if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL) > + if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl))) > + fndecl = decl; > + return gimple_builtin_call_types_compatible_p (stmt, fndecl); > + } > return false; > } > > @@ -2854,7 +2859,12 @@ gimple_call_builtin_p (const gimple *stm > if (is_gimple_call (stmt) > && (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE > && DECL_BUILT_IN_CLASS (fndecl) == klass) > - return gimple_builtin_call_types_compatible_p (stmt, fndecl); > + { > + if (klass == BUILT_IN_NORMAL) > + if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl))) > + fndecl = decl; > + return gimple_builtin_call_types_compatible_p (stmt, fndecl); > + } > return false; > } > > @@ -2867,7 +2877,11 @@ gimple_call_builtin_p (const gimple *stm > if (is_gimple_call (stmt) > && (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE > && fndecl_built_in_p (fndecl, code)) > - return gimple_builtin_call_types_compatible_p (stmt, fndecl); > + { > + if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl))) > + fndecl = decl; > + return gimple_builtin_call_types_compatible_p (stmt, fndecl); > + } > return false; > } > > @@ -2884,10 +2898,14 @@ gimple_call_combined_fn (const gimple *s > return as_combined_fn (gimple_call_internal_fn (call)); > > tree fndecl = gimple_call_fndecl (stmt); > - if (fndecl > - && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL) > - && gimple_builtin_call_types_compatible_p (stmt, fndecl)) > - return as_combined_fn (DECL_FUNCTION_CODE (fndecl)); > + if (fndecl && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL)) > + { > + tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)); > + if (!decl) > + decl = fndecl; > + if (gimple_builtin_call_types_compatible_p (stmt, decl)) > + return as_combined_fn (DECL_FUNCTION_CODE (fndecl)); > + } > } > return CFN_LAST; > } > --- gcc/tree-ssa-strlen.cc.jj 2022-03-30 09:11:53.310103911 +0200 > +++ gcc/tree-ssa-strlen.cc 2022-04-05 12:22:56.023057416 +0200 > @@ -1736,12 +1736,6 @@ valid_builtin_call (gimple *stmt) > return false; > > tree callee = gimple_call_fndecl (stmt); > - tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (callee)); > - if (decl > - && decl != callee > - && !gimple_builtin_call_types_compatible_p (stmt, decl)) > - return false; > - > switch (DECL_FUNCTION_CODE (callee)) > { > case BUILT_IN_MEMCMP: > > > Jakub > >
Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes: > On Wed, 6 Apr 2022, Jakub Jelinek wrote: > >> On Wed, Apr 06, 2022 at 08:13:24AM +0200, Richard Biener wrote: >> > On Tue, 5 Apr 2022, Jakub Jelinek wrote: >> > >> > > On Tue, Apr 05, 2022 at 11:28:53AM +0200, Richard Biener wrote: >> > > > > In GIMPLE, we call: >> > > > > && gimple_builtin_call_types_compatible_p (stmt, fndecl) >> > > > > but that is insufficient, that verifies whether the arguments passed to >> > > > > GIMPLE_CALL match the fndecl argument types. But that fndecl may very well >> > > > > be the user declaration, so doesn't have to match exactly the builtin >> > > > > as predeclared by builtins.def etc. (though, there is the cotcha that say >> > > > > for FILE * we just use void * etc.). So e.g. in tree-ssa-strlen.cc >> > > > > we use additional: >> > > > > tree callee = gimple_call_fndecl (stmt); >> > > > > tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (callee)); >> > > > > if (decl >> > > > > && decl != callee >> > > > > && !gimple_builtin_call_types_compatible_p (stmt, decl)) >> > > > > return false; >> > > > >> > > > Yeah, I think we should use that (and only that) function decl >> > > > in get_call_combined_fn and gimple_call_combined_fn until the >> > > > frontend stops slapping wrong BUILT_IN_* on random decls. >> > > >> > > So, as a preparation step, this patch adjusts gimple_call_builtin_p >> > > and gimple_call_combined_fn so that they check argument types against >> > > the builtin_decl_explicit TYPE_ARG_TYPES rather than against the >> > > actual used fndecl. >> > > >> > > Bootstrapped/regtested on powerpc64le-linux, ok for trunk? >> > >> > You forgot to attach the patch ... >> >> Oops, here it is. Also bootstrapped/regtested on {x86_64,i686}-linux. > > OK. But it seems like the magic incantation to detect “real” built-in function calls is getting longer and longer. Can we not abstract this in a single place rather than have to repeat the same long sequence in multiple places? Thanks, Richard > > Thanks, > Richard. > >> 2022-04-06 Jakub Jelinek <jakub@redhat.com> >> >> PR tree-optimization/105150 >> * gimple.cc (gimple_call_builtin_p, gimple_call_combined_fn): >> For BUILT_IN_NORMAL calls, call gimple_builtin_call_types_compatible_p >> preferrably on builtin_decl_explicit decl rather than fndecl. >> * tree-ssa-strlen.cc (valid_builtin_call): Don't call >> gimple_builtin_call_types_compatible_p here. >> >> --- gcc/gimple.cc.jj 2022-02-09 15:15:59.436837973 +0100 >> +++ gcc/gimple.cc 2022-04-05 13:04:58.405586995 +0200 >> @@ -2841,7 +2841,12 @@ gimple_call_builtin_p (const gimple *stm >> if (is_gimple_call (stmt) >> && (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE >> && DECL_BUILT_IN_CLASS (fndecl) != NOT_BUILT_IN) >> - return gimple_builtin_call_types_compatible_p (stmt, fndecl); >> + { >> + if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL) >> + if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl))) >> + fndecl = decl; >> + return gimple_builtin_call_types_compatible_p (stmt, fndecl); >> + } >> return false; >> } >> >> @@ -2854,7 +2859,12 @@ gimple_call_builtin_p (const gimple *stm >> if (is_gimple_call (stmt) >> && (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE >> && DECL_BUILT_IN_CLASS (fndecl) == klass) >> - return gimple_builtin_call_types_compatible_p (stmt, fndecl); >> + { >> + if (klass == BUILT_IN_NORMAL) >> + if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl))) >> + fndecl = decl; >> + return gimple_builtin_call_types_compatible_p (stmt, fndecl); >> + } >> return false; >> } >> >> @@ -2867,7 +2877,11 @@ gimple_call_builtin_p (const gimple *stm >> if (is_gimple_call (stmt) >> && (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE >> && fndecl_built_in_p (fndecl, code)) >> - return gimple_builtin_call_types_compatible_p (stmt, fndecl); >> + { >> + if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl))) >> + fndecl = decl; >> + return gimple_builtin_call_types_compatible_p (stmt, fndecl); >> + } >> return false; >> } >> >> @@ -2884,10 +2898,14 @@ gimple_call_combined_fn (const gimple *s >> return as_combined_fn (gimple_call_internal_fn (call)); >> >> tree fndecl = gimple_call_fndecl (stmt); >> - if (fndecl >> - && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL) >> - && gimple_builtin_call_types_compatible_p (stmt, fndecl)) >> - return as_combined_fn (DECL_FUNCTION_CODE (fndecl)); >> + if (fndecl && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL)) >> + { >> + tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)); >> + if (!decl) >> + decl = fndecl; >> + if (gimple_builtin_call_types_compatible_p (stmt, decl)) >> + return as_combined_fn (DECL_FUNCTION_CODE (fndecl)); >> + } >> } >> return CFN_LAST; >> } >> --- gcc/tree-ssa-strlen.cc.jj 2022-03-30 09:11:53.310103911 +0200 >> +++ gcc/tree-ssa-strlen.cc 2022-04-05 12:22:56.023057416 +0200 >> @@ -1736,12 +1736,6 @@ valid_builtin_call (gimple *stmt) >> return false; >> >> tree callee = gimple_call_fndecl (stmt); >> - tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (callee)); >> - if (decl >> - && decl != callee >> - && !gimple_builtin_call_types_compatible_p (stmt, decl)) >> - return false; >> - >> switch (DECL_FUNCTION_CODE (callee)) >> { >> case BUILT_IN_MEMCMP: >> >> >> Jakub >> >>
On Wed, Apr 06, 2022 at 09:41:44AM +0100, Richard Sandiford wrote: > But it seems like the magic incantation to detect “real” built-in > function calls is getting longer and longer. Can we not abstract this > in a single place rather than have to repeat the same long sequence in > multiple places? I've already committed it, so it can be only dealt with an incremental patch. One possibility is to do it inside of gimple_builtin_call_types_compatible_p, after the assert do that: if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL) if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl))) fndecl = decl; but we then lose the theoretical possibility of comparing against the actual user declaration. Though I guess in the gimple-fold.cc gimple-low.cc gimple-match-head.cc calls to that function we also want this rather than what they do currently. Jakub
On Wed, 6 Apr 2022, Jakub Jelinek wrote: > On Wed, Apr 06, 2022 at 09:41:44AM +0100, Richard Sandiford wrote: > > But it seems like the magic incantation to detect “real” built-in > > function calls is getting longer and longer. Can we not abstract this > > in a single place rather than have to repeat the same long sequence in > > multiple places? > > I've already committed it, so it can be only dealt with an incremental > patch. > One possibility is to do it inside of > gimple_builtin_call_types_compatible_p, after the assert do that: > if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL) > if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl))) > fndecl = decl; > but we then lose the theoretical possibility of comparing against the > actual user declaration. Though I guess in the > gimple-fold.cc > gimple-low.cc > gimple-match-head.cc > calls to that function we also want this rather than what they do currently. Yes, I think it would be clearer to pass a BUILT_IN_* code to gimple_builtin_call_types_compatible_p and no decl and simply return false if we cannot get out hands at the "proper" decl from builtin_decl_explicit ... Richard.
On Wed, Apr 06, 2022 at 11:52:23AM +0200, Richard Biener wrote: > On Wed, 6 Apr 2022, Jakub Jelinek wrote: > > > On Wed, Apr 06, 2022 at 09:41:44AM +0100, Richard Sandiford wrote: > > > But it seems like the magic incantation to detect “real” built-in > > > function calls is getting longer and longer. Can we not abstract this > > > in a single place rather than have to repeat the same long sequence in > > > multiple places? > > > > I've already committed it, so it can be only dealt with an incremental > > patch. > > One possibility is to do it inside of > > gimple_builtin_call_types_compatible_p, after the assert do that: > > if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL) > > if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl))) > > fndecl = decl; > > but we then lose the theoretical possibility of comparing against the > > actual user declaration. Though I guess in the > > gimple-fold.cc > > gimple-low.cc > > gimple-match-head.cc > > calls to that function we also want this rather than what they do currently. > > Yes, I think it would be clearer to pass a BUILT_IN_* code to > gimple_builtin_call_types_compatible_p and no decl and simply return > false if we cannot get out hands at the "proper" decl from > builtin_decl_explicit ... That would mean we wouldn't verify the md or FE builtins anymore and we would need to check for BUILT_IN_NORMAL in every caller (right now we do that only in some of them). Here is what I had in mind (untested so far): 2022-04-06 Jakub Jelinek <jakub@redhat.com> PR tree-optimization/105150 * gimple.cc (gimple_builtin_call_types_compatible_p): Use builtin_decl_explicit here... (gimple_call_builtin_p, gimple_call_combined_fn): ... rather than here. --- gcc/gimple.cc.jj 2022-04-06 10:07:23.043064595 +0200 +++ gcc/gimple.cc 2022-04-06 11:31:31.704255242 +0200 @@ -2788,6 +2788,10 @@ gimple_builtin_call_types_compatible_p ( { gcc_checking_assert (DECL_BUILT_IN_CLASS (fndecl) != NOT_BUILT_IN); + if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL) + if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl))) + fndecl = decl; + tree ret = gimple_call_lhs (stmt); if (ret && !useless_type_conversion_p (TREE_TYPE (ret), @@ -2841,12 +2845,7 @@ gimple_call_builtin_p (const gimple *stm if (is_gimple_call (stmt) && (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE && DECL_BUILT_IN_CLASS (fndecl) != NOT_BUILT_IN) - { - if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL) - if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl))) - fndecl = decl; - return gimple_builtin_call_types_compatible_p (stmt, fndecl); - } + return gimple_builtin_call_types_compatible_p (stmt, fndecl); return false; } @@ -2859,12 +2858,7 @@ gimple_call_builtin_p (const gimple *stm if (is_gimple_call (stmt) && (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE && DECL_BUILT_IN_CLASS (fndecl) == klass) - { - if (klass == BUILT_IN_NORMAL) - if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl))) - fndecl = decl; - return gimple_builtin_call_types_compatible_p (stmt, fndecl); - } + return gimple_builtin_call_types_compatible_p (stmt, fndecl); return false; } @@ -2877,11 +2871,7 @@ gimple_call_builtin_p (const gimple *stm if (is_gimple_call (stmt) && (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE && fndecl_built_in_p (fndecl, code)) - { - if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl))) - fndecl = decl; - return gimple_builtin_call_types_compatible_p (stmt, fndecl); - } + return gimple_builtin_call_types_compatible_p (stmt, fndecl); return false; } @@ -2898,14 +2888,10 @@ gimple_call_combined_fn (const gimple *s return as_combined_fn (gimple_call_internal_fn (call)); tree fndecl = gimple_call_fndecl (stmt); - if (fndecl && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL)) - { - tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)); - if (!decl) - decl = fndecl; - if (gimple_builtin_call_types_compatible_p (stmt, decl)) - return as_combined_fn (DECL_FUNCTION_CODE (fndecl)); - } + if (fndecl + && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL) + && gimple_builtin_call_types_compatible_p (stmt, fndecl)) + return as_combined_fn (DECL_FUNCTION_CODE (fndecl)); } return CFN_LAST; } Jakub
On Wed, 6 Apr 2022, Jakub Jelinek wrote: > On Wed, Apr 06, 2022 at 11:52:23AM +0200, Richard Biener wrote: > > On Wed, 6 Apr 2022, Jakub Jelinek wrote: > > > > > On Wed, Apr 06, 2022 at 09:41:44AM +0100, Richard Sandiford wrote: > > > > But it seems like the magic incantation to detect “real” built-in > > > > function calls is getting longer and longer. Can we not abstract this > > > > in a single place rather than have to repeat the same long sequence in > > > > multiple places? > > > > > > I've already committed it, so it can be only dealt with an incremental > > > patch. > > > One possibility is to do it inside of > > > gimple_builtin_call_types_compatible_p, after the assert do that: > > > if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL) > > > if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl))) > > > fndecl = decl; > > > but we then lose the theoretical possibility of comparing against the > > > actual user declaration. Though I guess in the > > > gimple-fold.cc > > > gimple-low.cc > > > gimple-match-head.cc > > > calls to that function we also want this rather than what they do currently. > > > > Yes, I think it would be clearer to pass a BUILT_IN_* code to > > gimple_builtin_call_types_compatible_p and no decl and simply return > > false if we cannot get out hands at the "proper" decl from > > builtin_decl_explicit ... > > That would mean we wouldn't verify the md or FE builtins anymore > and we would need to check for BUILT_IN_NORMAL in every caller (right now > we do that only in some of them). > > Here is what I had in mind (untested so far): Yes, that works as well for me. Richard. > 2022-04-06 Jakub Jelinek <jakub@redhat.com> > > PR tree-optimization/105150 > * gimple.cc (gimple_builtin_call_types_compatible_p): Use > builtin_decl_explicit here... > (gimple_call_builtin_p, gimple_call_combined_fn): ... rather than > here. > > --- gcc/gimple.cc.jj 2022-04-06 10:07:23.043064595 +0200 > +++ gcc/gimple.cc 2022-04-06 11:31:31.704255242 +0200 > @@ -2788,6 +2788,10 @@ gimple_builtin_call_types_compatible_p ( > { > gcc_checking_assert (DECL_BUILT_IN_CLASS (fndecl) != NOT_BUILT_IN); > > + if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL) > + if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl))) > + fndecl = decl; > + > tree ret = gimple_call_lhs (stmt); > if (ret > && !useless_type_conversion_p (TREE_TYPE (ret), > @@ -2841,12 +2845,7 @@ gimple_call_builtin_p (const gimple *stm > if (is_gimple_call (stmt) > && (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE > && DECL_BUILT_IN_CLASS (fndecl) != NOT_BUILT_IN) > - { > - if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL) > - if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl))) > - fndecl = decl; > - return gimple_builtin_call_types_compatible_p (stmt, fndecl); > - } > + return gimple_builtin_call_types_compatible_p (stmt, fndecl); > return false; > } > > @@ -2859,12 +2858,7 @@ gimple_call_builtin_p (const gimple *stm > if (is_gimple_call (stmt) > && (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE > && DECL_BUILT_IN_CLASS (fndecl) == klass) > - { > - if (klass == BUILT_IN_NORMAL) > - if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl))) > - fndecl = decl; > - return gimple_builtin_call_types_compatible_p (stmt, fndecl); > - } > + return gimple_builtin_call_types_compatible_p (stmt, fndecl); > return false; > } > > @@ -2877,11 +2871,7 @@ gimple_call_builtin_p (const gimple *stm > if (is_gimple_call (stmt) > && (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE > && fndecl_built_in_p (fndecl, code)) > - { > - if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl))) > - fndecl = decl; > - return gimple_builtin_call_types_compatible_p (stmt, fndecl); > - } > + return gimple_builtin_call_types_compatible_p (stmt, fndecl); > return false; > } > > @@ -2898,14 +2888,10 @@ gimple_call_combined_fn (const gimple *s > return as_combined_fn (gimple_call_internal_fn (call)); > > tree fndecl = gimple_call_fndecl (stmt); > - if (fndecl && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL)) > - { > - tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)); > - if (!decl) > - decl = fndecl; > - if (gimple_builtin_call_types_compatible_p (stmt, decl)) > - return as_combined_fn (DECL_FUNCTION_CODE (fndecl)); > - } > + if (fndecl > + && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL) > + && gimple_builtin_call_types_compatible_p (stmt, fndecl)) > + return as_combined_fn (DECL_FUNCTION_CODE (fndecl)); > } > return CFN_LAST; > } > > > Jakub > >
Jakub Jelinek <jakub@redhat.com> writes: > On Wed, Apr 06, 2022 at 11:52:23AM +0200, Richard Biener wrote: >> On Wed, 6 Apr 2022, Jakub Jelinek wrote: >> >> > On Wed, Apr 06, 2022 at 09:41:44AM +0100, Richard Sandiford wrote: >> > > But it seems like the magic incantation to detect “real” built-in >> > > function calls is getting longer and longer. Can we not abstract this >> > > in a single place rather than have to repeat the same long sequence in >> > > multiple places? >> > >> > I've already committed it, so it can be only dealt with an incremental >> > patch. >> > One possibility is to do it inside of >> > gimple_builtin_call_types_compatible_p, after the assert do that: >> > if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL) >> > if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl))) >> > fndecl = decl; >> > but we then lose the theoretical possibility of comparing against the >> > actual user declaration. Though I guess in the >> > gimple-fold.cc >> > gimple-low.cc >> > gimple-match-head.cc >> > calls to that function we also want this rather than what they do currently. >> >> Yes, I think it would be clearer to pass a BUILT_IN_* code to >> gimple_builtin_call_types_compatible_p and no decl and simply return >> false if we cannot get out hands at the "proper" decl from >> builtin_decl_explicit ... > > That would mean we wouldn't verify the md or FE builtins anymore > and we would need to check for BUILT_IN_NORMAL in every caller (right now > we do that only in some of them). > > Here is what I had in mind (untested so far): > > 2022-04-06 Jakub Jelinek <jakub@redhat.com> > > PR tree-optimization/105150 > * gimple.cc (gimple_builtin_call_types_compatible_p): Use > builtin_decl_explicit here... > (gimple_call_builtin_p, gimple_call_combined_fn): ... rather than > here. Nice! Thanks for doing this. Richard > --- gcc/gimple.cc.jj 2022-04-06 10:07:23.043064595 +0200 > +++ gcc/gimple.cc 2022-04-06 11:31:31.704255242 +0200 > @@ -2788,6 +2788,10 @@ gimple_builtin_call_types_compatible_p ( > { > gcc_checking_assert (DECL_BUILT_IN_CLASS (fndecl) != NOT_BUILT_IN); > > + if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL) > + if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl))) > + fndecl = decl; > + > tree ret = gimple_call_lhs (stmt); > if (ret > && !useless_type_conversion_p (TREE_TYPE (ret), > @@ -2841,12 +2845,7 @@ gimple_call_builtin_p (const gimple *stm > if (is_gimple_call (stmt) > && (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE > && DECL_BUILT_IN_CLASS (fndecl) != NOT_BUILT_IN) > - { > - if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL) > - if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl))) > - fndecl = decl; > - return gimple_builtin_call_types_compatible_p (stmt, fndecl); > - } > + return gimple_builtin_call_types_compatible_p (stmt, fndecl); > return false; > } > > @@ -2859,12 +2858,7 @@ gimple_call_builtin_p (const gimple *stm > if (is_gimple_call (stmt) > && (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE > && DECL_BUILT_IN_CLASS (fndecl) == klass) > - { > - if (klass == BUILT_IN_NORMAL) > - if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl))) > - fndecl = decl; > - return gimple_builtin_call_types_compatible_p (stmt, fndecl); > - } > + return gimple_builtin_call_types_compatible_p (stmt, fndecl); > return false; > } > > @@ -2877,11 +2871,7 @@ gimple_call_builtin_p (const gimple *stm > if (is_gimple_call (stmt) > && (fndecl = gimple_call_fndecl (stmt)) != NULL_TREE > && fndecl_built_in_p (fndecl, code)) > - { > - if (tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl))) > - fndecl = decl; > - return gimple_builtin_call_types_compatible_p (stmt, fndecl); > - } > + return gimple_builtin_call_types_compatible_p (stmt, fndecl); > return false; > } > > @@ -2898,14 +2888,10 @@ gimple_call_combined_fn (const gimple *s > return as_combined_fn (gimple_call_internal_fn (call)); > > tree fndecl = gimple_call_fndecl (stmt); > - if (fndecl && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL)) > - { > - tree decl = builtin_decl_explicit (DECL_FUNCTION_CODE (fndecl)); > - if (!decl) > - decl = fndecl; > - if (gimple_builtin_call_types_compatible_p (stmt, decl)) > - return as_combined_fn (DECL_FUNCTION_CODE (fndecl)); > - } > + if (fndecl > + && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL) > + && gimple_builtin_call_types_compatible_p (stmt, fndecl)) > + return as_combined_fn (DECL_FUNCTION_CODE (fndecl)); > } > return CFN_LAST; > } > > > Jakub
--- gcc/match.pd.jj 2022-03-18 18:32:36.000000000 +0100 +++ gcc/match.pd 2022-04-04 19:49:28.621934784 +0200 @@ -4927,6 +4927,10 @@ (define_operator_list SYNC_FETCH_AND_AND (simplify (cmp (sq @0) REAL_CST@1) (switch + (if (!types_match (TREE_TYPE (@0), TREE_TYPE (@1))) + /* Punt if there is a type mismatch. */ + (if (false) + @1)) (if (REAL_VALUE_NEGATIVE (TREE_REAL_CST (@1))) (switch /* sqrt(x) < y is always false, if y is negative. */ @@ -5062,7 +5066,7 @@ (define_operator_list SYNC_FETCH_AND_AND /* Transform sqrt(x) cmp sqrt(y) -> x cmp y. */ (simplify (cmp (sq @0) (sq @1)) - (if (! HONOR_NANS (@0)) + (if (! HONOR_NANS (@0) && types_match (TREE_TYPE (@0), TREE_TYPE (@1))) (cmp @0 @1)))))) /* Optimize various special cases of (FTYPE) N CMP (FTYPE) M. */ --- gcc/testsuite/gcc.dg/pr105150.c.jj 2022-04-04 19:53:03.597935060 +0200 +++ gcc/testsuite/gcc.dg/pr105150.c 2022-04-04 19:54:55.734370333 +0200 @@ -0,0 +1,8 @@ +/* PR tree-optimization/105150 */ +/* { dg-options "-w -Ofast" } */ + +#define A(name) __typeof (__builtin_##name (0)) name (); \ + float name##1 () { return !name (1); } \ + double name##2 () { return name (1.0L); } +#define B(name) A(name) A(name##l) +B (sqrt)