Page MenuHomePhabricator

[AMDGPU] Disable d16 loads/stores to high halves on gfx90a
AbandonedPublic

Authored by krzysz00 on Oct 27 2021, 10:30 AM.

Details

Reviewers
t-tye
arsenm
Group Reviewers
Restricted Project
Summary

Testing reveals that, on gfx90a, *_load_short_d16_hi does note
preserve the low-order bits of the destination register, even with
SramECC disabled. To prevent the generation of semantically incorrect
code, disable D16PreservesUnesedBits on this target.

(A more specific test may be needed to account for future gfx90*
targets, but checking for gfx90a instructions was what was in the
existing code)

Diff Detail

Event Timeline

krzysz00 created this revision.Oct 27 2021, 10:30 AM
krzysz00 requested review of this revision.Oct 27 2021, 10:30 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 27 2021, 10:30 AM
krzysz00 added a reviewer: Restricted Project.Oct 27 2021, 10:32 AM

Needs test.

llvm/lib/Target/AMDGPU/GCNSubtarget.h
594–596

Typo gff90a.

@rampitec What sort of test would be appropriate? I don't know much about this part of LLVM

@rampitec What sort of test would be appropriate? I don't know much about this part of LLVM

Add run lines and checks for gfx90a to load-hi16.ll and store-hi16.ll.

Typo in commit message "does note". I don't think we can go ahead and infer the behavior here based on simple experimentation on one system. IIRC there was a combination of firmware and kernel driver fixes to get the correct behavior which you may be missing cc @t-tye

krzysz00 updated this revision to Diff 382763.Oct 27 2021, 1:14 PM
  • Update tests for the gfx90a changes
arsenm requested changes to this revision.Nov 9 2021, 9:26 AM

You're probably experiencing a driver issue

This revision now requires changes to proceed.Nov 9 2021, 9:26 AM

Typo in the summary: "D16PreservesUnesedBits" -> "D16PreservesUnusedBits"

krzysz00 abandoned this revision.Fri, Jan 7, 8:29 AM

Confirmed to be a kernel bug, closing.