[flang-commits] [PATCH] D149878: [flang][hlfir] Lower forall to HLFIR

[Tom Eccles via Phabricator via flang-commits]

tblah accepted this revision.
tblah added a comment.

Looks great to me!



================
Comment at: flang/test/Lower/HLFIR/forall.f90:114-115
+! CHECK:  %[[VAL_8:.*]] = fir.call @_QPifoo() fastmath<contract> : () -> i32
+! CHECK:  %[[VAL_9:.*]] = fir.call @_QPjfoo() fastmath<contract> : () -> i64
+! CHECK:  %[[VAL_10:.*]] = fir.call @_QPjbar() fastmath<contract> : () -> i64
+! CHECK:  hlfir.forall lb {
----------------
jeanPerier wrote:
> tblah wrote:
> > As I understand it, the limits to these iterations can be any integer expression (R1127). So jfoo() and jbar() might have side effects and so can't be hoisted outside of the outer forall.
> Thanks for pointing this out! I actually thought I was doing an optimization by hoisting these out, but after reading the standard again, it turns out I think we are //required// to hoist them //precisely// because they may have side effects...  
> 
> First, you are correct that they may have side effects, however, I think the standard tells in 11.1.7.4.2 that the concurrent-limits/steps can be evaluated in any order, and that we must not evaluate jfoo()/jbar() for each i (that we evaluate them exactly and only once).
> 
> Rational:
> > 10.2.4.2.2 Determination of the values for index variables.
> > 1. The values of the index variables are determined as they are for the DO CONCURRENT statement (11.1.7.4.2).
> 
> > 11.1.7.4.2: DO CONCURRENT loop control
> > 1. The concurrent-limit and concurrent-step expressions in the concurrent-control-list are evaluated. > These expressions may be evaluated in any order.
> 
> Plus the standard explicitly adds a constraint that the concurrent control for j cannot depend on i, so that they can be evaluated only once.
> 
> > C1123 (R1126) A concurrent-limit or concurrent-step in a concurrent-control shall not contain a reference to any index-name in the concurrent-control-list in which it appears.
> 
> Regarding the side effects they can have:
> > 10.1.4 Evaluation of operations
> > Execution of a function reference in the logical expression in [...] the concurrent-limits and concurrent-steps in a FORALL statement (10.2.4) is permitted to define variables in the subsidiary [...] forall-assignment-stmt.
> 
> While this allows concurrent-limits/step (of the outer forall only(*) ) expression to have side effect on the variables used in the nested forall-assignment statements, this does not allow the  concurrent-limits/step to have side effect that would affect other concurrent-limit/setp. The standard is quite clear here we could even evaluate jfoo()/jbar() before ifoo() and ibar().
> 
> (*) All this discussions only impact the outer Forall, since C1037 requires every procedure reference inside a Forall body to be pure, which includes the concurrent-control of the nested Forall. So we can evaluate the nested control again an again or never without any correctness implications.
> 
> Do you agree with the reading of the standard?
Ahh yes I think you are right. Thank you for checking this


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149878/new/

https://reviews.llvm.org/D149878