Page MenuHomePhabricator

[AMDGPU] Enable structurizer workarounds by default
ClosedPublic

Authored by sameerds on Jun 4 2020, 7:16 PM.

Details

Summary

Added some reviewers based on the git log of tests that are affected by this change. The change itself is a one-liner that simply switches the default value of a command-line option in the backend. But it affects control flow and each of the four affected tests needs to be checked. It is possible that some tests become redundant due to the way the control flow is now organized.

Diff Detail

Event Timeline

sameerds created this revision.Jun 4 2020, 7:16 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 4 2020, 7:17 PM
sameerds edited the summary of this revision. (Show Details)Jun 4 2020, 7:21 PM
cfang added inline comments.Jun 4 2020, 8:52 PM
llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
197

Can we come out with a name that is closer to what was implemented, like amdgpu-unify-loop-exits, or even amdgpu-normalize-loops(cfg)? "workarounds" is kind of misleading.

sameerds marked an inline comment as done.Jun 4 2020, 9:38 PM
sameerds added inline comments.
llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp
197

This one option controls more than one pass. The intention of naming this option is to indicate to the user that additional things are being done to work around known structurizer limitations. Each of these pass is used to avoid a specific situation where the structurizer is know to fail. We could have one option per pass like the names you suggest, but that sounds like overkill. We might even add more passes to the set of workarounds in the future.

nhaehnle accepted this revision.Jun 8 2020, 3:11 AM

LGTM

This revision is now accepted and ready to land.Jun 8 2020, 3:11 AM
This revision was automatically updated to reflect the committed changes.