Commit Message

Hello,
This is another step towards moving GIMPLE_SWITCH expansion to an
earlier point in the pipeline.
With the attached patch, some of the logic from stmt.c:add_case_node()
is moved to gimplify.c:gimplify_switch_expr(). This includes:
* Code to drop case labels that are out of range for the switch index
expression. (Actually, I suspect this code hasn't worked properly
since gimplification was introduced, because the switch index
expression can be promoted by language specific gimplification, so
expand_case never actually sees the proper type with the current
implementation in stmt.c.)
* Code to fold_convert case label values to the right type. I've opted
to go for folding to the original type of the SWITCH_EXPR, rather than
to the post-gimplification switch index type.
* Code to canonicalize CASE_LABEL's subnodes, CASE_LOW and CASE_HIGH.
I've chosen to impose strict requirements that CASE_HIGH > CASE_LOW if
CASE_HIGH is non-zero. This is different from what add_case_node does,
but I think it makes sense to go for the minimal representation here:
The case labels in stmt.c never lived very long (only during expand)
but GIMPLE_SWITCH statements stay around for much of the compilation
process and can also be streamed out, etc.
Bootstrapped and tested on powerpc-unknown-linux-gnu. OK for trunk?
Ciao!
Steven
* targhooks.c (default_case_values_threshold): Fix code style nit.
* stmt.c (add_case_node, expand_case): Move logic to remove/reduce
case range and type folding from here...
* gimplify.c (gimplify_switch_expr): ... to here.

Comments

On Wed, 18 Apr 2012, Steven Bosscher wrote:
> Hello,> > This is another step towards moving GIMPLE_SWITCH expansion to an> earlier point in the pipeline.> > With the attached patch, some of the logic from stmt.c:add_case_node()> is moved to gimplify.c:gimplify_switch_expr(). This includes:> > * Code to drop case labels that are out of range for the switch index> expression. (Actually, I suspect this code hasn't worked properly> since gimplification was introduced, because the switch index> expression can be promoted by language specific gimplification, so> expand_case never actually sees the proper type with the current> implementation in stmt.c.)> > * Code to fold_convert case label values to the right type. I've opted> to go for folding to the original type of the SWITCH_EXPR, rather than> to the post-gimplification switch index type.> > * Code to canonicalize CASE_LABEL's subnodes, CASE_LOW and CASE_HIGH.> I've chosen to impose strict requirements that CASE_HIGH > CASE_LOW if> CASE_HIGH is non-zero. This is different from what add_case_node does,> but I think it makes sense to go for the minimal representation here:> The case labels in stmt.c never lived very long (only during expand)> but GIMPLE_SWITCH statements stay around for much of the compilation> process and can also be streamed out, etc.> > Bootstrapped and tested on powerpc-unknown-linux-gnu. OK for trunk?
I wonder about
@@ -1575,6 +1575,9 @@ gimplify_switch_expr (tree *expr_p, gimple_seq *pr
tree switch_expr = *expr_p;
gimple_seq switch_body_seq = NULL;
enum gimplify_status ret;
+ tree index_type = TREE_TYPE (switch_expr);
+ if (index_type == void_type_node)
+ index_type = TREE_TYPE (SWITCH_COND (switch_expr));
what frontends use void_type_node here contrary to the docs in tree.def:
TREE_TYPE is the original type of the condition, before any
language required type conversions. It may be NULL, in which case
the original type and final types are assumed to be the same.
which says you'd need to handle NULL instead? Thus, can you either
adjust the docs in tree.def or the frontends?
Ok with either change.
Thanks,
Richard.

On Wed, Apr 18, 2012 at 1:23 PM, Richard Guenther <rguenther@suse.de> wrote:
> I suppose generic tree handling routines are confused by NULL TREE_TYPE> and thus changing the docs to void_type_node would be more appropriate.
I don't agree with that. The documented behavior is much older than
either the Fortran or the go front end, so the safest way is to fix
those front ends to match the documentation.
With my patch modified as attached, bootstrap&test still passes. OK?
Ciao!
Steven

On Wed, Apr 18, 2012 at 5:17 PM, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
> On Wed, Apr 18, 2012 at 1:23 PM, Richard Guenther <rguenther@suse.de> wrote:>> I suppose generic tree handling routines are confused by NULL TREE_TYPE>> and thus changing the docs to void_type_node would be more appropriate.>> I don't agree with that. The documented behavior is much older than> either the Fortran or the go front end, so the safest way is to fix> those front ends to match the documentation.>> With my patch modified as attached, bootstrap&test still passes. OK?
The Go bits approved on IRC by Iant, the Fortran bits are obvious, and
the rest was already approved. This is r186579 now.
Ciao!
Steven