[flang-commits] [flang] [llvm] [mlir] [flang][OpenMP] Lower target in_reduction (PR #199967)

[Sergio Afonso via flang-commits]

================

----------------
skatrak wrote:

This is not the first time I've had to point out in this PR that we're not supposed to make references to Flang in the omp dialect. Admittedly, there are some already around, but they are generally giving an example of a situation that justifies an edge case. This is not the situation here, this reads like what an LLM adds as a comment to justify a change based on the prompt given to it, resulting in an explanation that seems arbitrary without the context of the review discussion. The same comment block also for no reason explains what `verifyMapClause` does, also because it was part of an earlier review comment.

When I previously suggested to "make sure to always review your LLM's output to avoid redundant/irrelevant information" I was referring to this kind of thing. By doing so, I get to complete the review and you get to merge the PR faster. I also doubt we would have gone through so many rounds of review for this PR if it wasn't for how little attention appears to have been put into de-slopifying each version of the patch before asking for another review.

We're all constrained by review throughput, so anything you can do as a contributor to reduce reviewer workload benefits everyone. I hope this doesn't come across as accusatory, I'm only trying to share some perspective about the costs of not paying enough attention to what an LLM generates before asking another person to check it for you.

Also, I think the PR description is missing an "Assisted-by" tag, as per the [LLVM AI Tool Use Policy](https://llvm.org/docs/AIToolPolicy.html).

```suggestion
      // Select the map_entries entry that captures this in_reduction operand.
      // The verifier guarantees at least
      // one match exists; more than one matching entry is a lowering ambiguity
      // (the redirect cannot pick which map argument to rebind).
```

https://github.com/llvm/llvm-project/pull/199967