https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85599
--- Comment #33 from Steve Kargl <sgk at troutmask dot apl.washington.edu> --- On Fri, May 18, 2018 at 06:23:41PM +0000, janus at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=85599 > > --- Comment #32 from janus at gcc dot gnu.org --- > (In reply to Steve Kargl from comment #31) > > > This PR is not about reordering, but about short-circuiting. > > > > AFAICT this PR is about a function with side-effects, > > and re-ordering can have an potentially undesirable > > result. > > Yah, I guess you can dream up all kinds of possible issues with non-pure > functions, but that is really beyond the scope of this PR. I feel like I have > already opened up a barrel large enough, and have gotten sufficient flak for > that. I'm really not interested in taking this discussion any further. If you > are, please open a new PR for that. > I don't think you're getting flak. I think you're getting discussion, where both Thomas and I disagree with you on whether this is a bug or not. This has nothing to do with a pure or impure function. This has to do with special casing a binop to explicitly require that both arguments must be evaluated, because one of the operands may have a side-effect. The Fortran standard explicitly allows the current behavior. Thomas is willing to compromise to at least issue a warning that in '.false. .and. check()' check() may not be evaluated. I've suggested a new option -fno-short-circuit and a review of all binops to force evaluation of both operands. If I were to give you flak, I would have suggested that you contact the other Fortran vendors with bug reports for a missed optimization. It could take a very long time to evaluate check(). Why bother with the evaluation when the result for '.false. .and. check()' is known regardless of the result for check()? Yes, it is a rhetorical question as I know your answer that check() may have side-effects. And, if I were really to give you flak, I would suggest that a patch is louder than words. svn diff gcc/fortran/trans-expr.c Index: gcc/fortran/trans-expr.c =================================================================== --- gcc/fortran/trans-expr.c (revision 260235) +++ gcc/fortran/trans-expr.c (working copy) @@ -3446,6 +3446,12 @@ gfc_conv_expr_op (gfc_se * se, gfc_expr * expr) if (lop) { + if (expr->value.op.op == INTRINSIC_AND + || expr->value.op.op == INTRINSIC_OR) + { + lse.expr = gfc_evaluate_now (lse.expr, &se->pre); + rse.expr = gfc_evaluate_now (rse.expr, &se->pre); + } /* The result of logical ops is always logical_type_node. */ tmp = fold_build2_loc (input_location, code, logical_type_node, lse.expr, rse.expr); % gfcx -o z a.f90 && ./z check 1 check 2 I completely disagree with this patch without -fno-short-circuit. You're welcomed.
