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

Duncan P. N. Exon Smith via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Dec 10 22:12:39 PST 2022


dexonsmith added a comment.

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.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139767



More information about the llvm-commits mailing list