Page MenuHomePhabricator

[AArch64][GlobalISel] Enable use of the optsize predicate in the selector.
ClosedPublic

Authored by aemerson on Mar 1 2021, 1:51 PM.

Details

Summary

To do this while supporting the existing functionality in SelectionDAG of using PGO info, we add the ProfileSummaryInfo and LazyBlockFrequencyInfo analysis dependencies to the instruction selector pass.

Then, use the predicate to generate constant pool loads for f32 materialization, if we're targeting optsize/minsize.

Diff Detail

Event Timeline

aemerson created this revision.Mar 1 2021, 1:51 PM
aemerson requested review of this revision.Mar 1 2021, 1:51 PM
arsenm added inline comments.Mar 1 2021, 1:58 PM
llvm/include/llvm/CodeGen/GlobalISel/InstructionSelector.h
480

Why would CurMBB ever be null?

paquette added inline comments.Mar 1 2021, 1:58 PM
llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
2438

I would expect shouldOptForSize to return true when minsize is present and optsize is not present.

That case seems pretty atypical though. At least with clang, they always appear together.

aemerson added inline comments.Mar 1 2021, 2:22 PM
llvm/include/llvm/CodeGen/GlobalISel/InstructionSelector.h
480

Because these predicates get called once by the emitter-generated code before we actually start selecting a function.

llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
2438

I didn't realize minsize and optsize could co-exist. Yeah I guess it makes sense to move this logic.

paquette added inline comments.Mar 1 2021, 2:24 PM
llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
2438

Also, why the MachineFunction variant versus the MachineBasicBlock variant? Is there a difference in size if you use the MBB variant?

aemerson added inline comments.Mar 1 2021, 2:33 PM
llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp
2438

You only want to directly use the MBB variant if you know you have a non-null PSI and BFI available (because the MBB one is for using PGO info). Otherwise, you use the MF variant which delegates to the MBB one if it's available.

aemerson updated this revision to Diff 327294.Mar 1 2021, 2:34 PM

Move minsize check into shouldOptForSize() functions.

This comment was removed by paquette.
This revision is now accepted and ready to land.Mar 2 2021, 10:20 AM
This revision was landed with ongoing or failed builds.Mar 2 2021, 12:57 PM
This revision was automatically updated to reflect the committed changes.