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

2018-03-19 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment.

This was reverted in r327294 because it caused module-enabled builders to fail 
after I moved CXXRecordDecl::CanPassInRegisters to RecordDecl. I'm recommitting 
this patch with fixes to ASTDeclReader and ASTWriter.


Repository:
  rC Clang

https://reviews.llvm.org/D44095



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2018-03-09 Thread Akira Hatanaka via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC327206: [ObjC] Allow declaring __weak pointer fields in C 
structs in ARC. (authored by ahatanak, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D44095

Files:
  include/clang/AST/Decl.h
  include/clang/AST/DeclCXX.h
  include/clang/AST/Type.h
  lib/AST/ASTImporter.cpp
  lib/AST/Decl.cpp
  lib/AST/DeclCXX.cpp
  lib/AST/Type.cpp
  lib/CodeGen/CGBlocks.cpp
  lib/CodeGen/CGNonTrivialStruct.cpp
  lib/CodeGen/CGObjC.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/TargetInfo.cpp
  lib/Sema/SemaDecl.cpp
  lib/Serialization/ASTReaderDecl.cpp
  lib/Serialization/ASTWriter.cpp
  lib/Serialization/ASTWriterDecl.cpp
  test/CodeGenObjC/nontrivial-c-struct-exception.m
  test/CodeGenObjC/weak-in-c-struct.m

Index: include/clang/AST/Type.h
===
--- include/clang/AST/Type.h
+++ include/clang/AST/Type.h
@@ -1097,6 +1097,10 @@
 /// with the ARC __strong qualifier.
 PDIK_ARCStrong,
 
+/// The type is an Objective-C retainable pointer type that is qualified
+/// with the ARC __weak qualifier.
+PDIK_ARCWeak,
+
 /// The type is a struct containing a field whose type is not PCK_Trivial.
 PDIK_Struct
   };
@@ -1124,6 +1128,10 @@
 /// with the ARC __strong qualifier.
 PCK_ARCStrong,
 
+/// The type is an Objective-C retainable pointer type that is qualified
+/// with the ARC __weak qualifier.
+PCK_ARCWeak,
+
 /// The type is a struct containing a field whose type is neither
 /// PCK_Trivial nor PCK_VolatileTrivial.
 /// Note that a C++ struct type does not necessarily match this; C++ copying
@@ -1146,6 +1154,8 @@
   /// source object is placed in an uninitialized state.
   PrimitiveCopyKind isNonTrivialToPrimitiveDestructiveMove() const;
 
+  bool canPassInRegisters() const;
+
   enum DestructionKind {
 DK_none,
 DK_cxx_destructor,
Index: include/clang/AST/Decl.h
===
--- include/clang/AST/Decl.h
+++ include/clang/AST/Decl.h
@@ -3553,6 +3553,12 @@
   bool NonTrivialToPrimitiveCopy : 1;
   bool NonTrivialToPrimitiveDestroy : 1;
 
+  /// True if this class can be passed in a non-address-preserving fashion
+  /// (such as in registers).
+  /// This does not imply anything about how the ABI in use will actually
+  /// pass an object of this class.
+  bool CanPassInRegisters : 1;
+
 protected:
   RecordDecl(Kind DK, TagKind TK, const ASTContext &C, DeclContext *DC,
  SourceLocation StartLoc, SourceLocation IdLoc,
@@ -3636,6 +3642,18 @@
 NonTrivialToPrimitiveDestroy = true;
   }
 
+  /// Determine whether this class can be passed in registers. In C++ mode,
+  /// it must have at least one trivial, non-deleted copy or move constructor.
+  /// FIXME: This should be set as part of completeDefinition.
+  bool canPassInRegisters() const {
+return CanPassInRegisters;
+  }
+
+  /// Set that we can pass this RecordDecl in registers.
+  void setCanPassInRegisters(bool CanPass) {
+CanPassInRegisters = CanPass;
+  }
+
   /// \brief Determines whether this declaration represents the
   /// injected class name.
   ///
Index: include/clang/AST/DeclCXX.h
===
--- include/clang/AST/DeclCXX.h
+++ include/clang/AST/DeclCXX.h
@@ -467,12 +467,6 @@
 /// constructor.
 unsigned HasDefaultedDefaultConstructor : 1;
 
-/// \brief True if this class can be passed in a non-address-preserving
-/// fashion (such as in registers) according to the C++ language rules.
-/// This does not imply anything about how the ABI in use will actually
-/// pass an object of this class.
-unsigned CanPassInRegisters : 1;
-
 /// \brief True if a defaulted default constructor for this class would
 /// be constexpr.
 unsigned DefaultedDefaultConstructorIsConstexpr : 1;
@@ -1474,18 +1468,6 @@
 return data().HasIrrelevantDestructor;
   }
 
