[PATCH] D139861: [llvm] boilerplate for new callbrprepare codegen IR pass

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers added inline comments.


================
Comment at: llvm/lib/CodeGen/CallBrPrepare.cpp:9-17
+// This pass lowers callbrs in LLVM IR to something closer that the backend can
+// perform correct instruction selection for.  In particular, callbr may have
+// inline asm with output constraints that require specific physical registers.
+// These physical register will need to be copied back to virtual registers
+// somewhere. If we have critical edges, we may need to split them. Regardless
+// of critical edges, we create distinct new SSA values for each indirect
+// destination of the callbr, then remap users of the direct destinations value
----------------
nickdesaulniers wrote:
> jyknight wrote:
> > void wrote:
> > > jyknight wrote:
> > > > I think this description needs to be substantially edited for clarity.
> > > > 
> > > > A few points on substance: 
> > > > - the problem is not unique to physical register outputs, so mentioning that here is just misleading.
> > > > - Worth noting that this is primarily to make things easy for SelectionDAG, and in GlobalISel we may be able to lower directly without the IR pass.
> > > s/closer//
> > > [...] GlobalISel we may be able to lower directly [...]
> > 
> > Or, maybe not...since we probably still need to split critical edges, just not insert a new intrinsic? Dunno.
> I don't really have any experience with GlobalISel to provide colorful commentary on what it can or cannot do.  For instance, I don't think it can even select inline asm, so callbrprepare is kind of a non-starter until it has at least that support.
> 
> What are your thoughts on how I can reword this for clarity?  I agree I can drop "physical" from descriptions about "physical registers."  Anything else? I'm happy to replace the entire thing outright, if you have suggestions?
Updated to remove mention of `specific physical` registers in favor of just registers.  If you have further thoughts on how best to reword this, please reopen the thread with proposed wording. Marking as done for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139861