<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Thu, May 17, 2018 at 10:32 AM Xinliang David Li <<a href="mailto:xinliangli@gmail.com">xinliangli@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, May 17, 2018 at 2:03 AM, Chandler Carruth via llvm-dev <span dir="ltr"><<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Going to keep this RFC short and to the point:<div><br></div><div>TerminatorInst doesn't pull its weight in the type system. There is essentially a single relevant API -- iterating successors. There is no other interesting aspect shared -- the interface itself just dispatches to specific instructions to be implemented.</div><div><br></div><div>On the flip side, CallInst and InvokeInst have *massive* amounts of code shared and struggle to be effective due to being unable to share a base class in the type system. We have CallSite and a host of other complexity trying to cope with this, and honestly, it isn't doing such a great job.</div><div><br></div><div>I propose we make "terminator-ness" a *property* of an instruction and take it out of the type system. We can build a handful of APIs to dispatch between instructions with this property and expose successors. This should be really comparable to the existing code and have nearly no down sides.</div><div><br></div><div>Then I propose we restructure the type system to allow CallInst and InvokeInst to easily share logic:</div><div>- Create `CallBase` which is an *abstract* class derived from Instruction that provides all of the common call logic</div><div>- Make `CallInst` derive from this</div><div>- Make `InvokeInst` derive from this, extend it for EH aspects and successors</div><div>- Remove `CallSite` and all accompanying code, rewriting it to use `CallBase`.</div><div><br></div></div></blockquote><div><br></div><div>Agree that this is area that needs cleanup.</div><div><br></div><div>How about simplifying the type system even more -- Make CallInst the base class, and let InvokeInst Derive from it (i.e, no need for CallBase).</div></div></div></div></blockquote><div><br></div><div>It is important to be able to easily identify a non-invoke call. Writing: "isa<CallInst>(...) && !isa<InvokeInst>(...)" would be really painful and is a sign that this isn't the right model IMO.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></div><div>David</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div></div><div>The end result will, IMO, be a much simpler IR type system and implementation. The code interacting with instructions should also be much more consistent and clear w/o the awkward CallSite "abstraction".</div><div><br></div><div>Thoughts? Seem OK at a high level?</div><div><br></div><div>Happy to bikeshed the name `CallBase`, but I've discussed this with several folks, including Reid and Chris and nothing better came up really. `CallSite` might be nicer, but the confusion with the *existing* type seems much more problematic.</div><div><br></div><div><br></div><div>Assuming folks are happy with this direction, are there any incremental patches that folks would like to see in pre-commit review? I've only done some initial investigation of what it takes to cut this through. Provided folks are positive about the direction, I'll work on what this would actually look like in practice.</div><span class="m_-2039583033396046869HOEnZb"><font color="#888888"><div><br></div><div>-Chandler</div></font></span></div>
<br>_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
<br></blockquote></div><br></div></div>
</blockquote></div></div>