[PATCH] D44095: [ObjC] Allow declaring __weak pointer fields in C structs in ObjC-ARC

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added inline comments.


================
Comment at: include/clang/AST/Decl.h:3631
+    PassedIndirectly = true;
+  }
+
----------------
rjmccall wrote:
> I feel like this flag should be set by Sema for C++ types that have to be passed indirectly as well; it can then become the single point of truth for that information.
I moved CXXRecordDecl::CanPassInRegisters to RecordDecl along with its setter and getter functions.


================
Comment at: lib/CodeGen/CGNonTrivialStruct.cpp:764
+    Object = CGF->EmitObjCConsumeObject(QT, Object);
+    CGF->EmitARCStoreWeak(Addrs[DstIdx], Object, true);
+  }
----------------
rjmccall wrote:
> I guess this is the most reasonable thing to do, given that we don't have an entrypoint for it.  Please ask the ObjC runtime team to consider giving us one, though.  We could pretty easily peephole assignments into `__weak` variables where the source is loaded from a `__weak` l-value / x-value, and Swift would probably be able to take advantage of it, too.
> 
> You might want to go ahead and add `emitARCCopyAssignWeak` / `emitARCMoveAssignWeak` methods on CGF that do these operations and which can be optimized to use those entrypoints if/when they're added.
Do you mean we should ask for the following objc runtime functions and use them in visitARCWeak?

```
// dst and src are either null or registered as __weak objects.
void objc_copyAssignWeak(id *dst, id *src)
void objc_moveAssignWeak(id *dst, id *src)
````


================
Comment at: lib/CodeGen/TargetInfo.cpp:1326
+  if (RetTy.isPassedIndirectly() == QualType::APK_Struct)
+    return getNaturalAlignIndirect(RetTy);
+
----------------
rjmccall wrote:
> Can we combine this into a single function that covers both the C++ case and this new one?  Otherwise it seems likely that individual targets over time will only add one or the other.
I added another definition of classifyReturnType and taught getRecordArgABI to detect non-trivial C structs. Currently, x86, arm, and arm64 are the only targets that use the new overload of classifyReturnType. I can modify the other targets to use it too if that is desirable.


https://reviews.llvm.org/D44095