================
@@ -24,7 +25,82 @@ namespace flangomp {
namespace {
namespace looputils {
-using LoopNest = llvm::SetVector<fir::DoLoopOp>;
+/// Stores info needed about the induction/iteration variable for each `do
+/// concurrent` in a loop nest. This includes only for now:
+/// * the operation allocating memory for iteration variable,
+struct InductionVariableInfo {
+ mlir::Operation *iterVarMemDef;
+};
+
+using LoopNestToIndVarMap =
+ llvm::MapVector<fir::DoLoopOp, InductionVariableInfo>;
+
+/// Given an operation `op`, this returns true if one of `op`'s operands is
+/// "ultimately" the loop's induction variable. This helps in cases where the
+/// induction variable's use is "hidden" behind a convert/cast.
+///
+/// For example, give the following loop:
+/// ```
+/// fir.do_loop %ind_var = %lb to %ub step %s unordered {
+/// %ind_var_conv = fir.convert %ind_var : (index) -> i32
+/// fir.store %ind_var_conv to %i#1 : !fir.ref<i32>
+/// ...
+/// }
+/// ```
+///
+/// If \p op is the `fir.store` operation, then this function will return true
+/// since the IV is the "ultimate" opeerand to the `fir.store` op through the
+/// `%ind_var_conv` -> `%ind_var` conversion sequence.
+///
+/// For why this is useful, see its use in `findLoopIndVarMemDecl`.
+bool isIndVarUltimateOperand(mlir::Operation *op, fir::DoLoopOp doLoop) {
----------------
skatrak wrote:
I'm not too sure about this function. What it actually checks is that either
the induction variable of `doLoop` is one of the operands of `op`, that the
operation defining specifically the first operand of `op` has the induction
variable as one of its operands, that the operation defining its own first
operand has it, etc.
Rather than checking whether the induction variable is ultimately one of its
operands, it seems to check that one of its operands is or has been obtained
from (among potentially other values) the induction variable, with the
seemingly arbitrary condition that, if it's obtained indirectly, it must come
from a chain of "first-operands".
If I understand it correctly, the first example would meet that condition,
whereas the second wouldn't. I think that these should both be treated the same:
```mlir
fir.do_loop %ind_var = ... unordered {
%ind_var_conv = fir.convert %ind_var : (index) -> i32
// isIndVarUltimateOperand() returns true
%add = arith.addi %ind_var_conv, %x : i32
...
}
fir.do_loop %ind_var = ... unordered {
%ind_var_conv = fir.convert %ind_var : (index) -> i32
// isIndVarUltimateOperand() returns false
%add = arith.addi %x, %ind_var_conv : i32
...
}
```
If this is intended to handle `fir.convert` and `fir.store`, I think it makes
more sense to actually check for these specific operation types and usage
patterns rather than creating a pseudo-generic check that can easily break. I
don't think the solution is to check all operands, because at that point we're
just checking whether the induction variable participated in the expression
that was passed in, which doesn't seem to be what we want here.
https://github.com/llvm/llvm-project/pull/127633
_______________________________________________
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
