Page MenuHomePhabricator

[AMDGPU] Workaround for LDS Misalignment bug on GFX10
ClosedPublic

Authored by mbrkusanin on Thu, Sep 3, 7:54 AM.

Details

Reviewers
arsenm
foad
mareko
Summary

Add subtarget feature check to avoid using ds_read/write_b96/128 with too low
alignment if a bug is present on that specific hardware.
Add this "feature" to GFX 10.1.1 as it is also affected.
Add global-isel test.

Diff Detail

Event Timeline

mbrkusanin created this revision.Thu, Sep 3, 7:54 AM
mbrkusanin requested review of this revision.Thu, Sep 3, 7:54 AM

Note that tests with flat instructions are not copied to GlobalISel/lds-misaligned-bug.ll. While we can put similar check in allowsMisalignedMemoryAccessesImpl for flat address space as well it will cause SDag to produce less optimal code. For some reason it will break down a load 16, align 8 into four flat_load_dword instead of two flat_load_dwordx2 instructions (but not similar stores). This patch should fix problems mentioned in D84403 while I look into this.

arsenm added a comment.Thu, Sep 3, 7:56 AM

Can you add a comment to hasLDSMisalignedBug with what specifically is broken? Is b64 broken too?

piotr added a subscriber: piotr.Thu, Sep 3, 9:16 AM
mbrkusanin updated this revision to Diff 289924.Fri, Sep 4, 5:22 AM
  • Updated description of FeatureLdsMisalignedBug to match what is covered by tests.

Can you add a comment to hasLDSMisalignedBug with what specifically is broken? Is b64 broken too?

Is the updated description enough or do you prefer an explicit list of all instructions for hasLDSMisalignedBug? Something like:

// Hardware requires natural alignment for the following:
// ds_read/write_b64/96/128
// flat_load/store_dwordx2/3/4

b64 is affected be we currently use ds_read2_b32/write2_b32 instructions. We could potentially relax restrictions for b64 the same way it was done for b96/128 to make it more consistent.

mbrkusanin updated this revision to Diff 290441.Tue, Sep 8, 2:55 AM
mbrkusanin edited the summary of this revision. (Show Details)
  • Added FeatureLdsMisalignedBug to GFX 10.1.1
foad accepted this revision.Tue, Sep 8, 3:52 AM

Looks good to me if Matt has no further comments. I'm not sure whether gfx10.1.1 has the bug but it's certainly safe to assume it does, unless/until we know otherwise.

This revision is now accepted and ready to land.Tue, Sep 8, 3:52 AM
arsenm accepted this revision.Tue, Sep 8, 7:42 AM