This is an archive of the discontinued LLVM Phabricator instance.

[IR] Remove getNumSuccessorsV/getSuccessorV/setSuccessorV from the TerminatorInst subclasses as much as possible now that Value has been de-virtualized
ClosedPublic

Authored by craig.topper on Jun 7 2017, 1:49 PM.

Details

Summary

These used to be virtual methods that would enable doing the right thing with only a TerminatorInst pointer. I believe they were also acting as vtable anchors in my cases. I think the fact that they had a separate name ending in V was to allow a version without V to be called without a virtual call in a pre-C++11 final keyword world.

Now that that the value hierarchy has been de-virtualized we can clean this up.

Where possible the base methods in TerminatorInst dispatch directly to the public methods in the classes that have the same signature. For some classes this wasn't possible so I've left private method versions that match the name and signature of the version in TerminatorInst. All versions have been moved into the class definitions since we no longer need vtable anchors here.

Diff Detail

Event Timeline

craig.topper created this revision.Jun 7 2017, 1:49 PM

Remove "friend class TerminatorInst" from the ones that are able to reuse public methods.

And get all the changes included. Forgot i had a partial commit that I needed to include.

rnk accepted this revision.Jun 7 2017, 1:57 PM

lgtm

lib/IR/Instructions.cpp
90

Yep, this should do the right thing.

This revision is now accepted and ready to land.Jun 7 2017, 1:57 PM

One downfall of this is that a future new TerminatorInst that forgets to implement these will not get a compile error and instead of will get an infinite loop at runtime.

This revision was automatically updated to reflect the committed changes.