Page MenuHomePhabricator

[test][Attributor] Remove legacy PM RUN lines
Needs ReviewPublic

Authored by aeubanks on Oct 20 2021, 2:24 PM.

Diff Detail

Event Timeline

aeubanks created this revision.Oct 20 2021, 2:24 PM
aeubanks published this revision for review.Oct 20 2021, 2:56 PM
Herald added a reviewer: baziotis. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript

I'm a bit worried about this as the backend still used the old pm, right? The backend also uses the Attributor now, so we should test it.

I'm a bit worried about this as the backend still used the old pm, right? The backend also uses the Attributor now, so we should test it.

yeah codegen still uses the legacy PM
where are we using the Attributor in the codegen pipeline?

createAttributorLegacyPass() and createAttributorCGSCCLegacyPass() are only used in PassManagerBuilder, which is the legacy PM's optimization pipeline (not codegen pipeline), which is deprecated

createAttributorLegacyPass() and createAttributorCGSCCLegacyPass() are only used in PassManagerBuilder, which is the legacy PM's optimization pipeline (not codegen pipeline), which is deprecated

AMDGPU backend creates an Attributor instance explicitly: llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp

createAttributorLegacyPass() and createAttributorCGSCCLegacyPass() are only used in PassManagerBuilder, which is the legacy PM's optimization pipeline (not codegen pipeline), which is deprecated

AMDGPU backend creates an Attributor instance explicitly: llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp

Ah I see. Would it make sense to move this into the optimization pipeline? e.g. add it into AMDGPUTargetMachine::registerPassBuilderCallbacks(), probably as a PB.registerOptimizerLastEPCallback().
+@arsenm

createAttributorLegacyPass() and createAttributorCGSCCLegacyPass() are only used in PassManagerBuilder, which is the legacy PM's optimization pipeline (not codegen pipeline), which is deprecated

AMDGPU backend creates an Attributor instance explicitly: llvm/lib/Target/AMDGPU/AMDGPUAttributor.cpp

Ah I see. Would it make sense to move this into the optimization pipeline? e.g. add it into AMDGPUTargetMachine::registerPassBuilderCallbacks(), probably as a PB.registerOptimizerLastEPCallback().
+@arsenm

Seems like it's not so easy, I tried this and it breaks a bunch of llc tests.
Would it be ok to keep the amdgpu-attributor tests running against the legacy PM and the other attributor tests against the new PM?