Page MenuHomePhabricator

[GlobalISel] Create an opt-in unified GlobalISel + SelectionDAG pipeline.
Needs ReviewPublic

Authored by aemerson on May 12 2020, 12:14 AM.

Details

Summary

In order to enable LTO with GlobalISel, we need to be able to select between
GlobalISel and SDISel via some mechanism in the bitcode. Usually these kinds
of codegen options are done with function attributes and passes will inspect
these at link-time compilation to decide what to do. However, when that decision
involves changing the pass pipeline itself this model breaks down. We can't
change the pass pipeline once it's been set up on a per-function basis.

This patch adds a unified ISel pipeline that's opt-in for targets, and enabled
for AArch64 only at the moment. It basically just schedules the GlobalISel
passes before SelectionDAG selection, much like the fallback mechanism. However,
it uses a different query to decide whether or not to run the GlobalISel passes
at all (rather than the FailedISel MF property).

Each GlobalISel pass needs to be taught to *not* run unless the TargetMachine
indicates that GlobalISel was enabled, or if there's a "use-globalisel" function
attribute present.

There are a few things that break when doing this.

  • MachineDominator construction seems to crash when it's given a function which

doesn't have any blocks, which happens when it tries to run right after the
IRTranslator which bails out immediately. This is worked around by just creating
an empty block temporarily.

  • llc's handy -run-pass testing option breaks unless we somehow know in the pass

that's being run that we have been specifically requested. Some module metadata
is added to communicate to the passes in question that they need to run.

Diff Detail

Event Timeline

aemerson created this revision.May 12 2020, 12:14 AM
ychen added a subscriber: ychen.May 12 2020, 8:44 AM
paquette added inline comments.May 12 2020, 11:16 AM
llvm/lib/CodeGen/ResetMachineFunctionPass.cpp
78–81

delete commented out code?

llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
412–413

This comment makes it sound like this doesn't work at all?

If GISel skips a function, then SelectionDAG will just not select it?

llvm/lib/CodeGen/TargetPassConfig.cpp
1349–1352

maybe more succinct

if (!MF.getTarget().getGlobalISel() && !F.hasFnAttribute("use-globalisel"))
  return true;
arsenm added a subscriber: arsenm.May 12 2020, 11:23 AM

Why is this mechanism necessary? I think it's a bit much to change the selector for LTO per module/function as it's not really part of the program semantics, and therefore shouldn't be in the IR

aemerson marked 2 inline comments as done.May 12 2020, 3:52 PM

Why is this mechanism necessary? I think it's a bit much to change the selector for LTO per module/function as it's not really part of the program semantics, and therefore shouldn't be in the IR

As I replied to James on the dev list:

I think being able to select per function would actually be a nice feature for debugging and bug reducing purposes. The real reason I tried to not go for a linker flag was because it would be an exceptional case just for GlobalISel. No other codegen option seems to rely on a linker flag (maybe I’m wrong here?). Passing it via bitcode just seemed the Right Thing To Do, even if it was a pain to implement.

If the consensus is that a one off linker flag for GlobalISel is the right way to go, then that’s ok with me.

llvm/lib/CodeGen/ResetMachineFunctionPass.cpp
78–81

Yep.

llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
412–413

The comment is obsolete so should be disregarded.