-  /// \brief Determine whether this class has at least one trivial, non-deleted
-  /// copy or move constructor.
-  bool canPassInRegisters() const {
-return data().CanPassInRegisters;
-  }
-
-  /// \brief Set that we can pass this RecordDecl in registers.
-  // FIXME: This should be set as part of completeDefinition.
-  void setCanPassInRegisters(bool CanPass) {
-data().CanPassInRegisters = CanPass;
-  }
-
   /// Determine whether the triviality for the purpose of calls for this class
   /// is overridden to be trivial because this class or the type of one of its
   /// subobjects has attribute "trivial_abi".
Index: test/CodeGenObjC/weak-in-c-struct.m
===
--- test/CodeGenObjC/weak-in-c-struct.m
+++ test/CodeGenObjC/weak-in-c-struct.m
@@ -0,0 +1,193 @@
+// RUN: %clang_cc1 -triple arm64-apple-ios11 -fobjc-arc -fblocks -fobjc-runtime=ios-11.0 -emit-llvm

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

2018-03-09 Thread Akira Hatanaka via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL327206: [ObjC] Allow declaring __weak pointer fields in C 
structs in ARC. (authored by ahatanak, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D44095?vs=137500&id=137900#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D44095

Files:
  cfe/trunk/include/clang/AST/Decl.h
  cfe/trunk/include/clang/AST/DeclCXX.h
  cfe/trunk/include/clang/AST/Type.h
  cfe/trunk/lib/AST/ASTImporter.cpp
  cfe/trunk/lib/AST/Decl.cpp
  cfe/trunk/lib/AST/DeclCXX.cpp
  cfe/trunk/lib/AST/Type.cpp
  cfe/trunk/lib/CodeGen/CGBlocks.cpp
  cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp
  cfe/trunk/lib/CodeGen/CGObjC.cpp
  cfe/trunk/lib/CodeGen/CodeGenFunction.h
  cfe/trunk/lib/CodeGen/TargetInfo.cpp
  cfe/trunk/lib/Sema/SemaDecl.cpp
  cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
  cfe/trunk/lib/Serialization/ASTWriter.cpp
  cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
  cfe/trunk/test/CodeGenObjC/nontrivial-c-struct-exception.m
  cfe/trunk/test/CodeGenObjC/weak-in-c-struct.m

Index: cfe/trunk/lib/Serialization/ASTWriter.cpp
===
--- cfe/trunk/lib/Serialization/ASTWriter.cpp
+++ cfe/trunk/lib/Serialization/ASTWriter.cpp
@@ -6018,7 +6018,6 @@
   Record->push_back(Data.HasIrrelevantDestructor);
   Record->push_back(Data.HasConstexprNonCopyMoveConstructor);
   Record->push_back(Data.HasDefaultedDefaultConstructor);
-  Record->push_back(Data.CanPassInRegisters);
   Record->push_back(Data.DefaultedDefaultConstructorIsConstexpr);
   Record->push_back(Data.HasConstexprDefaultConstructor);
   Record->push_back(Data.HasNonLiteralTypeFieldsOrBases);
Index: cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
===
--- cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
+++ cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
@@ -465,6 +465,7 @@
   Record.push_back(D->isAnonymousStructOrUnion());
   Record.push_back(D->hasObjectMember());
   Record.push_back(D->hasVolatileMember());
+  Record.push_back(D->canPassInRegisters());
 
   if (D->getDeclContext() == D->getLexicalDeclContext() &&
   !D->hasAttrs() &&
@@ -1899,6 +1900,7 @@
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // AnonymousStructUnion
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // hasObjectMember
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // hasVolatileMember
+  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // canPassInRegisters
   // DC
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6));   // LexicalOffset
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6));   // VisibleOffset
Index: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
===
--- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
+++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
@@ -739,6 +739,7 @@
   RD->setAnonymousStructOrUnion(Record.readInt());
   RD->setHasObjectMember(Record.readInt());
   RD->setHasVolatileMember(Record.readInt());
