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

[Raoul Gough via Phabricator via llvm-commits]

drti added inline comments.


================
Comment at: llvm/include/llvm/Target/TargetMachine.h:429
+  /// Provide safe downcast
+  LLVMTargetMachine *asLLVMTargetMachine() override { return this; }
+
----------------
arsenm wrote:
> I don't understand the point of this, there's no cast here
This overrides the baseclass version of the function which returns nullptr. I think ideally we'd have proper dyn_cast<LLVMTargetMachine> support but the TargetMachine base class isn't set up for this. The virtual function here is just an easy way to get safer downcasting rather than client code using an unchecked static_cast. @ychen was also questioning the utility of this, by the way, suggesting to use the static_cast like some existing code already does. What do you think?


================
Comment at: llvm/lib/CodeGen/TargetPassConfig.cpp:746
+
+void TargetPassConfig::removeExtension(ExtensionID ID) {
+  if (!GlobalExtensions.isConstructed())
----------------
arsenm wrote:
> Is removal ever really needed?
I guess the only situation where it's useful is if shared objects/DLLs are dynamically unloaded - any registered extension would crash if not removed before unloading its code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98591