[PATCH] D158147: [RFC][GlobalISel] convergence control tokens and intrinsics

[Sameer Sahasrabuddhe via Phabricator via llvm-commits]

sameerds marked 2 inline comments as done and 2 inline comments as done.
sameerds added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp:2462
+    auto *Token = Bundle->Inputs[0].get();
+    ConvergenceCtrlToken = getOrCreateVReg(*Token);
+  }
----------------
arsenm wrote:
> Just directly create the token vreg 
I didn't understand. In the LLVM IR, this is a token use, and the token definition was elsewhere. We still need to find that existing register, right?


================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:122
 
-  if (isa<GIntrinsic>(MI))
+  if (auto *GI = dyn_cast<GIntrinsic>(&MI)) {
+    auto ID = GI->getIntrinsicID();
----------------
arsenm wrote:
> Wait, why is this here? Generic intrinsics shouldn't reach the legalizer, they should go through G_* instruction wrappers. If your intention was to default legalize to erase, those should be just in the default lower action
I don't understand, mostly because I am quite unfamiliar with the backend. But in GlobalISel.cpp, the legalizer comes before selection. So this definitely hits the legalizer? By "default lower action", you mean somewhere in `InstructionSelect::runOnMachineFunction()` before it calls the target-specific virtual function `InstructionSelector::select()`?


================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:133
+      Register Token = MI.defs().begin()->getReg();
+      for (auto &Use : MRI.use_operands(Token)) {
+        auto *UserInstr = Use.getParent();
----------------
arsenm wrote:
> Verifier should reject debug uses of tokens
The verifier (see the new patch) rejects any uses other than a call or an intrinsic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158147