rjmccall added inline comments.
================
Comment at: lib/AST/MicrosoftMangle.cpp:448
mangleVariableEncoding(VD);
- else
+ else if (!isa<ObjCInterfaceDecl>(D))
llvm_unreachable("Tried to mangle unexpected NamedDecl!");
----------------
I don't think we want `ObjCInterfaceDecl`s to enter this function normally; I'd
prefer to leave that assertion intact.
================
Comment at: lib/AST/MicrosoftMangle.cpp:2574
mangleTagTypeKind(TTK_Struct);
- mangleName(T->getDecl());
+ mangle(T->getDecl(), ".objc_cls_");
}
----------------
You're not really reusing any interesting logic from `mangle` here; please just
stream the literal directly into `Out`.
Also, this will add the actual identifier to the substitution set, whereas your
implementation below in the case for ObjCObjectType adds the prefixed
identifier to the substitution set.
================
Comment at: lib/CodeGen/CGBlocks.cpp:1262
+ if (IsWindows) {
+ auto *Init = llvm::Function::Create(llvm::FunctionType::get(CGM.VoidTy,
+ {}), llvm::GlobalValue::InternalLinkage, ".block_isa_init",
----------------
theraven wrote:
> DHowett-MSFT wrote:
> > Is there value in emitting a list of blocks that need to be initialized,
> > then initializing them in one go in a COMDAT-foldable function?
> I think that the best solution is to move this into the back end, so that
> this code goes away in the front end, but anything that's referring to a
> dllimport global in a global initialiser is transformed in the back end to a
> list of initialisations and a comdat function that walks the list and sets
> them up. That said, this seems sufficiently generally useful that it would
> be nice for the function to be in the CRT bits.
>
>
> I should be in Redmond some time in October, so maybe we can discuss it with
> some of the VS team then?
Can the blocks part of this patch be split out? It's basically a totally
different bugfix.
================
Comment at: lib/CodeGen/CGBlocks.cpp:1216
bool IsOpenCL = CGM.getLangOpts().OpenCL;
+ bool IsWindows = CGM.getTarget().getTriple().isOSWindows();
if (!IsOpenCL) {
----------------
Should this be a PE/COFF-specific restriction? Or otherwise somehow restricted
to the "native" environment? I don't know enough about all the UNIX-on-Windows
approaches to know whether the same restriction would apply to all of them. I
did think they generally used the Itanium C++ ABI, and the Itanium ABI relies
on linking v-tables from the C++ standard library. Maybe those environments
just all use static linking to get around that.
================
Comment at: lib/CodeGen/CGBlocks.cpp:1276
+ InitVar->setSection(".CRT$XCLa");
+ CGM.addUsedGlobal(InitVar);
+ }
----------------
Is the priority system not good enough?
================
Comment at: lib/CodeGen/CGObjCGNU.cpp:915
+ return name;
+ }
/// The GCC ABI superclass message lookup function. Takes a pointer to a
----------------
Can this section-names cleanup also just be in a separate patch?
================
Comment at: lib/CodeGen/CGObjCGNU.cpp:2262
llvm::Constant *CGObjCGNUstep::GetEHType(QualType T) {
+ if (CGM.getContext().getTargetInfo().getTriple().isWindowsMSVCEnvironment())
+ return CGM.getCXXABI().getAddrOfRTTIDescriptor(T);
----------------
I think this is the third different way that you test for Windows in this
patch. :) Is there a reason why this is just testing for MSVC and the others
are testing for PE/COFF or for Windows generally?
================
Comment at: lib/CodeGen/CGObjCGNU.cpp:3817
+ if (isRethrow && CGF.CGM.getTarget().getTriple().isWindowsMSVCEnvironment())
{
+ CGF.EmitRuntimeCallOrInvoke(ExceptionReThrowFn).setDoesNotReturn();
+ }
----------------
You're sure here that the static information aligns with the dynamic?
================
Comment at: lib/CodeGen/CGObjCRuntime.cpp:125
llvm::Constant *TypeInfo;
+ unsigned Flags;
};
----------------
Please add some sort of comment about the meaning and source of these flags.
================
Comment at: lib/CodeGen/EHScopeStack.h:424
+ void Emit(CodeGenFunction &CGF, Flags F) override;
+};
+
----------------
Please add a function on SGF to push this cleanup or something instead of
globally declaring it.
Repository:
rC Clang
https://reviews.llvm.org/D50144
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
