[PATCH] D79769: [GlobalISel] Create an opt-in unified GlobalISel + SelectionDAG pipeline.

[Amara Emerson via Phabricator via llvm-commits]

aemerson marked 2 inline comments as done.
aemerson added a comment.

In D79769#2032002 <https://reviews.llvm.org/D79769#2032002>, @arsenm wrote:

> 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.





================
Comment at: llvm/lib/CodeGen/ResetMachineFunctionPass.cpp:78-81
+        /*        using MFP = MachineFunctionProperties::Property;
+        MF.getProperties().reset(MFP::Legalized);
+        MF.getProperties().reset(MFP::RegBankSelected);
+        MF.getProperties().reset(MFP::Selected);*/
----------------
paquette wrote:
> delete commented out code?
Yep.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp:412-413
   // If we already selected that function, we do not need to run SDISel.
+  // FIXME: GlobalISel passes are scheduled before SelectionDAGISel, and
+  // they will set the Selected property even if they are being skipped.
   if (mf.getProperties().hasProperty(
----------------
paquette wrote:
> This comment makes it sound like this doesn't work at all?
> 
> If GISel skips a function, then SelectionDAG will just not select it?
The comment is obsolete so should be disregarded.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79769/new/

https://reviews.llvm.org/D79769