This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPUCodegenPrepare] Add NewPM Support
ClosedPublic

Authored by gandhi21299 on May 23 2023, 12:08 PM.

Diff Detail

Event Timeline

gandhi21299 created this revision.May 23 2023, 12:08 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 23 2023, 12:08 PM
gandhi21299 requested review of this revision.May 23 2023, 12:08 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 23 2023, 12:08 PM
gandhi21299 added a project: Restricted Project.
arsenm accepted this revision.May 23 2023, 2:22 PM
arsenm added inline comments.
llvm/lib/Target/AMDGPU/AMDGPUCodeGenPrepare.cpp
1901

I'd assume we could preserve some of these

This revision is now accepted and ready to land.May 23 2023, 2:22 PM
  • Right, I believe all analyses are preserved @arsenm
  • Right, I believe all analyses are preserved @arsenm

definitely doesn't look like all analyses are preserved, the pass adds instructions
if the pass doesn't modify the control flow graph, then you can preserve CFGAnalyses

  • Preserve CFGAnalyses if the function was modified (asserting no structural changes). Preserve all analyses otherwise.
  • rebase against main
aeubanks accepted this revision.May 24 2023, 9:50 AM

lgtm (I didn't verify that the pass actually doesn't make CFG changes)

llvm/lib/Target/AMDGPU/AMDGPUCodeGenPrepare.cpp
1883–1886

put this in the if block

1888–1890

extraneous empty line

gandhi21299 marked 3 inline comments as done.
  • Suggested edits

All insertions/deletions are of instructions and none of them are related to branch instructions. Hence, CFGAnalyses are preserved.

arsenm added inline comments.May 24 2023, 10:14 AM
llvm/lib/Target/AMDGPU/AMDGPUCodeGenPrepare.cpp
1885

The CFG does change if the div expansion happens

Preserve TargetLibraryAnalysis and AssumptionAnalysis in all cases. If no changes are made by the pass, preserve all analyses. Otherwise preserve the two only.

Preserve TargetLibraryAnalysis and AssumptionAnalysis in all cases. If no changes are made by the pass, preserve all analyses. Otherwise preserve the two only.

there is no need to preserve those two, they are always preserved (see the invalidate method) even if you don't preserve all analyses

gandhi21299 marked an inline comment as done.May 24 2023, 11:14 AM
  • Revert back to rev-1

you could keep track of if you've made CFG changes, but this still lgtm

I only found one place in the code where there may be structural changes in the CFG.

aeubanks added inline comments.May 25 2023, 2:20 PM
llvm/lib/Target/AMDGPU/AMDGPUCodeGenPrepare.cpp
1902

!FlowChanged?

gandhi21299 marked an inline comment as done.May 25 2023, 2:52 PM
This revision was automatically updated to reflect the committed changes.