[llvm-dev] RFC: Removing TerminatorInst, simplifying calls

[Xinliang David Li via llvm-dev]

On Thu, May 17, 2018 at 1:24 PM, Chandler Carruth <chandlerc at gmail.com>
wrote:

> On Thu, May 17, 2018 at 10:32 AM Xinliang David Li <xinliangli at gmail.com>
> wrote:
>
>>
>>
>> On Thu, May 17, 2018 at 2:03 AM, Chandler Carruth via llvm-dev <
>> llvm-dev at lists.llvm.org> wrote:
>>
>>> Going to keep this RFC short and to the point:
>>>
>>> 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.
>>>
>>> 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.
>>>
>>> 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.
>>>
>>> Then I propose we restructure the type system to allow CallInst and
>>> InvokeInst to easily share logic:
>>> - Create `CallBase` which is an *abstract* class derived from
>>> Instruction that provides all of the common call logic
>>> - Make `CallInst` derive from this
>>> - Make `InvokeInst` derive from this, extend it for EH aspects and
>>> successors
>>> - Remove `CallSite` and all accompanying code, rewriting it to use
>>> `CallBase`.
>>>
>>>
>> Agree that this is area that needs cleanup.
>>
>> 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).
>>
>
> 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.
>
>


You have a point here, the question is how often this kind of query really
happens?  Conceptually, it is more often to see these two patterns: 1)
common logic which only checks isa<CallInst> and 2) Invoke Specific logic
which checks isa<InvokeInst>

David


>
>> David
>>
>>
>>
>>> 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".
>>>
>>> Thoughts? Seem OK at a high level?
>>>
>>> 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.
>>>
>>>
>>> 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.
>>>
>>> -Chandler
>>>
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> llvm-dev at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>>
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180517/de8f7c03/attachment-0001.html>