------- 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

Reply via email to