<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Fri, May 18, 2018 at 10:26 PM Chris Lattner via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
<br>
> On May 17, 2018, at 2:03 AM, Chandler Carruth via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br>
> <br>
> Going to keep this RFC short and to the point:<br>
> <br>
> 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.<br>
> <br>
> 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.<br>
> <br>
> 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.<br>
> <br>
> Then I propose we restructure the type system to allow CallInst and InvokeInst to easily share logic:<br>
> - Create `CallBase` which is an *abstract* class derived from Instruction that provides all of the common call logic<br>
> - Make `CallInst` derive from this<br>
> - Make `InvokeInst` derive from this, extend it for EH aspects and successors<br>
> - Remove `CallSite` and all accompanying code, rewriting it to use `CallBase`.<br>
> <br>
> 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”.<br>
<br>
+1, this seems like a great direction.  Random question, which we might have talked about but I forgot: can we just eliminate CallBase and InvokeInst entirely?  Why not make “isTerminator()” for calls return true if the call has a normal/unwind destination set?<br></blockquote><div><br></div><div>Eventually, this might make sense, but I'm not currently certain.</div><div><br></div><div>I think storing stuff like successors is useful to model as a separate type to start. Once we get there, I think it'll be easier to see if the tradeoff isn't actually worth it and we can just make this a a property of calls.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
-Chris<br>
<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>
</blockquote></div></div>