Page MenuHomePhabricator

AMDGPU: Fold immediate offset into BUFFER_LOAD_DWORD lowered from SMEM
ClosedPublic

Authored by mareko on Oct 13 2017, 5:50 PM.

Details

Summary

-5.3% code size in affected shaders.

Changed stats only:

48486 shaders in 30489 tests
Totals:
SGPRS: 2086406 -> 2072430 (-0.67 %)
VGPRS: 1626872 -> 1627960 (0.07 %)
Spilled SGPRs: 7865 -> 7912 (0.60 %)
Code Size: 60978060 -> 60188764 (-1.29 %) bytes
Max Waves: 374530 -> 374342 (-0.05 %)

Totals from affected shaders:
SGPRS: 299664 -> 285688 (-4.66 %)
VGPRS: 233844 -> 234932 (0.47 %)
Spilled SGPRs: 3959 -> 4006 (1.19 %)
Code Size: 14905272 -> 14115976 (-5.30 %) bytes
Max Waves: 46202 -> 46014 (-0.41 %)

Diff Detail

Repository
rL LLVM

Event Timeline

mareko created this revision.Oct 13 2017, 5:50 PM
nhaehnle edited edge metadata.Oct 23 2017, 8:14 AM

Some small comments inline. More generally, I think stuff like this belongs in a MIR-level InstCombine pass. We just don't have the infrastructure for that yet, and this patch is a nice improvement in the meantime.

lib/Target/AMDGPU/SIInstrInfo.cpp
3727–3732 ↗(On Diff #119335)

Could you please reduce the nesting here a bit by wrapping this first part in if (Src && Src->isReg())?

3742 ↗(On Diff #119335)

Not a huge deal, but it seems a bit silly not to handle the case of an immediate Offset == 0. Such stuff should mostly be caught earlier, but who knows what kind of nonsense intermediate passes generate again.

P.S.: It would help to use ReviewBoard's feature of defining dependent (parent/child) revisions, like I've just done now. It sucks that this is not done automatically, unfortunately I haven't found a good way to do proper reviews of series with it yet. (This is my mine gripe with all those shiny tools written by web hipsters...)

Another improvement that can be done here is that LLVM sometimes generates OR instead of ADD, and I don't know how to tell when OR means ADD.

mareko updated this revision to Diff 121031.Oct 31 2017, 11:53 AM
  • address feedback
  • Offset == 0 also means that the operand is not an immediate.
This revision was automatically updated to reflect the committed changes.