+  RD->setCanPassInRegisters(Record.readInt());
   return Redecl;
 }
 
@@ -1584,7 +1585,6 @@
   Data.HasIrrelevantDestructor = Record.readInt();
   Data.HasConstexprNonCopyMoveConstructor = Record.readInt();
   Data.HasDefaultedDefaultConstructor = Record.readInt();
-  Data.CanPassInRegisters = Record.readInt();
   Data.DefaultedDefaultConstructorIsConstexpr = Record.readInt();
   Data.HasConstexprDefaultConstructor = Record.readInt();
   Data.HasNonLiteralTypeFieldsOrBases = Record.readInt();
@@ -1724,7 +1724,6 @@
   MATCH_FIELD(HasIrrelevantDestructor)
   OR_FIELD(HasConstexprNonCopyMoveConstructor)
   OR_FIELD(HasDefaultedDefaultConstructor)
-  MATCH_FIELD(CanPassInRegisters)
   MATCH_FIELD(DefaultedDefaultConstructorIsConstexpr)
   OR_FIELD(HasConstexprDefaultConstructor)
   MATCH_FIELD(HasNonLiteralTypeFieldsOrBases)
Index: cfe/trunk/lib/CodeGen/TargetInfo.cpp
===
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp
@@ -140,8 +140,11 @@
 static CGCXXABI::RecordArgABI getRecordArgABI(const RecordType *RT,
   CGCXXABI &CXXABI) {
   const CXXRecordDecl *RD = dyn_cast(RT->getDecl());
-  if (!RD)
+  if (!RD) {
+if (!RT->getDecl()->canPassInRegisters())
+  return CGCXXABI::RAA_Indirect;
 return CGCXXABI::RAA_Default;
+  }
   return CXXABI.getRecordArgABI(RD);
 }
 
@@ -153,6 +156,20 @@
   return getRecordArgABI(RT, CXXABI);
 }
 
+static bool classifyReturnType(const CGCXXABI &CXXABI, CGFunctionInfo &FI,
+   const ABIInfo &Info) {
+  QualType Ty = FI.getReturnType();
+
+  if (const auto *RT = Ty->getAs())
+if (!isa(RT->getDecl()) &&
+!RT->getDecl()->canPassInRegisters()) {
+  FI.getReturnInfo() = Info.getNaturalAlignIndirect(Ty);
+ 

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

2018-03-07 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

LGTM, thanks.




Comment at: lib/CodeGen/CGNonTrivialStruct.cpp:764
+Object = CGF->EmitObjCConsumeObject(QT, Object);
+CGF->EmitARCStoreWeak(Addrs[DstIdx], Object, true);
+  }

ahatanak wrote:
> 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)
> 
I meant that we should implement `emitARCCopyAssignWeak` and 
`emitARCMoveAssignWeak` by calling those functions if they're available, yes.

(Obviously that would be a follow-up patch, even assuming the ObjC runtime 
agrees to add them.)

Those functions would be specified as leaving the source in a zeroed state, 
which is as good as uninitialized, so they could be used for true-destructive 
moves as well.


https://reviews.llvm.org/D44095



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2018-03-07 Thread 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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2018-03-07 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak updated this revision to Diff 137500.
ahatanak marked 2 inline comments as done.
ahatanak added a comment.

Address review comments.


