[PATCH] D37206: [ItaniumCXXABI] Always use linkonce_odr linkage for RTTI data on MinGW

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added inline comments.


================
Comment at: lib/CodeGen/ItaniumCXXABI.cpp:2990-2993
+      bool IsMinGW =
+          CGM.getContext().getTargetInfo().getTriple().getEnvironment() ==
+              llvm::Triple::GNU &&
+          CGM.getContext().getTargetInfo().getTriple().isOSWindows();
----------------
This is equivalent to: `CGM.getContext().getTargetInfo().getTripleInfo().isWindowsGNUEnvironment()`


================
Comment at: lib/CodeGen/ItaniumCXXABI.cpp:2994
+          CGM.getContext().getTargetInfo().getTriple().isOSWindows();
+      // MinGW always uses LinkOnceODRLinkage for type info.
+      if (RD->isDynamicClass() && !IsMinGW)
----------------
mstorsjo wrote:
> mstorsjo wrote:
> > Thanks for having a look at this!
> > 
> > When giving this a second thought now, should we perhaps skip the check for `Environment == GNU` altogether? Or should `xxx-windows-itanium` be different by restricting this behaviour strictly to GNU (does it matter)?
> > 
> > And given the existing test case that you added with the previous form of this code, and the new test I'm adding here, are there any other odd corner cases I should check?
> To rephrase; previously, the comment talked about mingw specifically, but the code only checked for the dllimport attribute. So perhaps I should drop the check for GNU and just check for windows?
Im not 100% certain off the top of my head, but I believe that `windows-itanium` should actually import the typename, so this should be okay.


https://reviews.llvm.org/D37206