[PATCH] D82673: [ModuloSchedule] Make PeelingModuloScheduleExpander inheritable.

Wed Jul 1 10:48:46 PDT 2020

hgreving added a comment.

In D82673#2125936 <https://reviews.llvm.org/D82673#2125936>, @dblaikie wrote:

> Adding an extension point with no testing inside LLVM seems a bit problematic - is there no existing target that could use this? Or a reasonable scale of unit test that could exercise this extension point without a full-blown target?
>
> (also this produced a warning about the fact that this type doesn't have a virtual dtor - was fixed by @jfb in c7586444ca787c3845ac4ad0bd603709f2abbb0f <https://reviews.llvm.org/rGc7586444ca787c3845ac4ad0bd603709f2abbb0f> )

The change is basically NFC. It was made in order to allow us using the upstream pass by inheriting from it. I'm not sure what test you would write, are you thinking about a test that tests protected vs. private? Heaxgon is the only upstream target upstream using the modulo scheduler. Full disclosure, we're a bit short staffed and are not in a position writing Hexagon tests. We _would_ like to use the upstream pass and when the time comes, add our improvements to LLVM upstream. Making the peeling modulo scheduler inheritable is a reasonable interface change, because different subtargets have different peeling requirements while sharing common semantics. The change simply allowed for the entire interface to be inheritable. Reality is, if the change is unwanted, we would end up downstreaming the pass entirely and developement on the upstream pass would stop - for now. If you're ok with the change, I think it's beneficial to everybody at this point. LMK, thanks for considering

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82673