https://reviews.llvm.org/D44095

Files:
  include/clang/AST/Decl.h
  include/clang/AST/DeclCXX.h
  include/clang/AST/Type.h
  lib/AST/ASTImporter.cpp
  lib/AST/Decl.cpp
  lib/AST/DeclCXX.cpp
  lib/AST/Type.cpp
  lib/CodeGen/CGBlocks.cpp
  lib/CodeGen/CGNonTrivialStruct.cpp
  lib/CodeGen/CGObjC.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/TargetInfo.cpp
  lib/Sema/SemaDecl.cpp
  lib/Serialization/ASTReaderDecl.cpp
  lib/Serialization/ASTWriter.cpp
  lib/Serialization/ASTWriterDecl.cpp
  test/CodeGenObjC/nontrivial-c-struct-exception.m
  test/CodeGenObjC/weak-in-c-struct.m

Index: test/CodeGenObjC/weak-in-c-struct.m
===
--- /dev/null
+++ test/CodeGenObjC/weak-in-c-struct.m
@@ -0,0 +1,193 @@
+// RUN: %clang_cc1 -triple arm64-apple-ios11 -fobjc-arc -fblocks -fobjc-runtime=ios-11.0 -emit-llvm -o - %s | FileCheck -check-prefix=ARM64 -check-prefix=COMMON %s
+// RUN: %clang_cc1 -triple thumbv7-apple-ios10 -fobjc-arc -fblocks -fobjc-runtime=ios-10.0 -emit-llvm -o - %s | FileCheck -check-prefix=COMMON %s
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.13 -fobjc-arc -fblocks -fobjc-runtime=macosx-10.13.0 -emit-llvm -o - %s | FileCheck -check-prefix=COMMON %s
+// RUN: %clang_cc1 -triple i386-apple-macosx10.13.0 -fobjc-arc -fblocks -fobjc-runtime=macosx-fragile-10.13.0 -emit-llvm -o - %s | FileCheck -check-prefix=COMMON %s
+
+typedef void (^BlockTy)(void);
+
+// COMMON: %[[STRUCT_WEAK:.*]] = type { i32, i8* }
+
+typedef struct {
+  int f0;
+  __weak id f1;
+} Weak;
+
+Weak getWeak(void);
+void calleeWeak(Weak);
+
+// ARM64: define void @test_constructor_destructor_Weak()
+// ARM64: %[[T:.*]] = alloca %[[STRUCT_WEAK]], align 8
+// ARM64: %[[V0:.*]] = bitcast %[[STRUCT_WEAK]]* %[[T]] to i8**
+// ARM64: call void @__default_constructor_8_w8(i8** %[[V0]])
+// ARM64: %[[V1:.*]] = bitcast %[[STRUCT_WEAK]]* %[[T]] to i8**
+// ARM64: call void @__destructor_8_w8(i8** %[[V1]])
+// ARM64: ret void
+
+// ARM64: define linkonce_odr hidden void @__default_constructor_8_w8(i8** %[[DST:.*]])
+// ARM64: %[[DST_ADDR:.*]] = alloca i8**, align 8
+// ARM64: store i8** %[[DST]], i8*** %[[DST_ADDR]], align 8
+// ARM64: %[[V0:.*]] = load i8**, i8*** %[[DST_ADDR]], align 8
+// ARM64: %[[V1]] = bitcast i8** %[[V0]] to i8*
+// ARM64: %[[V2:.*]] = getelementptr inbounds i8, i8* %[[V1]], i64 8
+// ARM64: %[[V3:.*]] = bitcast i8* %[[V2]] to i8**
+// ARM64: %[[V4:.*]] = bitcast i8** %[[V3]] to i8*
+// ARM64: call void @llvm.memset.p0i8.i64(i8* align 8 %[[V4]], i8 0, i64 8, i1 false)
+
+// ARM64: define linkonce_odr hidden void @__destructor_8_w8(i8** %[[DST:.*]])
+// ARM64: %[[DST_ADDR:.*]] = alloca i8**, align 8
+// ARM64: store i8** %[[DST]], i8*** %[[DST_ADDR]], align 8
+// ARM64: %[[V0:.*]] = load i8**, i8*** %[[DST_ADDR]], align 8
+// ARM64: %[[V1:.*]] = bitcast i8** %[[V0]] to i8*
+// ARM64: %[[V2:.*]] = getelementptr inbounds i8, i8* %[[V1]], i64 8
+// ARM64: %[[V3:.*]] = bitcast i8* %[[V2]] to i8**
+// ARM64: call void @objc_destroyWeak(i8** %[[V3]])
+
+void test_constructor_destructor_Weak(void) {
+  Weak t;
+}
+
+// ARM64: define void @test_copy_constructor_Weak(%[[STRUCT_WEAK]]* %{{.*}})
+// ARM64: call void @__copy_constructor_8_8_t0w4_w8(i8** %{{.*}}, i8** %{{.*}})
+// ARM64: call void @__destructor_8_w8(i8** %{{.*}})
+
+// ARM64: define linkonce_odr hidden void @__copy_constructor_8_8_t0w4_w8(i8** %[[DST:.*]], i8** %[[SRC:.*]])
+// ARM64: %[[DST_ADDR:.*]] = alloca i8**, align 8
+// ARM64: %[[SRC_ADDR:.*]] = alloca i8**, align 8
+// ARM64: store i8** %[[DST]], i8*** %[[DST_ADDR]], align 8
+// ARM64: store i8** %[[SRC]], i8*** %[[SRC_ADDR]], align 8
+// ARM64: %[[V0:.*]] = load i8**, i8*** %[[DST_ADDR]], align 8
+// ARM64: %[[V1:.*]] = load i8**, i8*** %[[SRC_ADDR]], align 8
+// ARM64: %[[V2:.*]] = bitcast i8** %[[V0]] to i32*
+// ARM64: %[[V3:.*]] = bitcast i8** %[[V1]] to i32*
+// ARM64: %[[V4:.*]] = load i32, i32* %[[V3]], align 8
+// ARM64: store i32 %[[V4]], i32* %[[V2]], align 8
+// ARM64: %[[V5:.*]] = bitcast i8** %[[V0]] to i8*
+// ARM64: %[[V6:.*]] = getelementptr inbounds i8, i8* %[[V5]], i64 8
+// ARM64: %[[V7:.*]] = bitcast i8* %[[V6]] to i8**
+// ARM64: %[[V8:.*]] = bitcast i8** %[[V1]] to i8*
+// ARM64: %[[V9:.*]] = getelementptr inbounds i8, i8* %[[V8]], i64 8
+// ARM64: %[[V10:.*]] = bitcast i8* %[[V9]] to i8**
+// ARM64: call void @objc_copyWeak(i8** %[[V7]], i8** %[[V10]])
+
+void test_copy_constructor_Weak(Weak *s) {
+  Weak t = *s;
+}
+
+// ARM64: define void @test_copy_assignment_Weak(%[[STRUCT_WEAK]]* %{{.*}}, %[[STRUCT_WEAK]]* %{{.*}})
+// ARM64: call void @__copy_assignment_8_8_t0w4_w8(i8** %{{.*}}, i8** %{{.*}})
+
+// ARM64: define linkonce_odr hidden void @__copy_assignment_8_8_t0w4_w8(i8** %[[DST:.*]], i8** %[[SRC:.*]])
+// ARM64: %[[DST_ADDR:.*]] = alloca i8**, align 8
+// ARM64: %[[SRC_ADDR:.*]] =

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

