[PATCH] D139767: [DFAPacketizer] Move DefaultVLIWScheduler class declaration to header file

[Darshan Bhat via Phabricator via llvm-commits]

DarshanRamakant added a comment.

In D139767#3986749 <https://reviews.llvm.org/D139767#3986749>, @dexonsmith wrote:

> In D139767#3986728 <https://reviews.llvm.org/D139767#3986728>, @DarshanRamakant wrote:
>
>> In D139767#3986476 <https://reviews.llvm.org/D139767#3986476>, @dexonsmith wrote:
>>
>>> I don't know much about this code; resigning as reviewer. But I took a quick look. I wonder if this the right fix, or whether `VLIWPacketizerList::VLIWScheduler`'s type should be `ScheduleDAGInstrs`, and downcast when necessary on use, allowing `DefaultVLIWScheduler` to stay private. (Up to others to sort out!)
>>
>> Thanks @dexonsmith for the suggestion. But I think even if we want to downcast ( dynamic_cast) the declaration should be available to the derived class, which is not the case in the current implementation.
>
> Not sure I'm following, maybe I've misread something, but it seems like:
>
> - Your example (HexagonPacketizerList) could just call `ScheduleDAGInstrs::schedule()`, which is a public method. No need to downcast.
> - Within llvm/lib/CodeGen/DFAPacketizer.cpp, code that needs to call `addMutation()` might need to downcast. This would be a `static_cast` though, since the implementation knows the type. (Note: LLVM builds without RTTI, so `dynamic_cast` never works. If you don't know what type to downcast to, you need to save type info explicitly somehow in the base class.)

My idea was to make "DefaultVLIWScheduler" class type available to derived classes of "VLIWPacketizerList" so that even "addMutatation()" can be used in derived classes. But I also ok with changing the member type to base class. Waiting for other reviewers comments.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139767/new/

https://reviews.llvm.org/D139767