This is an archive of the discontinued LLVM Phabricator instance.

GlobalISel: Pass MachineIRBuilder to applyMappingImpl
ClosedPublic

Authored by arsenm on Jul 27 2023, 12:49 PM.

Details

Summary

The target should not have to construct MachineIRBuilders during
RegBankSelect (we should perhaps hide the constructors for it). The
pass should own the builder setup with the desired CSE configuration
(although currently the pass does not use the CSE builder, which is
what I want to fix).

Diff Detail

Event Timeline

arsenm created this revision.Jul 27 2023, 12:49 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 27 2023, 12:49 PM
arsenm requested review of this revision.Jul 27 2023, 12:49 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 27 2023, 12:49 PM
Herald added a subscriber: wdng. · View Herald Transcript
Pierre-vh added inline comments.Jul 28 2023, 1:47 AM
llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.h
56–58

Could we make the builder a member of the class?

arsenm added inline comments.Jul 28 2023, 4:14 AM
llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.h
56–58

This is owned by the subtarget and shouldn’t be stateful. All the methods should be const

arsenm added inline comments.Jul 28 2023, 4:16 AM
llvm/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h
25 ↗(On Diff #544895)

This include shouldn't be necessary

arsenm updated this revision to Diff 545096.Jul 28 2023, 4:21 AM

Drop include

Pierre-vh accepted this revision.Jul 31 2023, 12:39 AM
This revision is now accepted and ready to land.Jul 31 2023, 12:39 AM