2018-03-05 Thread John McCall via Phabricator via cfe-commits
rjmccall added inline comments.



Comment at: include/clang/AST/Decl.h:3631
+PassedIndirectly = true;
+  }
+

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.



Comment at: lib/CodeGen/CGNonTrivialStruct.cpp:764
+Object = CGF->EmitObjCConsumeObject(QT, Object);
+CGF->EmitARCStoreWeak(Addrs[DstIdx], Object, true);
+  }

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.



Comment at: lib/CodeGen/TargetInfo.cpp:1326
+  if (RetTy.isPassedIndirectly() == QualType::APK_Struct)
+return getNaturalAlignIndirect(RetTy);
+

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.


Repository:
  rC Clang

https://reviews.llvm.org/D44095



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


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

2018-03-05 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak created this revision.
ahatanak added reviewers: rjmccall, doug.gregor, rsmith.

This patch makes changes that are needed to allow `__weak` fields in C structs 
when ARC is enabled.


Repository:
  rC Clang

https://reviews.llvm.org/D44095

Files:
  include/clang/AST/Decl.h
  include/clang/AST/Type.h
  lib/AST/Decl.cpp
  lib/AST/Type.cpp
  lib/CodeGen/CGBlocks.cpp
  lib/CodeGen/CGNonTrivialStruct.cpp
  lib/CodeGen/TargetInfo.cpp
  lib/Sema/SemaDecl.cpp
  test/CodeGenObjC/nontrivial-c-struct-exception.m
  test/CodeGenObjC/weak-in-c-struct.m

