Page MenuHomePhabricator

AMDGPU/SI: DAGMutation for removing deps between CSEs
AbandonedPublic

Authored by arsenm on Feb 2 2017, 7:07 AM.

Details

Reviewers
tstellarAMD

Diff Detail

Event Timeline

tstellarAMD created this revision.Feb 2 2017, 7:07 AM

Correctly remove predecessors and clean up some of the loops.

arsenm added inline comments.Feb 2 2017, 9:36 AM
lib/Target/AMDGPU/GCNSchedStrategy.cpp
394–395

I thought MachineCSE ran well before the scheduler?

test/CodeGen/AMDGPU/init-m0-sched-deps.mir
22–26

Can you add a few more complex cases, like multiple operations using the same def of m0

arsenm added inline comments.Feb 2 2017, 9:46 AM
lib/Target/AMDGPU/GCNSchedStrategy.cpp
319–323

This is broader than the move immediate case. There need to be some tests where this happens for other instruction types

326–329

I think trying to handle multiple defs will be hazardous

arsenm commandeered this revision.Feb 10 2017, 10:55 AM
arsenm updated this revision to Diff 88019.
arsenm edited reviewers, added: tstellarAMD; removed: arsenm.

Add more tests

Should this try to delete the redundant moves itself? MachineCSE runs earlier on SSA, so this isn't getting cleaned up. It helps with the clustering, but I get sequences of re-initing m0 to -1 like:
s_mov_b32 m0, -1
s_mov_b32 m0, -1
s_mov_b32 m0, -1
s_mov_b32 m0, -1
v_mov_b32_e32 v50, 0x4a8
ds_read2_b32 v[82:83], v50 offset1:1
ds_read2_b32 v[50:51], v35 offset1:1

Should this try to delete the redundant moves itself? MachineCSE runs earlier on SSA, so this isn't getting cleaned up. It helps with the clustering, but I get sequences of re-initing m0 to -1 like:

I tried to have the DAGMutation delete the redundant moves, but I started getting assertion failures all over (maybe you can get this to work, not sure). Might have to just run MachineCSE after scheduling.

Should this try to delete the redundant moves itself? MachineCSE runs earlier on SSA, so this isn't getting cleaned up. It helps with the clustering, but I get sequences of re-initing m0 to -1 like:

I tried to have the DAGMutation delete the redundant moves, but I started getting assertion failures all over (maybe you can get this to work, not sure). Might have to just run MachineCSE after scheduling.

I would be somewhat surprised if the scheduler allowed deleting instructions, I doubt anywhere else does this

t-tye added a subscriber: t-tye.Mar 22 2017, 6:38 PM
tony-tye removed a subscriber: tony-tye.Mar 22 2017, 6:50 PM
arsenm abandoned this revision.Feb 21 2019, 6:46 PM

I vaguely remember finding an alternative to this