Page MenuHomePhabricator

[AMDGPU] Fix high occupancy calculation and print it
ClosedPublic

Authored by rampitec on Jul 29 2019, 3:51 PM.

Details

Summary

We had couple places which still return 10 as a maximum
occupancy. Fixed.

Also print comment about occupancy as compiler see it.

Diff Detail

Event Timeline

rampitec created this revision.Jul 29 2019, 3:51 PM
arsenm added inline comments.Jul 29 2019, 4:06 PM
lib/Target/AMDGPU/AMDGPUSubtarget.h
1322

Should just set a field in the constructor rather than introducing a virtual

rampitec marked an inline comment as done.Jul 29 2019, 4:28 PM
rampitec added inline comments.
lib/Target/AMDGPU/AMDGPUSubtarget.h
1322

That is too early, constructor does not know it yet.
Also look at the other functions of the same family, they are all virtual: getMaxWorkGroupsPerCU, getMinFlatWorkGroupSize, getMaxFlatWorkGroupSize, getMinWavesPerEU, getMaxWavesPerEU(unsigned FlatWorkGroupSize).

rampitec marked 2 inline comments as done.Jul 29 2019, 5:23 PM
rampitec added inline comments.
lib/Target/AMDGPU/AMDGPUSubtarget.h
1322

Well, OK, I still can do it. Just overwrite value in GCNSubtarget(). It looks a little weird because I still have to duplicate this function in the GCNSubtarget(), otherwise C++ has problems with name resolve. Either this or we will always have to specify AMDGPUSubtarget:: prefix.

rampitec updated this revision to Diff 212256.Jul 29 2019, 5:24 PM

Added field MaxWavesPerEU instead of virtualizing the function.

rampitec updated this revision to Diff 212258.Jul 29 2019, 5:32 PM

Removed "GCNSubtarget::getMaxWavesPerEU()" in favor of using "AMDGPUSubtarget::getMaxWavesPerEU;"

arsenm accepted this revision.Jul 30 2019, 5:08 PM

LGTM

This revision is now accepted and ready to land.Jul 30 2019, 5:08 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJul 30 2019, 6:06 PM