Index: test/CodeGenObjC/weak-in-c-struct.m
===
--- /dev/null
+++ test/CodeGenObjC/weak-in-c-struct.m
@@ -0,0 +1,193 @@
+// RUN: %clang_cc1 -triple arm64-apple-ios11 -fobjc-arc -fblocks -fobjc-runtime=ios-11.0 -emit-llvm -o - %s | FileCheck -check-prefix=ARM64 -check-prefix=COMMON %s
+// RUN: %clang_cc1 -triple thumbv7-apple-ios10 -fobjc-arc -fblocks -fobjc-runtime=ios-10.0 -emit-llvm -o - %s | FileCheck -check-prefix=COMMON %s
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.13 -fobjc-arc -fblocks -fobjc-runtime=macosx-10.13.0 -emit-llvm -o - %s | FileCheck -check-prefix=COMMON %s
+// RUN: %clang_cc1 -triple i386-apple-macosx10.13.0 -fobjc-arc -fblocks -fobjc-runtime=macosx-fragile-10.13.0 -emit-llvm -o - %s | FileCheck -check-prefix=COMMON %s
+
+typedef void (^BlockTy)(void);
+
+// COMMON: %[[STRUCT_WEAK:.*]] = type { i32, i8* }
+
+typedef struct {
+  int f0;
+  __weak id f1;
+} Weak;
+
+Weak getWeak(void);
+void calleeWeak(Weak);
+
+// ARM64: define void @test_constructor_destructor_Weak()
+// ARM64: %[[T:.*]] = alloca %[[STRUCT_WEAK]], align 8
+// ARM64: %[[V0:.*]] = bitcast %[[STRUCT_WEAK]]* %[[T]] to i8**
+// ARM64: call void @__default_constructor_8_w8(i8** %[[V0]])
+// ARM64: %[[V1:.*]] = bitcast %[[STRUCT_WEAK]]* %[[T]] to i8**
+// ARM64: call void @__destructor_8_w8(i8** %[[V1]])
+// ARM64: ret void
+
+// ARM64: define linkonce_odr hidden void @__default_constructor_8_w8(i8** %[[DST:.*]])
+// ARM64: %[[DST_ADDR:.*]] = alloca i8**, align 8
+// ARM64: store i8** %[[DST]], i8*** %[[DST_ADDR]], align 8
+// ARM64: %[[V0:.*]] = load i8**, i8*** %[[DST_ADDR]], align 8
+// ARM64: %[[V1]] = bitcast i8** %[[V0]] to i8*
+// ARM64: %[[V2:.*]] = getelementptr inbounds i8, i8* %[[V1]], i64 8
+// ARM64: %[[V3:.*]] = bitcast i8* %[[V2]] to i8**
+// ARM64: %[[V4:.*]] = bitcast i8** %[[V3]] to i8*
+// ARM64: call void @llvm.memset.p0i8.i64(i8* align 8 %[[V4]], i8 0, i64 8, i1 false)
+
+// ARM64: define linkonce_odr hidden void @__destructor_8_w8(i8** %[[DST:.*]])
+// ARM64: %[[DST_ADDR:.*]] = alloca i8**, align 8
+// ARM64: store i8** %[[DST]], i8*** %[[DST_ADDR]], align 8
+// ARM64: %[[V0:.*]] = load i8**, i8*** %[[DST_ADDR]], align 8
+// ARM64: %[[V1:.*]] = bitcast i8** %[[V0]] to i8*
+// ARM64: %[[V2:.*]] = getelementptr inbounds i8, i8* %[[V1]], i64 8
+// ARM64: %[[V3:.*]] = bitcast i8* %[[V2]] to i8**
+// ARM64: call void @objc_destroyWeak(i8** %[[V3]])
+
+void test_constructor_destructor_Weak(void) {
+  Weak t;
+}
+
+// ARM64: define void @test_copy_constructor_Weak(%[[STRUCT_WEAK]]* %{{.*}})
+// ARM64: call void @__copy_constructor_8_8_t0w4_w8(i8** %{{.*}}, i8** %{{.*}})
+// ARM64: call void @__destructor_8_w8(i8** %{{.*}})
+
+// ARM64: define linkonce_odr hidden void @__copy_constructor_8_8_t0w4_w8(i8** %[[DST:.*]], i8** %[[SRC:.*]])
+// ARM64: %[[DST_ADDR:.*]] = alloca i8**, align 8
+// ARM64: %[[SRC_ADDR:.*]] = alloca i8**, align 8
+// ARM64: store i8** %[[DST]], i8*** %[[DST_ADDR]], align 8
+// ARM64: store i8** %[[SRC]], i8*** %[[SRC_ADDR]], align 8
+// ARM64: %[[V0:.*]] = load i8**, i8*** %[[DST_ADDR]], align 8
+// ARM64: %[[V1:.*]] = load i8**, i8*** %[[SRC_ADDR]], align 8
+// ARM64: %[[V2:.*]] = bitcast i8** %[[V0]] to i32*
+// ARM64: %[[V3:.*]] = bitcast i8** %[[V1]] to i32*
+// ARM64: %[[V4:.*]] = load i32, i32* %[[V3]], align 8
+// ARM64: store i32 %[[V4]], i32* %[[V2]], align 8
+// ARM64: %[[V5:.*]] = bitcast i8** %[[V0]] to i8*
+// ARM64: %[[V6:.*]] = getelementptr inbounds i8, i8* %[[V5]], i64 8
+// ARM64: %[[V7:.*]] = bitcast i8* %[[V6]] to i8**
+// ARM64: %[[V8:.*]] = bitcast i8** %[[V1]] to i8*
+// ARM64: %[[V9:.*]] = getelementptr inbounds i8, i8* %[[V8]], i64 8
+// ARM64: %[[V10:.*]] = bitcast i8* %[[V9]] to i8**
+// ARM64: call void @objc_copyWeak(i8** %[[V7]], i8** %[[V10]])
+
+void test_copy_constructor_Weak(Weak *s) {
+  Weak t = *s;
+}
+
+// ARM64: define void @test_copy_assignment_Weak(%[[STRUCT_WEAK]]* %{{.*}}, %[[STRUCT_WEAK]]* %{{.*}})
+// ARM64: call void @__copy_assignment_8_8_t0w4_w8(i8** %{{.*}}, i8** %{{.*}})
+
+// ARM64: define linkonce_odr hidden void @__copy_assignment_8_8_t0w4_w8(i8** %[[DST:.*]], i8** %[[SRC:.*]])
+// ARM64: %[[DST_ADDR:.*]] = alloca i8**, align 8
+// ARM64: %[[SRC_ADDR:.*]] = alloca i8**, align 8
+// ARM64: store i8** %[[DST]], i8*** %[[DST_ADDR]], align 8
+// ARM64: store i8** %[[SRC]], i8*** %[[SRC_ADDR]], align 8
+// ARM64: %[[V0:.*]] = load