| Message ID | 54E9CFF3.6070908@mentor.com |
|---|---|
| State | New |
| Headers | show |
On Sun, 22 Feb 2015, Tom de Vries wrote: > On 20-02-15 12:54, Richard Biener wrote: > > On Thu, 19 Feb 2015, Tom de Vries wrote: > > > > > On 19-02-15 14:07, Richard Biener wrote: > > > > On Thu, 19 Feb 2015, Jakub Jelinek wrote: > > > > > > > > > On Thu, Feb 19, 2015 at 01:44:46PM +0100, Richard Biener wrote: > > > > > > I'd call it a bug though, and we do have internal fns in > > > > > > generic already thus the issue is latent (with ubsan at least). > > > > > > > > > > > > Which means ok for trunk now. > > > > > > > > > > But the patch should better handle the internal calls right. > > > > > I.e. return 0 only if only one, not both CALL_EXPR_FNs are NULL, > > > > > or if both are NULL and CALL_EXPR_IFN is different, or if > > > > > call_expr_nargs is different. > > > > > > > > The question is whether generic call handling works (esp. > > > > call_expr_flags > > > > works correctly - the argument compare should work fine already). > > > > > > > > Tom - care to update the patch? > > > > > > > > > > I agree, the current patch is conservative and we can do betterns, > - > . > > > But I think it's wiser to do that as a stage1 follow-up, and commit this > > > conservative patch for stage4. Is that acceptable? > > > > Then just defer it for stage1 completely. If a problem pops up with > > GCC 5 we can backport the proper patch together with a testcase. > > > > Updated patch according to Jakub's comments, retested. > > OK for stage1? Ok. Thanks, Richard.
[ forwarding. for some reason, this email didn't make it to gcc-patches ml archive ] -------- Forwarded Message -------- Subject: Re: [PATCH][4/5] Handle internal_fn in operand_equal_p Date: Mon, 23 Feb 2015 10:03:34 +0100 From: Richard Biener <rguenther@suse.de> To: Tom de Vries <Tom_deVries@mentor.com> CC: Jakub Jelinek <jakub@redhat.com>, GCC Patches <gcc-patches@gcc.gnu.org>, Michael Matz <matz@suse.de> On Sun, 22 Feb 2015, Tom de Vries wrote: > On 20-02-15 12:54, Richard Biener wrote: > > On Thu, 19 Feb 2015, Tom de Vries wrote: > > > > > On 19-02-15 14:07, Richard Biener wrote: > > > > On Thu, 19 Feb 2015, Jakub Jelinek wrote: > > > > > > > > > On Thu, Feb 19, 2015 at 01:44:46PM +0100, Richard Biener wrote: > > > > > > I'd call it a bug though, and we do have internal fns in > > > > > > generic already thus the issue is latent (with ubsan at least). > > > > > > > > > > > > Which means ok for trunk now. > > > > > > > > > > But the patch should better handle the internal calls right. > > > > > I.e. return 0 only if only one, not both CALL_EXPR_FNs are NULL, > > > > > or if both are NULL and CALL_EXPR_IFN is different, or if > > > > > call_expr_nargs is different. > > > > > > > > The question is whether generic call handling works (esp. > > > > call_expr_flags > > > > works correctly - the argument compare should work fine already). > > > > > > > > Tom - care to update the patch? > > > > > > > > > > I agree, the current patch is conservative and we can do betterns, > - > . > > > But I think it's wiser to do that as a stage1 follow-up, and commit this > > > conservative patch for stage4. Is that acceptable? > > > > Then just defer it for stage1 completely. If a problem pops up with > > GCC 5 we can backport the proper patch together with a testcase. > > > > Updated patch according to Jakub's comments, retested. > > OK for stage1? Ok. Thanks, Richard.
2015-02-17 Tom de Vries <tom@codesourcery.com> * fold-const.c (operand_equal_p): Handle INTERNAL_FNs. * calls.c (call_expr_flags): Same. --- gcc/calls.c | 2 ++ gcc/fold-const.c | 23 +++++++++++++++++++---- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/gcc/calls.c b/gcc/calls.c index ec44624..2919464 100644 --- a/gcc/calls.c +++ b/gcc/calls.c @@ -847,6 +847,8 @@ call_expr_flags (const_tree t) if (decl) flags = flags_from_decl_or_type (decl); + else if (CALL_EXPR_FN (t) == NULL_TREE) + flags = internal_fn_flags (CALL_EXPR_IFN (t)); else { t = TREE_TYPE (CALL_EXPR_FN (t)); diff --git a/gcc/fold-const.c b/gcc/fold-const.c index 8377120..3013adb 100644 --- a/gcc/fold-const.c +++ b/gcc/fold-const.c @@ -3032,11 +3032,26 @@ operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags) switch (TREE_CODE (arg0)) { case CALL_EXPR: - /* If the CALL_EXPRs call different functions, then they - clearly can not be equal. */ - if (! operand_equal_p (CALL_EXPR_FN (arg0), CALL_EXPR_FN (arg1), - flags)) + if ((CALL_EXPR_FN (arg0) == NULL_TREE) + != (CALL_EXPR_FN (arg1) == NULL_TREE)) + /* If not both CALL_EXPRs are either internal or normal function + functions, then they are not equal. */ return 0; + else if (CALL_EXPR_FN (arg0) == NULL_TREE) + { + /* If the CALL_EXPRs call different internal functions, then they + are not equal. */ + if (CALL_EXPR_IFN (arg0) != CALL_EXPR_IFN (arg1)) + return 0; + } + else + { + /* If the CALL_EXPRs call different functions, then they are not + equal. */ + if (! operand_equal_p (CALL_EXPR_FN (arg0), CALL_EXPR_FN (arg1), + flags)) + return 0; + } { unsigned int cef = call_expr_flags (arg0); -- 1.9.1