[PATCH] D53765: [RFC prototype] Implementation of asm-goto support in LLVM

[Craig Topper via Phabricator via llvm-commits]

craig.topper marked 2 inline comments as done.
craig.topper added inline comments.


================
Comment at: lib/CodeGen/GlobalISel/IRTranslator.cpp:1246
+
+  // FIXME: support whatever these are.
+  if (I.countOperandBundlesOfType(LLVMContext::OB_deopt))
----------------
chandlerc wrote:
> chandlerc wrote:
> > craig.topper wrote:
> > > void wrote:
> > > > "whether these are" what? :-)
> > > No I think the person who wrote this was throwing their hands up and saying they don't what that is.
> > Maybe have the verifier reject if we can't lower here? Regardless, the comment still seems to need some amount of fixing.
> > 
> > `deopt` bundles are a reasonable thing, and just not really a reasonable thing (at this stage) to support on callbr.
> (still seems to need update)
Turns out this comment was copied directly from the InvokeInst handler above.


================
Comment at: lib/CodeGen/GlobalISel/IRTranslator.cpp:1300
+
+  // FIXME: track probabilities.
+  MachineBasicBlock &ReturnMBB = getMBB(*ReturnBB);
----------------
chandlerc wrote:
> craig.topper wrote:
> > chandlerc wrote:
> > > This complex to thread through?
> > Original author copied that comment out of visitInvokeInst
> (comment remains)
Does global isel do anything with probabilities anywhere else? I can't see any code obviously doing it. SelectionDAGBuilder has some helper functions that we used for CallBr, but I don't see equivalent helpers here or any calls passing probability to addSuccessor.


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

https://reviews.llvm.org/D53765