[PATCH] D98591: [CodeGen] Add extension points for TargetPassConfig::addMachinePasses

[Raoul Gough via Phabricator via llvm-commits]

drti added a comment.

Thanks very much @ychen for looking at my review, and thanks for pointing me at the examples/Bye/Bye.cpp example code. I didn't know we could do that kind of thing in the build and I'll post an update with a new example later this week.

I see your other two comments as being closely related to each other. If we handle target filtering in the TargetPassConfig implementation then it makes sense for the test to confirm that the filtering works, i.e. an extension registered for one target is not mistakenly invoked for a different one. If we put the responsibility on the client code to filter out unwanted targets then the test wouldn't have to care and any target machine (so long as it can be cast to LLVMTargetMachine, of course) would do.

I can see one benefit to doing the filtering in the implementation rather than the client - in makes it obvious that the filtering is necessary, i.e. the API forces the client code to select a specific target. Otherwise the client code could omit the filtering and then it would work during developer testing say, but crash mysteriously if invoked on an unsuitable target. Maybe that's unlikely, so please let me know if you'd prefer me to simpilfy the implementation by removing the filtering.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98591