This is an archive of the discontinued LLVM Phabricator instance.

AMDGPU: Fix the broken dominator tree
ClosedPublic

Authored by cfang on Oct 23 2019, 2:41 PM.

Details

Summary

In loadSRsrcFromVGPR, if MBB is the same as Succ, Remiander is not the immediate dominator of Succ.

Diff Detail

Event Timeline

cfang created this revision.Oct 23 2019, 2:41 PM
cfang updated this revision to Diff 226276.Oct 24 2019, 9:15 AM
cfang edited the summary of this revision. (Show Details)

Add test case.

arsenm added inline comments.Oct 24 2019, 3:04 PM
lib/Target/AMDGPU/SIInstrInfo.cpp
4459

->properlyDominates?

arsenm added inline comments.Oct 24 2019, 3:06 PM
test/CodeGen/AMDGPU/immediate-dominator-fix.ll
4

The test and file name need work. This is specifically a problem when the resource descriptor is emitted as a waterfall loop

40

I would prefer using a different way of getting the VGPR resource descriptor. Can you just do the load of the full resource descriptor from memory?

cfang updated this revision to Diff 226356.Oct 24 2019, 4:35 PM
cfang marked 3 inline comments as done.

Update based on the comments

  1. Use properlyDiminates
  2. Change to relevant names for the file and test
  3. Use a full load of the vgpr descriptor.
lib/Target/AMDGPU/SIInstrInfo.cpp
4459

right. Thanks.

arsenm accepted this revision.Oct 25 2019, 9:56 AM

LGTM

This revision is now accepted and ready to land.Oct 25 2019, 9:56 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptOct 25 2019, 1:15 PM
Herald added a subscriber: hiraditya. · View Herald Transcript