On 01/22/2012 03:38 AM, Tom de Vries wrote:
Sorry I didn't notice this patch until now; please CC me on C++ patches,
or at least mention C++ in the subject line.
+ tree expr = NULL;
+ append_to_statement_list (*block, &expr);
+ *block = expr;
Rather than doing this dance here, I think it would be better to enhance
append_to_statement_list to handle the case of the list argument being a
non-list.
+ cp_walk_tree (&incr, cp_genericize_r, data, NULL);
+ if (incr && EXPR_P (incr))
+ SET_EXPR_LOCATION (incr, start_locus);
It might be better to set the location on incr before genericizing, so
that the location can trickle down.
+ t = build1 (GOTO_EXPR, void_type_node, get_bc_label (bc_break));
+ SET_EXPR_LOCATION (t, start_locus);
Here you can use build1_loc instead of two separate statements.
- /* If we use a LOOP_EXPR here, we have to feed the whole thing
- back through the main gimplifier to lower it. Given that we
- have to gimplify the loop body NOW so that we can resolve
- break/continue stmts, seems easier to just expand to gotos. */
Since we're now lowering the loop at genericize time rather than
gimplify, what's the rationale for not using LOOP_EXPR/EXIT_EXPR? We
should still say something here. I suppose the rationale is that the C
front end currently goes straight to gotos.
if (cond != error_mark_node)
{
- gimplify_expr (&cond, &exit_seq, NULL, is_gimple_val, fb_rvalue);
- stmt = gimple_build_cond (NE_EXPR, cond,
- build_int_cst (TREE_TYPE (cond), 0),
- gimple_label_label (top),
- get_bc_label (bc_break));
- gimple_seq_add_stmt (&exit_seq, stmt);
+ cond = build2 (NE_EXPR, boolean_type_node, cond,
+ build_int_cst (TREE_TYPE (cond), 0));
I don't think we still need this extra comparison to 0, that seems like
a gimple-specific thing.
if (FOR_INIT_STMT (stmt))
+ append_to_statement_list (FOR_INIT_STMT (stmt), &expr);
+ genericize_cp_loop (&loop, EXPR_LOCATION (stmt), FOR_COND (stmt),
+ FOR_BODY (stmt), FOR_EXPR (stmt), 1, walk_subtrees, data);
The call to genericize_cp_loop will clear *walk_subtrees, which means we
don't genericize the FOR_INIT_STMT.
+ tree jump = build_and_jump (&label);
Again, let's use build1_loc.
+ *stmt_p = build_and_jump (&label);
+ SET_EXPR_LOCATION (*stmt_p, location);
And here.
+ stmt = make_node (OMP_FOR);
Why make a new OMP_FOR rather than repurpose the one we already have?
We've already modified its operands.
Jason
