| Message ID | 1363268930-22910-1-git-send-email-james.greenhalgh@arm.com |
|---|---|
| State | New |
| Headers | show |
On 03/14/2013 09:48 AM, James Greenhalgh wrote:
> Is this OK to commit to 4.9 when stage 1 opens up?
Yes, but please add the other new tree codes as well.
Jason
On Thu, Mar 14, 2013 at 02:51:30PM -0400, Jason Merrill wrote: > On 03/14/2013 09:48 AM, James Greenhalgh wrote: > >Is this OK to commit to 4.9 when stage 1 opens up? > > Yes, but please add the other new tree codes as well. I wonder if it wouldn't be better to fold the target builtins only later on (e.g. guard the folding with cfun && gimple_in_ssa_p (cfun) (or if we have any predicate that is set starting with gimplification or so)). Having all the FEs have to deal with myriads of weird tree codes etc. isn't IMHO desirable. Jakub
On Thu, Mar 14, 2013 at 1:55 PM, Jakub Jelinek <jakub@redhat.com> wrote: > On Thu, Mar 14, 2013 at 02:51:30PM -0400, Jason Merrill wrote: >> On 03/14/2013 09:48 AM, James Greenhalgh wrote: >> >Is this OK to commit to 4.9 when stage 1 opens up? >> >> Yes, but please add the other new tree codes as well. > > I wonder if it wouldn't be better to fold the target builtins only later on > (e.g. guard the folding with cfun && gimple_in_ssa_p (cfun) (or if we have > any predicate that is set starting with gimplification or so)). > Having all the FEs have to deal with myriads of weird tree codes etc. isn't > IMHO desirable. Agreed. -- Gaby
On Thu, 14 Mar 2013, Jakub Jelinek wrote: > I wonder if it wouldn't be better to fold the target builtins only later on > (e.g. guard the folding with cfun && gimple_in_ssa_p (cfun) (or if we have > any predicate that is set starting with gimplification or so)). > Having all the FEs have to deal with myriads of weird tree codes etc. isn't > IMHO desirable. Wouldn't that prevent from using those builtins in constant expressions? That seems undesirable. Maybe an alternative could be to push some of the functionality from potential_constant_expression_1 to the middle-end?
On Thu, Mar 14, 2013 at 10:08 PM, Marc Glisse <marc.glisse@inria.fr> wrote: > On Thu, 14 Mar 2013, Jakub Jelinek wrote: > >> I wonder if it wouldn't be better to fold the target builtins only later >> on >> (e.g. guard the folding with cfun && gimple_in_ssa_p (cfun) (or if we have >> any predicate that is set starting with gimplification or so)). >> Having all the FEs have to deal with myriads of weird tree codes etc. >> isn't >> IMHO desirable. > > > Wouldn't that prevent from using those builtins in constant expressions? > That seems undesirable. Maybe an alternative could be to push some of the > functionality from potential_constant_expression_1 to the middle-end? True, but is that bad? If we want to delay such folding then please don't do it with a magic flag but instead do the folding only via fold_stmt - that is, add a new target hook that folds a gimple call. I bet we have only a very limited set of target call foldings, so transitioning them all to fold gimple calls would be easy. Richard. > -- > Marc Glisse
On Fri, Mar 15, 2013 at 3:51 AM, Richard Biener <richard.guenther@gmail.com> wrote: > On Thu, Mar 14, 2013 at 10:08 PM, Marc Glisse <marc.glisse@inria.fr> wrote: >> On Thu, 14 Mar 2013, Jakub Jelinek wrote: >> >>> I wonder if it wouldn't be better to fold the target builtins only later >>> on >>> (e.g. guard the folding with cfun && gimple_in_ssa_p (cfun) (or if we have >>> any predicate that is set starting with gimplification or so)). >>> Having all the FEs have to deal with myriads of weird tree codes etc. >>> isn't >>> IMHO desirable. >> >> >> Wouldn't that prevent from using those builtins in constant expressions? >> That seems undesirable. Maybe an alternative could be to push some of the >> functionality from potential_constant_expression_1 to the middle-end? > > True, but is that bad? > > If we want to delay such folding then please don't do it with a magic flag > but instead do the folding only via fold_stmt - that is, add a new target hook > that folds a gimple call. I bet we have only a very limited set of target call > foldings, so transitioning them all to fold gimple calls would be easy. upon reflection, I think we don't want to delay. the C++ front-end needs to know (while type checking) whether a certain operation can be evaluated at compile time and possibly get the value of the operation. -- Gaby
On Fri, Mar 15, 2013 at 08:00:50AM -0500, Gabriel Dos Reis wrote: > On Fri, Mar 15, 2013 at 3:51 AM, Richard Biener > <richard.guenther@gmail.com> wrote: > > On Thu, Mar 14, 2013 at 10:08 PM, Marc Glisse <marc.glisse@inria.fr> wrote: > >> On Thu, 14 Mar 2013, Jakub Jelinek wrote: > >> > >>> I wonder if it wouldn't be better to fold the target builtins only later > >>> on > >>> (e.g. guard the folding with cfun && gimple_in_ssa_p (cfun) (or if we have > >>> any predicate that is set starting with gimplification or so)). > >>> Having all the FEs have to deal with myriads of weird tree codes etc. > >>> isn't > >>> IMHO desirable. > >> > >> > >> Wouldn't that prevent from using those builtins in constant expressions? > >> That seems undesirable. Maybe an alternative could be to push some of the > >> functionality from potential_constant_expression_1 to the middle-end? > > > > True, but is that bad? > > > > If we want to delay such folding then please don't do it with a magic flag > > but instead do the folding only via fold_stmt - that is, add a new target hook > > that folds a gimple call. I bet we have only a very limited set of target call > > foldings, so transitioning them all to fold gimple calls would be easy. > > upon reflection, I think we don't want to delay. the C++ front-end > needs to know (while type checking) whether a certain operation > can be evaluated at compile time and possibly get the value of the > operation. If all arguments to the target builtin constant, then it better not be folded into REDUC_PLUS_EXPR, but instead just some constant. That is just fine. If all arguments to the target builtin aren't constant, then it won't be a constant expression anyway, there is no point in showing all those weird tree codes to the FE. Jakub
> From: Jakub Jelinek [mailto:jakub@redhat.com] > Sent: 15 March 2013 13:06 > On Fri, Mar 15, 2013 at 08:00:50AM -0500, Gabriel Dos Reis wrote: > > On Fri, Mar 15, 2013 at 3:51 AM, Richard Biener > > <richard.guenther@gmail.com> wrote: > > > On Thu, Mar 14, 2013 at 10:08 PM, Marc Glisse > <marc.glisse@inria.fr> wrote: > > >> On Thu, 14 Mar 2013, Jakub Jelinek wrote: > > >> > > >>> I wonder if it wouldn't be better to fold the target builtins > only later > > >>> on > > >>> (e.g. guard the folding with cfun && gimple_in_ssa_p (cfun) (or > if we have > > >>> any predicate that is set starting with gimplification or so)). > > >>> Having all the FEs have to deal with myriads of weird tree codes > etc. > > >>> isn't > > >>> IMHO desirable. > > >> > > >> > > >> Wouldn't that prevent from using those builtins in constant > expressions? > > >> That seems undesirable. Maybe an alternative could be to push some > of the > > >> functionality from potential_constant_expression_1 to the middle- > end? > > > > > > True, but is that bad? > > > > > > If we want to delay such folding then please don't do it with a > magic flag > > > but instead do the folding only via fold_stmt - that is, add a new > target hook > > > that folds a gimple call. I bet we have only a very limited set of > target call > > > foldings, so transitioning them all to fold gimple calls would be > easy. > > > > upon reflection, I think we don't want to delay. the C++ front-end > > needs to know (while type checking) whether a certain operation > > can be evaluated at compile time and possibly get the value of the > > operation. > > If all arguments to the target builtin constant, then it better not be > folded into REDUC_PLUS_EXPR, but instead just some constant. That is > just > fine. If all arguments to the target builtin aren't constant, then it > won't > be a constant expression anyway, there is no point in showing all those > weird tree codes to the FE. So based on this argument, while a front-end may not need to implement all the weird tree codes it should at least have a sensible default case for an tree code which it doesn't understand. That would suggest the correct fix would be to change potential_constant_expression_1 to simply return false if it encounters a tree it doesn't know about, rather than the current gcc_unreachable (). The argument would be; if we found an unexpected tree code then it must have been introduced by folding. If it was a constant, folding would have returned the constant not the expression. So it must not be a constant. Is that be sensible? It certainly seems like someone intended to explicitly enumerate all the possible cases and ensure that they were correctly handled. I have some other patches kicking around which will fold to REDUC_MAX_EXPR, which will be another unhandled tree code. The approach of adding a case in potential_constant_expression_1 each time a shiny new tree code appears just feels wrong. James
On Wed, Mar 20, 2013 at 1:03 PM, James Greenhalgh <james.greenhalgh@arm.com> wrote: > Is that be sensible? It certainly seems like someone intended to > explicitly enumerate all the possible cases and ensure that they were > correctly handled. That someone would be me. We need to catch loudly any front-end tree code, e.g. ASTs, object we may have missed, as opposed to silently ignoring them with possible miscompilation and pray that someone might be sufficiently pissed off and report it as a bug. What is wrong isn't that the front-end inserts internal coverage check; rather it is the fact that we don't have enough separation between front-end asts and middle-end stuff. The convenience of adding a middle-end optimization (which this essentially is) should not trump correctness of the implementation of standard semantics. -- Gaby
On 03/20/2013 02:03 PM, James Greenhalgh wrote: > The argument would be; if we found an unexpected tree code then > it must have been introduced by folding. If it was a constant, > folding would have returned the constant not the expression. So it must > not be a constant. Unless the operands involve calls to constexpr functions, which aren't folded by fold. Jason
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c index 9446f83..232057a 100644 --- a/gcc/cp/semantics.c +++ b/gcc/cp/semantics.c @@ -8600,6 +8600,7 @@ potential_constant_expression_1 (tree t, bool want_rval, tsubst_flags_t flags) case ABS_EXPR: case TRUTH_NOT_EXPR: case FIXED_CONVERT_EXPR: + case REDUC_PLUS_EXPR: case UNARY_PLUS_EXPR: return potential_constant_expression_1 (TREE_OPERAND (t, 0), rval, flags);