------- Additional Comments From roger at eyesopen dot com 2005-03-25 06:03 ------- Splitting non_value into maybe_lvalue_p is a good thing, is totally safe and is preapproved for both mainline and the 4.0 branch. The remaining change to fold_ternary/fold_cond_expr_with_comparison are more controversial, and can theoretically be discussed independently.
Reading through each of the transformations of COND_EXPR in fold, all of the problematic transformations are guarded in the block beginning at line 4268 of fold-const.c. This is the set of "A op B ? A : B" transformations. All other transformations are either lvalue-safe, or require either operand 2 or operand 3 to be a non-lvalue (typically operand 3 must be a constant). I believe a suitable 4.0-timescale (grotesque hack workaround) is (untested): --- 4265,4275 ---- a number and A is not. The conditions in the original expressions will be false, so all four give B. The min() and max() versions would give a NaN instead. */ ! if (operand_equal_for_comparison_p (arg01, arg2, arg00) ! && (in_gimple_form ! || strcmp (lang_hooks.name, "GNU C++") != 0 ! || ! maybe_lvalue_p (arg1) ! || ! maybe_lvalue_p (arg2))) { tree comp_op0 = arg00; tree comp_op1 = arg01; The maybe_lvalue_p tests should be obvious from the previous versions of Alexandre's patch. The remaining two lines are both hideous hacks. The first is that these transformations only need to be disabled for the C++ front-end. This is (AFAIK) the only language front-end that uses COND_EXPRs as lvalues, and disabling "x > y ? x : y" into MAX_EXPR (where x and y are VAR_DECLs) is less than ideal for the remaining front-ends. The second equally bad hack is to re-allow this transformation even for C++ once we're in the tree-ssa optimizers. I believe once we're in gimple, COND_EXPR is no longer allowed as the lhs of an assignment, hence the MAX_EXPR/MIN_EXPR recognition transformations are the gimple-level should be able to clean-up any rvalue COND_EXPRs they're presented with. Clearly, testing lang_hooks.name is an option of last resort. I'm increasingly convinced that the correct long term solution is to introduce a new LCOND_EXPR tree node for use by the C++ end. Either as a C++-only tree code, or a generic tree code. Additionally, depending upon whether we go ahead and deprecate >?= and <?= operators, we may or may not also require LMIN_EXPR and LMAX_EXPR. This allows us to correctly separate the semantics: MIN_EXPR is commutative, LMIN_EXPR isn't, MIN_EXPR is not an lvalue, LMIN_EXPR is. If either operand of a L*_EXPR is not an lvalue, it can be folded to the rvalue form. The problematic transformations above can then be controlled via COND_EXPR vs. LCOND_EXPR rather than checking the front-end. cp/call.c's build_conditional_expr is then tweaked to use LCOND_EXPR instead of COND_EXPR. Finally either with tweaks to the grammar, gimplification or g++'s other tree manipulation machinery we can introduce a lower_to_rvalue call to recursively convert LCOND_EXPR into COND_EXPR, as soon as we know the expression can't be used as an lvalue. [Aside: In fact, a fold_rvalue could be used by the middle-end, to additionally handle cases such as const_array[0] which can't currently be handled due to the fear of being used in an lvalue-context, such as &(const_array[0]). Whilst I see LCOND_EXPR (and maybe LMIN_EXPR, LMAX_EXPR) as the long-term way to go, this is unquestionably too disruptive for the gcc-4_0-branch. This leaves us with two options, either suffer an rare miscompilation or take a performance hit in the sake of correctness. I'm in favour of the latter provided we don't unfairly impact the C/fortran/ada and Java front-ends that aren't otherwise affected. The above changes even attempt to allow the std::min and std::max operators to continue to be optimized, in an attempt to limit the performance impact to pre-gimple tree-folding during C++ parsing. Alex, could you confirm that the above suggestion resolves the PR when used in combination with your maybe_lvalue_p split? Mark, do you agree that this is a reasonable (the most reasonable?) compromise in the 4.0 timeframe? I'm not proud of querying the front-end in fold-const.c, but the semantics required of COND_EXPR by the C++ front-end conflict with those of the other front-ends. Normally this means different tree codes, but that's a luxury I don't think we can currently afford. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19199