Message ID | 20131209222440.GL11710@redhat.com |
---|---|
State | New |
Headers | show |
On Mon, 9 Dec 2013, Marek Polacek wrote: > Back in April 2011, Richard S. submitted the implementation of > internal functions [1]. It originally had this hunk of code: > > if (code == SSA_NAME > && (g = SSA_NAME_DEF_STMT (ssa_name)) > - && gimple_code (g) == GIMPLE_CALL) > + && gimple_code (g) == GIMPLE_CALL > + && !gimple_call_internal_p (g)) > pmode = promote_function_mode (type, mode, &unsignedp, > TREE_TYPE > (TREE_TYPE (gimple_call_fn (g))), > > but the !gimple_call_internal_p (g) was turned into an assert. This patch > turns the assert back into a condition. That's because I actually hit that > assert with -fsanitize=signed-integer-overflow on PPC64, where we expand internal > calls at expand time. On PPC64, integers are promoted to long when passed to > a function, that is why the assert doesn't trigger on x86_64. > There shouldn't be any harm in not calling promote_function_mode since internal > functions do not result in a call instruction. > > I'm not attaching any new testcase, because that's not needed - we ICE on PPC64 > with current testsuite. > > Regtested/bootstrapped on powerpc64-unknown-linux-gnu and x86_64-unknown-linux-gnu. > Ok for trunk? Looks good to me - Richard? Thanks, Richard. > [1] http://gcc.gnu.org/ml/gcc-patches/2011-04/msg00609.html > > 2013-12-09 Marek Polacek <polacek@redhat.com> > > PR sanitizer/59399 > * expr.c (expand_expr_real_1): Remove assert dealing with > internal calls and turn that into a condition instead. > > --- gcc/expr.c.mp2 2013-12-09 18:54:16.235624592 +0100 > +++ gcc/expr.c 2013-12-09 18:55:03.580792572 +0100 > @@ -9482,13 +9482,11 @@ expand_expr_real_1 (tree exp, rtx target > the same mode we got when the variable was declared. */ > if (code == SSA_NAME > && (g = SSA_NAME_DEF_STMT (ssa_name)) > - && gimple_code (g) == GIMPLE_CALL) > - { > - gcc_assert (!gimple_call_internal_p (g)); > - pmode = promote_function_mode (type, mode, &unsignedp, > - gimple_call_fntype (g), > - 2); > - } > + && gimple_code (g) == GIMPLE_CALL > + && !gimple_call_internal_p (g)) > + pmode = promote_function_mode (type, mode, &unsignedp, > + gimple_call_fntype (g), > + 2); > else > pmode = promote_decl_mode (exp, &unsignedp); > gcc_assert (GET_MODE (decl_rtl) == pmode); > > Marek > >
Richard Biener <rguenther@suse.de> writes: > On Mon, 9 Dec 2013, Marek Polacek wrote: > >> Back in April 2011, Richard S. submitted the implementation of >> internal functions [1]. It originally had this hunk of code: >> >> if (code == SSA_NAME >> && (g = SSA_NAME_DEF_STMT (ssa_name)) >> - && gimple_code (g) == GIMPLE_CALL) >> + && gimple_code (g) == GIMPLE_CALL >> + && !gimple_call_internal_p (g)) >> pmode = promote_function_mode (type, mode, &unsignedp, >> TREE_TYPE >> (TREE_TYPE (gimple_call_fn (g))), >> >> but the !gimple_call_internal_p (g) was turned into an assert. This patch >> turns the assert back into a condition. That's because I actually hit that >> assert with -fsanitize=signed-integer-overflow on PPC64, where we expand internal >> calls at expand time. On PPC64, integers are promoted to long when passed to >> a function, that is why the assert doesn't trigger on x86_64. >> There shouldn't be any harm in not calling promote_function_mode since internal >> functions do not result in a call instruction. >> >> I'm not attaching any new testcase, because that's not needed - we ICE on PPC64 >> with current testsuite. >> >> Regtested/bootstrapped on powerpc64-unknown-linux-gnu and x86_64-unknown-linux-gnu. >> Ok for trunk? > > Looks good to me - Richard? Yeah, fine with me too. I suppose this wasn't ever likely to hit with the original ARM use. Thanks, Richard
--- gcc/expr.c.mp2 2013-12-09 18:54:16.235624592 +0100 +++ gcc/expr.c 2013-12-09 18:55:03.580792572 +0100 @@ -9482,13 +9482,11 @@ expand_expr_real_1 (tree exp, rtx target the same mode we got when the variable was declared. */ if (code == SSA_NAME && (g = SSA_NAME_DEF_STMT (ssa_name)) - && gimple_code (g) == GIMPLE_CALL) - { - gcc_assert (!gimple_call_internal_p (g)); - pmode = promote_function_mode (type, mode, &unsignedp, - gimple_call_fntype (g), - 2); - } + && gimple_code (g) == GIMPLE_CALL + && !gimple_call_internal_p (g)) + pmode = promote_function_mode (type, mode, &unsignedp, + gimple_call_fntype (g), + 2); else pmode = promote_decl_mode (exp, &unsignedp); gcc_assert (GET_MODE (decl_rtl) == pmode);