This is an archive of the discontinued LLVM Phabricator instance.

[SDag][AMDGPU] Maintain DAG divergence through instruction selection
AbandonedPublic

Authored by foad on Sep 29 2020, 6:28 AM.

Details

Summary

Maintain and verify the IsDivergent property of SDNodes until after
instruction selection. This requires target support for identifying
MachineSDNodes that are known to be divergent or uniform.

Diff Detail

Event Timeline

foad created this revision.Sep 29 2020, 6:28 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 29 2020, 6:28 AM
foad requested review of this revision.Sep 29 2020, 6:28 AM

Currently there are no users of the post-instruction-selection divergence information, but does this seem potentially useful for anything?

I used it for analyzing post-isel DAG dumps, looking for places where we might have selected VALU instructions for uniform nodes for no good reason.

Currently there are no users of the post-instruction-selection divergence information, but does this seem potentially useful for anything?

I used it for analyzing post-isel DAG dumps, looking for places where we might have selected VALU instructions for uniform nodes for no good reason.

I think putting any effort into DAG infrastructure is a waste of time at this point

I think this is useful to have until we completely switch to GlobalISel. I agree with any new effort being low !/$ but since you have made it, I'd vote to get this in.

BTW, I think we can have lit tests to verify this. Dump DAG before and after isel and check with file-check. I am not sure if just dumping with existing dump() routine would dump all the required information but its worthwhile to have a test, IMHO.

That is beneficial to know if an instruction is divergent or uniform even after selection because it cannot be determined purely on a VALU vs SALU basis or register classes. However, I do not like that list of opcodes in the isSDNodeSourceOfDivergence(), it is error prone.

foad added a comment.Sep 29 2020, 10:27 AM

However, I do not like that list of opcodes in the isSDNodeSourceOfDivergence(), it is error prone.

These lists are the SDNode equivalents of GCNTTIImpl::isSourceOfDivergence (which uses some data defined in .td files) and GCNTTIImpl::isAlwaysUniform. I don't know if there's a better way to generate them.

However, I do not like that list of opcodes in the isSDNodeSourceOfDivergence(), it is error prone.

These lists are the SDNode equivalents of GCNTTIImpl::isSourceOfDivergence (which uses some data defined in .td files) and GCNTTIImpl::isAlwaysUniform. I don't know if there's a better way to generate them.

It can use a flag in Desc I think.

It is not clear to me why do we need to query divergence information for MachineSDNode?
After unstruction selection is done we should have all the instructions selected correctly to VALU vs SALU basing on the information that is available at the selection stage.
Thus, we can use isDivergent bit value set for the MachineSDNode in case we need to recompute or update divergence information after selection.
So, instead of adding machine opcodes to isSDNodeSourceOfDivergence it is better to mark that opcodes right away as they are selected.

After the selection is done we'd rather interested in VALU or SALU property instead of divergence. We have lots of opcodes that are VALU even if uniform.
For that we may like to have theseparate analysis pass that could work similar to the divergence analysis but propagate the VALU/SALU property instead of divergence.
BUt creating such a "one more divergence analysis" upon the MIR requires a lot of efforts and strong reasons.

foad added a comment.Nov 16 2020, 8:32 AM

It is not clear to me why do we need to query divergence information for MachineSDNode?

We don't need it. It is just something I found useful when I was doing some debugging and analysis of the code we generate.

arsenm resigned from this revision.Mar 30 2021, 6:19 PM

I guess this is technically correct but I wouldn't spend time getting this accurate

foad abandoned this revision.Mar 31 2021, 1:34 AM

Abandoning. It was just an experiment. There is no real use case for it.