This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Make visibility of overridden methods in RISCVISelLowering match the parent
ClosedPublic

Authored by asb on May 14 2020, 2:13 AM.

Details

Summary

Currently, some fairly arbitrary subset of overriden methods in RISCVISelLowering are private rather than public (which is the visibility they have in TargetLowering). I suspect this is a holdover from too closely copying another backend.

I don't see a good reason for having different visibility in the child vs the parent (though I could be missing something?), and D78545 pointed out this can be difficult for some downstream patches. This patch solves the wider problem by simply making all overridden methods match the public visiblity of the parent.

Diff Detail

Event Timeline

asb created this revision.May 14 2020, 2:13 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 14 2020, 2:13 AM
lenary accepted this revision.May 14 2020, 4:21 AM

I am in favour of this reasoning around matching the visibility - it serves downstream users best (in general), and there's no good reason not to allow people to subclass these classes for their own usage (though I don't know how much this happens in reality).

This revision is now accepted and ready to land.May 14 2020, 4:21 AM
This revision was automatically updated to reflect the committed changes.