[llvm] [AMDGPU] Add target hook to isGlobalMemoryObject (PR #112781)

Austin Kerbow via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 17 14:46:51 PDT 2024


kerbowa wrote:

A previous version of this patch was posted a while ago on phabricator. We need to address this bug now since there are confirmed cases where it is impacting correctness.

I've experimented with about 4 different patches to solve this. After doing that work, I've just come full circle back to this patch. It is the best, and most correct way to solve the problem. I'll explain why below.

For background, we want to have custom DAG building with IGLP instructions so preferably the generic DAG builder would not add any edges for these instructions, and we can then add custom edges in our IGLP mutation.

The IGLP instructions do have unmolded side effects, but our backend can reason about them and relax the edges that would normally be added. To do that this patch introduces a target hook to the DAG builder to decide whether or not to add edges for side effect producing instructions.

In that old review Matt suggested an alternative approach to add a pseudo memory operand to these instructions that doesn't alias anything. This approach doesn't work since it leads to the exact same problem as before. Even if an instruction doesn't alias anything there will still be valid edges added between other instructions with unmodeled side effects. We end up with the same bug but it will be even more of an edge case.

Another approach is to remove all of these instructions from the block before invoking the scheduler and DAG builder, record information about them, and apply the DAG mutations accordingly, then finally re-add the instructions after scheduling. This is needlessly expensive and hacky.

A final approach is to modify the instruction description of these instructions to not have side effects in our backend before scheduling. An obvious hack.

The best approach is instead to admit that these instructions do have difficult to model side effects, however our backend can reason about them so we want to modify the DAG builder accordingly. In my opinion, this hook is the best way to do that.

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


More information about the llvm-commits mailing list