[llvm] [NVPTX] Attempt to load params using symbol addition node directly (PR #119935)

Kevin McAfee via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 8 11:13:28 PST 2025


kalxr wrote:

Some background in [this comment](https://github.com/llvm/llvm-project/pull/119935#discussion_r1907679302) and up the thread. My perspective is that the different output is fully explainable by the `--debug-counter=dagcombine=0` option acting as intended, which is to say that it:
1) Only affects assert builds.
2) Disables all DAG combines.
3) Only disables DAG combines.

It seems that (1) may not be desirable. I don't have a strong opinion on it, and I lack the context and authroity to make any kind of decision about it. If there is disagreement over whether it is true, I would appreciate an alternative explanation.

It seems that there is potentially disagreement over whether (2) and (3) are true. I have claimed that the debug counter used in the test is referenced in one place and only one place - `DAGCombiner::combine` - and the only impact it can have is returning early without attempting a combine. That is point (3). If there is disagreement on that, I would appreciate an alternative explanation.

(2) is less easily verifiable, but I would argue it doesn't necessarily matter if it's true. If we want NVPTX to function correctly with a legalized SelectionDAG where an arbitrary set of nodes did/didn't go through combines, then we probably want a fix like the one proposed in this MR. If for some reason we only want things to work when ZERO combines occurred, then maybe we need something else, but I can't imagine why we would want that.

If there is disagreement with any of the above, we need to sort that out before we'll get anywhere. If we agree, then we know why the compiler output is different with assertions and we know that the compiler output changes are intentional. I think all that is left is to decide whether the changes are something we want to handle correctly, i.e. should NVPTX be capable of handling a legalized SelectionDAG where an arbitrary set of nodes went through combines? 



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


More information about the llvm-commits mailing list