2011/10/11 Michael Matz <m...@suse.de>: > Hi, > > On Mon, 10 Oct 2011, Kai Tietz wrote: > >> To ensure that we use simple_operand_p in all cases, beside for >> branching AND/OR chains, in same way as before, I added to this function >> an additional argument, by which the looking into comparisons can be >> activated. > > Better make it a separate function the first tests your new conditions, > and then calls simple_operand_p.
Well, either I make it a new function and call it instead of simple_operand_p, or I need to modify old simple_operand_p. The logic of new and old version is incompatible and has not to be called on same tree, as otherwise new check is vain. >> +fold_truth_andor_1 (location_t loc, enum tree_code code, tree truth_type, >> + tree lhs, tree rhs) >> { >> /* If this is the "or" of two comparisons, we can do something if >> the comparisons are NE_EXPR. If this is the "and", we can do something >> @@ -5149,13 +5176,6 @@ fold_truthop (location_t loc, enum tree_ >> build2 (BIT_IOR_EXPR, TREE_TYPE (ll_arg), >> ll_arg, rl_arg), >> build_int_cst (TREE_TYPE (ll_arg), 0)); >> - >> - if (LOGICAL_OP_NON_SHORT_CIRCUIT) >> - { >> - if (code != orig_code || lhs != orig_lhs || rhs != orig_rhs) >> - return build2_loc (loc, code, truth_type, lhs, rhs); >> - return NULL_TREE; >> - } > > Why do you remove this hunk? Shouldn't you instead move the hunk you > added to fold_truth_andor() here. I realize this needs some TLC to > fold_truth_andor_1, because right now it early-outs for non-comparisons, > but it seems the better place. I.e. somehow move the below code into the > above branch, with the associated diddling on fold_truth_andor_1 that it > gets called. This hunk is removed, as it is vain to do here. Btw richi asked for it, and I agree that new TRUTH-AND/OR packing is better done at a single place in fold_truth_andor only. >> + if ((code == TRUTH_ANDIF_EXPR || code == TRUTH_ORIF_EXPR) >> + && (BRANCH_COST (optimize_function_for_speed_p (cfun), >> + false) >= 2) >> + && !TREE_SIDE_EFFECTS (arg1) >> + && LOGICAL_OP_NON_SHORT_CIRCUIT >> + && simple_operand_p (arg1, true)) >> + { >> + enum tree_code ncode = (code == TRUTH_ANDIF_EXPR ? TRUTH_AND_EXPR >> + : TRUTH_OR_EXPR); >> + >> + /* We don't want to pack more then two leafs to an non-IF > > Missing continuation of the sentence? Well, here is a colon missing. >> + If tree-code of left-hand operand isn't an AND/OR-IF code and not >> + equal to CODE, then we don't want to add right-hand operand. >> + If the inner right-hand side of left-hand operand has side-effects, >> + or isn't simple, then we can't add to it, as otherwise we might >> + destroy if-sequence. */ > > >> + if (TREE_CODE (arg0) == code >> + /* Needed for sequence points to handle trappings, and >> + side-effects. */ >> + && !TREE_SIDE_EFFECTS (TREE_OPERAND (arg0, 1)) >> + && simple_operand_p (TREE_OPERAND (arg0, 1), true)) >> + { >> + tem = fold_build2_loc (loc, ncode, type, TREE_OPERAND (arg0, 1), >> + arg1); >> + return fold_build2_loc (loc, code, type, TREE_OPERAND (arg0, 0), >> + tem); >> + } >> + /* Needed for sequence points to handle trappings, and side-effects. >> */ >> + else if (!TREE_SIDE_EFFECTS (arg0) >> + && simple_operand_p (arg0, true)) >> + return fold_build2_loc (loc, ncode, type, arg0, arg1); >> + } >> + > > > Ciao, > Michael. > Kai