Hi Yuao,
Yuao Ma wrote:
On Mon, Sep 8, 2025 at 10:54 PM Tobias Burnus <[email protected]> wrote:
BTW, just thinking about it: When simplifying, I wonder
whether we want to have a special case for
cond ? expr1 : expr1
Namely, condition is only known at runtime but both expressions are
identically? I think gfc_dep_compare_expr (...) == 0 does this.
(If you add it, please re-check that my claim is correct.)
It seems like a good optimization to me. However, our current
implementation already seems to have this capability. […]
My initial assumption is that COND_EXPR will perform
this transformation for simple scalar types that we currently support.
I believe we will need this optimization for future complex types, so
perhaps we should hold this until we actually need this part.
Yes, I was thinking about the more complex cases. And this is for later.
I am now actually wondering about side-effects. For:
(f() > 0.0 ? expr1 : expr1)
There can be two side effects: (a) 'f()' modifying some global variable
(unless pure) and (b) for signalling NAN. Thus, we might need to be a
bit careful about 'cond'. - But in any case, we should only expand 'x'
only once – and possibly can just execute (with -ffinite-math) 'f()'
without using the result, additionally avoiding a conditional jump.
But as mentioned, that's for later, but might imply that we want to do
this change late when expanding the condition to tree code rather than
doing it during the simplication.
* * *
Yeah, you convinced me. Just using a nullptr doesn't seem to work.
I came up with another idea: using a gfc_expr with type
EXPR_CONDITIONAL and a condition of nullptr (actually cond/true/false
could all be nullptr).
For any valid cond-expr, the condition must be a valid
scalar-logical-expr. So using a nullptr cond to indicate a .NIL. seems
safe. It is also localized, no need to touch the EXPR_OP part. Not
sure if this will have any bad case, though.
I think that works; it has the downside if being a bit like magic
numbers, but on the pro side it is restricted to EXPR_CONDITIONAL,
which is nice in a way. Still,
(cond ? .nil. : a )
being represented as (cond ? (nullptr ? nullptr : nullptr) : a)
feels slightly odd, but I think there is no ideal solution.
* * *
Regarding the patch:
This patch adds support for conditional expressions in Fortran 2023 for limited
types (logical, numerical), and also includes limited support for conditional
arguments without `.nil.` support.
'limited types' sounds odd – in what sense is a type such as 'real' limited?
I think it should be either 'for some types' or 'for a limited set of types'.
gcc/fortran/ChangeLog:
* dump-parse-tree.cc (show_expr): Add support for EXPR_CONDITIONAL.
* expr.cc (gfc_get_conditional_expr): Add cond-expr constructor.
(gfc_copy_expr): Add support for EXPR_CONDITIONAL.
(free_expr0): Ditto.
(gfc_is_constant_expr): Ditto.
(simplify_conditional): Ditto.
(gfc_simplify_expr): Ditto.
(gfc_check_init_expr): Ditto.
(check_restricted): Ditto.
(gfc_traverse_expr): Ditto.
Note that you can combine all function names in a single function, i.e.:
* expr.cc (gfc_get_conditional_expr): Add cond-expr constructor.
(gfc_copy_expr, free_expr0, gfc_is_constant_expr,
simplify_conditional, gfc_simplify_expr, gfc_check_init_expr,
check_restricted, gfc_traverse_expr): Add support for
EXPR_CONDITIONAL.
avoiding the long list of 'Ditto.'. You still need a ditto/likewise
when you apply the same change to multiple files as in ...
* module.cc (mio_expr): Add support for EXPR_CONDITIONAL.
* resolve.cc (resolve_conditional): Ditto.
(It also makes sense to use it, when you apply the same change as
the previous entry to a function - but want to document some
other change in addition.)
* * *
--- a/gcc/fortran/trans-array.cc
+++ b/gcc/fortran/trans-array.cc
@@ -12713,6 +12713,15 @@ gfc_walk_op_expr (gfc_ss * ss, gfc_expr * expr)
return head2;
}
+static gfc_ss *
+gfc_walk_conditional_expr (gfc_ss *ss, gfc_expr *expr)
+{
+ gfc_ss *head;
+
+ head = gfc_walk_subexpr (ss, expr->value.conditional.true_expr);
+ head = gfc_walk_subexpr (head, expr->value.conditional.false_expr);
+ return head;
+}
I am wondering whether also the condition needs to be walked or only
true and false.
As this relates to arrays, we could also leave out this change until
walking array is supported (cf. below).
* * *
All in all, I think the patch is nearly ready to go.
It handles most cases fine, there are still some corner
case issues - and a wide swath of code unsupported but
rejected with an error.
My idea would be:
(a) To add a sorry for arrays (i.e. expr->rank > 0) - those
are a pretty common and will currently fail with an ICE.
(b) Possibly, fix one or the other issue of the examples
below [or all of them]
(c) Defer the rest.
[For (b), I think the se.pre/se.post issue should really be
handled now, while the rest could be deferred.]
Then, in follow-up work (one or multiple patches), address:
(A) .NIL. as a feature
(B) Handle the remaining non-array issues of below (if any)
Once done, you can look at adding array, character,
derived-type or polymophism support - or do a short detour
to other topics and return.
But now to the examples that illustrate some issues:
* * *
The following seems to require some additional check
to yield a sorry:
internal compiler error: in gfc_conv_variable, at fortran/trans-expr.cc:3197
I wonder whether we should just disallow any array for now?
I bet there are some special cases where it might work,
but so far all array examples, I tried, fail.
implicit none
logical :: cond
integer :: array(4)
array=[1,2,3,4]
cond = .true.
print *, (cond ? array : array + f() ) ) ! ICE
call sub( (cond ? array : array + f() ) ) ! ICE
contains
integer function f()
f = 1
error stop "should not be called"
end
subroutine sub(x)
integer :: x(:)
if (any (x /= [1,2,3,4])) error stop "invalid value"
print *, x
end
end
* * *
Note: Even when rejecting arrays, gfortran might produce questionable code:
implicit none
integer, allocatable :: aa(:), bb(:)
aa = [1,2]
print *, (aa(1) > 0 ? aa(2) : f(bb(::2)))
contains
integer function f(x)
integer :: x(*)
f = 42
end
end
However, it seems to work most of the time...
* * *
But, the following fails ('error stop'):
implicit none
integer :: aa(2)
aa = [1,2]
print *, (aa(1) > 0 ? aa(2) : g())
contains
integer function g()
allocatable :: g
error stop "should not be called"
g = 3
end
end
for the same reason.
I think the solution for the two is:
In gfc_conv_conditional_expr:
if (true_se.pre || flase_se.pre)
gfc_add_expr_to_block (&se->pre,
fold_build3_loc (input_location, COND_EXPR, void_type_node,
condition, true_se.pre ? gfc_finish_block (&true_se.pre) :
build_empty_stmt (input_location)),
(likewise for false_se)
And also the same for {true,false}_se.post.
* * *
The following is might be an issue in the caller, but nonetheless
it fails with conditionals (gfc_match_omp_clauses [openmp.cc:2611]
calling gfc_extract_int) - while it works with arithmetic expressions
and named constants. (Compile with '-fopenmp'.)
implicit none
integer, parameter :: collapse_param = 1
logical :: ll
integer :: i
! !$omp do collapse( collapse_param ) ! OK
! !$omp do collapse( 2 - collapse_param ) OK
!$omp do collapse( (1 > 0 ? 1 : 2) ) ! Fails with: 'Constant expression
required'
do i = 1, 4
end do
end
* * *
implicit none
integer :: i,j
do concurrent (i=j: 5) local(j) ! Error: Variable ‘j’ referenced in
concurrent-header at (1) must not appear in LOCAL locality-spec
end do
do concurrent (i=(j > 1 ? j : 1) : 5) local(j) ! Not diagnosed
end do
end
* * *
The following feels odd, i.e. for a compile-time condition,
I'd expect a warning here - after all, 'gfc_resolve_expr' has
been called - such that the condition should be simplifyable.
(Again, compile with -fopenmp.)
implicit none
integer :: i
!$omp simd simdlen( -1 ) ! Warning: INTEGER expression of SIMDLEN clause at (1)
must be positive
do i = 1, 4
end do
!$omp simd simdlen( (1 > 0 ? -1 : -2) ) ! No warning shown here
do i = 1, 4
end do
end
* * *
Tobias
