[PATCH] D50144: Add Windows support for the GNUstep Objective-C ABI V2.

[David Chisnall via Phabricator via cfe-commits]

theraven updated this revision to Diff 159312.
theraven added a comment.

- Fix failing test.


Repository:
  rC Clang

https://reviews.llvm.org/D50144

Files:
  include/clang/Driver/Options.td
  lib/AST/MicrosoftMangle.cpp
  lib/CodeGen/CGBlocks.cpp
  lib/CodeGen/CGException.cpp
  lib/CodeGen/CGObjCGNU.cpp
  lib/CodeGen/CGObjCRuntime.cpp
  lib/CodeGen/CGObjCRuntime.h
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/EHScopeStack.h
  lib/Driver/ToolChains/Clang.cpp
  test/CodeGenObjC/gnu-init.m
  test/CodeGenObjC/gnustep2-proto.m
  test/CodeGenObjCXX/arc-marker-funclet.mm
  test/CodeGenObjCXX/microsoft-abi-arc-param-order.mm
  test/CodeGenObjCXX/msabi-objc-extensions.mm
  test/CodeGenObjCXX/msabi-objc-types.mm

Index: test/CodeGenObjCXX/msabi-objc-types.mm
===
--- test/CodeGenObjCXX/msabi-objc-types.mm
+++ test/CodeGenObjCXX/msabi-objc-types.mm
@@ -3,166 +3,166 @@
 @class I;
 
 id kid;
-// CHECK: @"?kid@@3PAUobjc_object@@A" =  dso_local global
+// CHECK: @"?kid@@3PAU.objc_object@@A" =  dso_local global
 
 Class klass;
-// CHECK: @"?klass@@3PAUobjc_class@@A" = dso_local global
+// CHECK: @"?klass@@3PAU.objc_class@@A" = dso_local global
 
 I *kI;
-// CHECK: @"?kI@@3PAUI@@A" = dso_local global
+// CHECK: @"?kI@@3PAU.objc_cls_I@@A" = dso_local global
 
 void f(I *) {}
-// CHECK-LABEL: "?f@@YAXPAUI@@@Z"
+// CHECK-LABEL: "?f@@YAXPAU.objc_cls_I@@@Z"
 
 void f(const I *) {}
-// CHECK-LABEL: "?f@@YAXPBUI@@@Z"
+// CHECK-LABEL: "?f@@YAXPBU.objc_cls_I@@@Z"
 
 void f(I &) {}
-// CHECK-LABEL: "?f@@YAXAAUI@@@Z"
+// CHECK-LABEL: "?f@@YAXAAU.objc_cls_I@@@Z"
 
 void f(const I &) {}
-// CHECK-LABEL: "?f@@YAXABUI@@@Z"
+// CHECK-LABEL: "?f@@YAXABU.objc_cls_I@@@Z"
 
 void f(const I &&) {}
-// CHECK-LABEL: "?f@@YAX$$QBUI@@@Z"
+// CHECK-LABEL: "?f@@YAX$$QBU.objc_cls_I@@@Z"
 
 void g(id) {}
-// CHECK-LABEL: "?g@@YAXPAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAXPAU.objc_object@@@Z"
 
 void g(id &) {}
-// CHECK-LABEL: "?g@@YAXAAPAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAXAAPAU.objc_object@@@Z"
 
 void g(const id &) {}
-// CHECK-LABEL: "?g@@YAXABQAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAXABQAU.objc_object@@@Z"
 
 void g(id &&) {}
-// CHECK-LABEL: "?g@@YAX$$QAPAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAX$$QAPAU.objc_object@@@Z"
 
 void h(Class) {}
-// CHECK-LABEL: "?h@@YAXPAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAXPAU.objc_class@@@Z"
 
 void h(Class &) {}
-// CHECK-LABEL: "?h@@YAXAAPAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAXAAPAU.objc_class@@@Z"
 
 void h(const Class &) {}
-// CHECK-LABEL: "?h@@YAXABQAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAXABQAU.objc_class@@@Z"
 
 void h(Class &&) {}
-// CHECK-LABEL: "?h@@YAX$$QAPAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAX$$QAPAU.objc_class@@@Z"
 
 I *i() { return nullptr; }
-// CHECK-LABEL: "?i@@YAPAUI@@XZ"
+// CHECK-LABEL: "?i@@YAPAU.objc_cls_I@@XZ"
 
 const I *j() { return nullptr; }
-// CHECK-LABEL: "?j@@YAPBUI@@XZ"
+// CHECK-LABEL: "?j@@YAPBU.objc_cls_I@@XZ"
 
 I &k() { return *kI; }
-// CHECK-LABEL: "?k@@YAAAUI@@XZ"
+// CHECK-LABEL: "?k@@YAAAU.objc_cls_I@@XZ"
 
 const I &l() { return *kI; }
-// CHECK-LABEL: "?l@@YAABUI@@XZ"
+// CHECK-LABEL: "?l@@YAABU.objc_cls_I@@XZ"
 
 void m(const id) {}
-// CHECK-LABEL: "?m@@YAXQAUobjc_object@@@Z"
+// CHECK-LABEL: "?m@@YAXQAU.objc_object@@@Z"
 
 void m(const I *) {}
-// CHECK-LABEL: "?m@@YAXPBUI@@@Z"
+// CHECK-LABEL: "?m@@YAXPBU.objc_cls_I@@@Z"
 
 void n(SEL) {}
-// CHECK-LABEL: "?n@@YAXPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXPAU.objc_selector@@@Z"
 
 void n(SEL *) {}
-// CHECK-LABEL: "?n@@YAXPAPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXPAPAU.objc_selector@@@Z"
 
 void n(const SEL *) {}
-// CHECK-LABEL: "?n@@YAXPBQAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXPBQAU.objc_selector@@@Z"
 
 void n(SEL &) {}
-// CHECK-LABEL: "?n@@YAXAAPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXAAPAU.objc_selector@@@Z"
 
 void n(const SEL &) {}
-// CHECK-LABEL: "?n@@YAXABQAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXABQAU.objc_selector@@@Z"
 
 void n(SEL &&) {}
-// CHECK-LABEL: "?n@@YAX$$QAPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAX$$QAPAU.objc_selector@@@Z"
 
 struct __declspec(dllexport) s {
   struct s &operator=(const struct s &) = delete;
 
   void m(I *) {}
-  // CHECK-LABEL: "?m@s@@QAAXPAUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXPAU.objc_cls_I@@@Z"
 
   void m(const I *) {}
-  // CHECK-LABEL: "?m@s@@QAAXPBUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXPBU.objc_cls_I@@@Z"
 
   void m(I &) {}
-  // CHECK-LABEL: "?m@s@@QAAXAAUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXAAU.objc_cls_I@@@Z"
 
   void m(const I &) {}
-  // CHECK-LABEL: "?m@s@@QAAXABUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXABU.objc_cls_I@@@Z"
 
   void m(I &&) {}
-  // CHECK-LABEL: "?m@s@@QAAX$$QAUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAX$$QAU.objc_cls_I@@@Z"
 
   void m(const I &&) {}
-  // CHECK-LABEL: "?m@s@@QAAX$$QBUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAX$$QBU.objc_cls_I@@@Z"
 
   void m(id) {}
-  // CHECK-LABEL: "?m@s@@QAAXPAUobjc_object@@@Z"
+  // CHEC

[PATCH] D50144: Add Windows support for the GNUstep Objective-C ABI V2.

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

I'd like to commit this, unless @rjmccall has any objections.


Repository:
  rC Clang

https://reviews.llvm.org/D50144



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

[PATCH] D50144: Add Windows support for the GNUstep Objective-C ABI V2.

[David Chisnall via Phabricator via cfe-commits]

theraven updated this revision to Diff 159651.
theraven marked 2 inline comments as done.
theraven added a comment.
Herald added a subscriber: mgrang.

- Address Dustin's review comments.
- Fix an issue in protocol generation.
- Fix failing test.
- [gnu-objc] Make selector order deterministic.
- Address rjmcall's review comments.


Repository:
  rC Clang

https://reviews.llvm.org/D50144

Files:
  include/clang/Driver/Options.td
  lib/AST/MicrosoftMangle.cpp
  lib/CodeGen/CGBlocks.cpp
  lib/CodeGen/CGException.cpp
  lib/CodeGen/CGObjCGNU.cpp
  lib/CodeGen/CGObjCRuntime.cpp
  lib/CodeGen/CGObjCRuntime.h
  lib/CodeGen/CodeGenFunction.h
  lib/Driver/ToolChains/Clang.cpp
  test/CodeGenObjC/gnu-init.m
  test/CodeGenObjC/gnustep2-proto.m
  test/CodeGenObjCXX/arc-marker-funclet.mm
  test/CodeGenObjCXX/microsoft-abi-arc-param-order.mm
  test/CodeGenObjCXX/msabi-objc-extensions.mm
  test/CodeGenObjCXX/msabi-objc-types.mm

Index: test/CodeGenObjCXX/msabi-objc-types.mm
===
--- test/CodeGenObjCXX/msabi-objc-types.mm
+++ test/CodeGenObjCXX/msabi-objc-types.mm
@@ -3,166 +3,166 @@
 @class I;
 
 id kid;
-// CHECK: @"?kid@@3PAUobjc_object@@A" =  dso_local global
+// CHECK: @"?kid@@3PAU.objc_object@@A" =  dso_local global
 
 Class klass;
-// CHECK: @"?klass@@3PAUobjc_class@@A" = dso_local global
+// CHECK: @"?klass@@3PAU.objc_class@@A" = dso_local global
 
 I *kI;
-// CHECK: @"?kI@@3PAUI@@A" = dso_local global
+// CHECK: @"?kI@@3PAU.objc_cls_I@@A" = dso_local global
 
 void f(I *) {}
-// CHECK-LABEL: "?f@@YAXPAUI@@@Z"
+// CHECK-LABEL: "?f@@YAXPAU.objc_cls_I@@@Z"
 
 void f(const I *) {}
-// CHECK-LABEL: "?f@@YAXPBUI@@@Z"
+// CHECK-LABEL: "?f@@YAXPBU.objc_cls_I@@@Z"
 
 void f(I &) {}
-// CHECK-LABEL: "?f@@YAXAAUI@@@Z"
+// CHECK-LABEL: "?f@@YAXAAU.objc_cls_I@@@Z"
 
 void f(const I &) {}
-// CHECK-LABEL: "?f@@YAXABUI@@@Z"
+// CHECK-LABEL: "?f@@YAXABU.objc_cls_I@@@Z"
 
 void f(const I &&) {}
-// CHECK-LABEL: "?f@@YAX$$QBUI@@@Z"
+// CHECK-LABEL: "?f@@YAX$$QBU.objc_cls_I@@@Z"
 
 void g(id) {}
-// CHECK-LABEL: "?g@@YAXPAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAXPAU.objc_object@@@Z"
 
 void g(id &) {}
-// CHECK-LABEL: "?g@@YAXAAPAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAXAAPAU.objc_object@@@Z"
 
 void g(const id &) {}
-// CHECK-LABEL: "?g@@YAXABQAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAXABQAU.objc_object@@@Z"
 
 void g(id &&) {}
-// CHECK-LABEL: "?g@@YAX$$QAPAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAX$$QAPAU.objc_object@@@Z"
 
 void h(Class) {}
-// CHECK-LABEL: "?h@@YAXPAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAXPAU.objc_class@@@Z"
 
 void h(Class &) {}
-// CHECK-LABEL: "?h@@YAXAAPAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAXAAPAU.objc_class@@@Z"
 
 void h(const Class &) {}
-// CHECK-LABEL: "?h@@YAXABQAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAXABQAU.objc_class@@@Z"
 
 void h(Class &&) {}
-// CHECK-LABEL: "?h@@YAX$$QAPAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAX$$QAPAU.objc_class@@@Z"
 
 I *i() { return nullptr; }
-// CHECK-LABEL: "?i@@YAPAUI@@XZ"
+// CHECK-LABEL: "?i@@YAPAU.objc_cls_I@@XZ"
 
 const I *j() { return nullptr; }
-// CHECK-LABEL: "?j@@YAPBUI@@XZ"
+// CHECK-LABEL: "?j@@YAPBU.objc_cls_I@@XZ"
 
 I &k() { return *kI; }
-// CHECK-LABEL: "?k@@YAAAUI@@XZ"
+// CHECK-LABEL: "?k@@YAAAU.objc_cls_I@@XZ"
 
 const I &l() { return *kI; }
-// CHECK-LABEL: "?l@@YAABUI@@XZ"
+// CHECK-LABEL: "?l@@YAABU.objc_cls_I@@XZ"
 
 void m(const id) {}
-// CHECK-LABEL: "?m@@YAXQAUobjc_object@@@Z"
+// CHECK-LABEL: "?m@@YAXQAU.objc_object@@@Z"
 
 void m(const I *) {}
-// CHECK-LABEL: "?m@@YAXPBUI@@@Z"
+// CHECK-LABEL: "?m@@YAXPBU.objc_cls_I@@@Z"
 
 void n(SEL) {}
-// CHECK-LABEL: "?n@@YAXPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXPAU.objc_selector@@@Z"
 
 void n(SEL *) {}
-// CHECK-LABEL: "?n@@YAXPAPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXPAPAU.objc_selector@@@Z"
 
 void n(const SEL *) {}
-// CHECK-LABEL: "?n@@YAXPBQAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXPBQAU.objc_selector@@@Z"
 
 void n(SEL &) {}
-// CHECK-LABEL: "?n@@YAXAAPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXAAPAU.objc_selector@@@Z"
 
 void n(const SEL &) {}
-// CHECK-LABEL: "?n@@YAXABQAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXABQAU.objc_selector@@@Z"
 
 void n(SEL &&) {}
-// CHECK-LABEL: "?n@@YAX$$QAPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAX$$QAPAU.objc_selector@@@Z"
 
 struct __declspec(dllexport) s {
   struct s &operator=(const struct s &) = delete;
 
   void m(I *) {}
-  // CHECK-LABEL: "?m@s@@QAAXPAUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXPAU.objc_cls_I@@@Z"
 
   void m(const I *) {}
-  // CHECK-LABEL: "?m@s@@QAAXPBUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXPBU.objc_cls_I@@@Z"
 
   void m(I &) {}
-  // CHECK-LABEL: "?m@s@@QAAXAAUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXAAU.objc_cls_I@@@Z"
 
   void m(const I &) {}
-  // CHECK-LABEL: "?m@s@@QAAXABUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXABU.objc_cls_I@@@Z"
 
   void m(I &&) {}
-  // CHECK-LABEL: "?m@s@@QAAX$$QAUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAX$$QAU.objc_cl

[PATCH] D50144: Add Windows support for the GNUstep Objective-C ABI V2.

[David Chisnall via Phabricator via cfe-commits]

theraven updated this revision to Diff 159653.
theraven added a comment.

- Revert blocks part of the patch to put in a separate review.


Repository:
  rC Clang

https://reviews.llvm.org/D50144

Files:
  include/clang/Driver/Options.td
  lib/AST/MicrosoftMangle.cpp
  lib/CodeGen/CGException.cpp
  lib/CodeGen/CGObjCGNU.cpp
  lib/CodeGen/CGObjCRuntime.cpp
  lib/CodeGen/CGObjCRuntime.h
  lib/CodeGen/CodeGenFunction.h
  lib/Driver/ToolChains/Clang.cpp
  test/CodeGenObjC/gnu-init.m
  test/CodeGenObjC/gnustep2-proto.m
  test/CodeGenObjCXX/arc-marker-funclet.mm
  test/CodeGenObjCXX/microsoft-abi-arc-param-order.mm
  test/CodeGenObjCXX/msabi-objc-extensions.mm
  test/CodeGenObjCXX/msabi-objc-types.mm

Index: test/CodeGenObjCXX/msabi-objc-types.mm
===
--- test/CodeGenObjCXX/msabi-objc-types.mm
+++ test/CodeGenObjCXX/msabi-objc-types.mm
@@ -3,166 +3,166 @@
 @class I;
 
 id kid;
-// CHECK: @"?kid@@3PAUobjc_object@@A" =  dso_local global
+// CHECK: @"?kid@@3PAU.objc_object@@A" =  dso_local global
 
 Class klass;
-// CHECK: @"?klass@@3PAUobjc_class@@A" = dso_local global
+// CHECK: @"?klass@@3PAU.objc_class@@A" = dso_local global
 
 I *kI;
-// CHECK: @"?kI@@3PAUI@@A" = dso_local global
+// CHECK: @"?kI@@3PAU.objc_cls_I@@A" = dso_local global
 
 void f(I *) {}
-// CHECK-LABEL: "?f@@YAXPAUI@@@Z"
+// CHECK-LABEL: "?f@@YAXPAU.objc_cls_I@@@Z"
 
 void f(const I *) {}
-// CHECK-LABEL: "?f@@YAXPBUI@@@Z"
+// CHECK-LABEL: "?f@@YAXPBU.objc_cls_I@@@Z"
 
 void f(I &) {}
-// CHECK-LABEL: "?f@@YAXAAUI@@@Z"
+// CHECK-LABEL: "?f@@YAXAAU.objc_cls_I@@@Z"
 
 void f(const I &) {}
-// CHECK-LABEL: "?f@@YAXABUI@@@Z"
+// CHECK-LABEL: "?f@@YAXABU.objc_cls_I@@@Z"
 
 void f(const I &&) {}
-// CHECK-LABEL: "?f@@YAX$$QBUI@@@Z"
+// CHECK-LABEL: "?f@@YAX$$QBU.objc_cls_I@@@Z"
 
 void g(id) {}
-// CHECK-LABEL: "?g@@YAXPAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAXPAU.objc_object@@@Z"
 
 void g(id &) {}
-// CHECK-LABEL: "?g@@YAXAAPAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAXAAPAU.objc_object@@@Z"
 
 void g(const id &) {}
-// CHECK-LABEL: "?g@@YAXABQAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAXABQAU.objc_object@@@Z"
 
 void g(id &&) {}
-// CHECK-LABEL: "?g@@YAX$$QAPAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAX$$QAPAU.objc_object@@@Z"
 
 void h(Class) {}
-// CHECK-LABEL: "?h@@YAXPAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAXPAU.objc_class@@@Z"
 
 void h(Class &) {}
-// CHECK-LABEL: "?h@@YAXAAPAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAXAAPAU.objc_class@@@Z"
 
 void h(const Class &) {}
-// CHECK-LABEL: "?h@@YAXABQAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAXABQAU.objc_class@@@Z"
 
 void h(Class &&) {}
-// CHECK-LABEL: "?h@@YAX$$QAPAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAX$$QAPAU.objc_class@@@Z"
 
 I *i() { return nullptr; }
-// CHECK-LABEL: "?i@@YAPAUI@@XZ"
+// CHECK-LABEL: "?i@@YAPAU.objc_cls_I@@XZ"
 
 const I *j() { return nullptr; }
-// CHECK-LABEL: "?j@@YAPBUI@@XZ"
+// CHECK-LABEL: "?j@@YAPBU.objc_cls_I@@XZ"
 
 I &k() { return *kI; }
-// CHECK-LABEL: "?k@@YAAAUI@@XZ"
+// CHECK-LABEL: "?k@@YAAAU.objc_cls_I@@XZ"
 
 const I &l() { return *kI; }
-// CHECK-LABEL: "?l@@YAABUI@@XZ"
+// CHECK-LABEL: "?l@@YAABU.objc_cls_I@@XZ"
 
 void m(const id) {}
-// CHECK-LABEL: "?m@@YAXQAUobjc_object@@@Z"
+// CHECK-LABEL: "?m@@YAXQAU.objc_object@@@Z"
 
 void m(const I *) {}
-// CHECK-LABEL: "?m@@YAXPBUI@@@Z"
+// CHECK-LABEL: "?m@@YAXPBU.objc_cls_I@@@Z"
 
 void n(SEL) {}
-// CHECK-LABEL: "?n@@YAXPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXPAU.objc_selector@@@Z"
 
 void n(SEL *) {}
-// CHECK-LABEL: "?n@@YAXPAPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXPAPAU.objc_selector@@@Z"
 
 void n(const SEL *) {}
-// CHECK-LABEL: "?n@@YAXPBQAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXPBQAU.objc_selector@@@Z"
 
 void n(SEL &) {}
-// CHECK-LABEL: "?n@@YAXAAPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXAAPAU.objc_selector@@@Z"
 
 void n(const SEL &) {}
-// CHECK-LABEL: "?n@@YAXABQAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXABQAU.objc_selector@@@Z"
 
 void n(SEL &&) {}
-// CHECK-LABEL: "?n@@YAX$$QAPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAX$$QAPAU.objc_selector@@@Z"
 
 struct __declspec(dllexport) s {
   struct s &operator=(const struct s &) = delete;
 
   void m(I *) {}
-  // CHECK-LABEL: "?m@s@@QAAXPAUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXPAU.objc_cls_I@@@Z"
 
   void m(const I *) {}
-  // CHECK-LABEL: "?m@s@@QAAXPBUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXPBU.objc_cls_I@@@Z"
 
   void m(I &) {}
-  // CHECK-LABEL: "?m@s@@QAAXAAUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXAAU.objc_cls_I@@@Z"
 
   void m(const I &) {}
-  // CHECK-LABEL: "?m@s@@QAAXABUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXABU.objc_cls_I@@@Z"
 
   void m(I &&) {}
-  // CHECK-LABEL: "?m@s@@QAAX$$QAUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAX$$QAU.objc_cls_I@@@Z"
 
   void m(const I &&) {}
-  // CHECK-LABEL: "?m@s@@QAAX$$QBUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAX$$QBU.objc_cls_I@@@Z"
 
   void m(id) {}
-  // CHECK-LABEL: "?m@s@@QAAXPAUobjc_object@@@Z"
+  // CHECK-LABEL: "?m@

[PATCH] D50436: Correctly initialise global blocks on Windows.

[David Chisnall via Phabricator via cfe-commits]

theraven created this revision.
theraven added a reviewer: rjmccall.
Herald added a subscriber: cfe-commits.

Windows does not allow globals to be initialised to point to globals in
another DLL.  Exported globals may be referenced only from code.  Work
around this by creating an initialiser that runs in early library
initialisation and sets the isa pointer.


Repository:
  rC Clang

https://reviews.llvm.org/D50436

Files:
  lib/CodeGen/CGBlocks.cpp
  test/CodeGen/global-blocks-win32.c


Index: test/CodeGen/global-blocks-win32.c
===
--- /dev/null
+++ test/CodeGen/global-blocks-win32.c
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -fblocks -triple i386-pc-windows-msvc %s -emit-llvm -o - 
-fblocks | FileCheck %s
+
+
+int (^x)(void) = ^() { return 21; };
+
+
+// Check that the block literal is emitted with a null isa pointer
+// CHECK: @__block_literal_global = internal global { i8**, i32, i32, i8*, 
%struct.__block_descriptor* } { i8** null, 
+
+// Check that _NSConcreteGlobalBlock has the correct dllimport specifier.
+// CHECK: @_NSConcreteGlobalBlock = external dllimport global i8*
+// Check that we create an initialiser pointer in the correct section (early 
library initialisation).
+// CHECK: @.block_isa_init_ptr = internal constant void ()* @.block_isa_init, 
section ".CRT$XCLa"
+
+// Check that we emit an initialiser for it.
+// CHECK: define internal void @.block_isa_init() {
+// CHECK: store i8** @_NSConcreteGlobalBlock, i8*** getelementptr inbounds ({ 
i8**, i32, i32, i8*, %struct.__block_descriptor* }, { i8**, i32, i32, i8*, 
%struct.__block_descriptor* }* @__block_literal_global, i32 0, i32 0), align 4
+
Index: lib/CodeGen/CGBlocks.cpp
===
--- lib/CodeGen/CGBlocks.cpp
+++ lib/CodeGen/CGBlocks.cpp
@@ -1213,9 +1213,13 @@
   auto fields = builder.beginStruct();
 
   bool IsOpenCL = CGM.getLangOpts().OpenCL;
+  bool IsWindows = CGM.getTarget().getTriple().isOSWindows();
   if (!IsOpenCL) {
 // isa
-fields.add(CGM.getNSConcreteGlobalBlock());
+if (IsWindows)
+  fields.addNullPointer(CGM.Int8PtrPtrTy);
+else
+  fields.add(CGM.getNSConcreteGlobalBlock());
 
 // __flags
 BlockFlags flags = BLOCK_IS_GLOBAL | BLOCK_HAS_SIGNATURE;
@@ -1250,7 +1254,27 @@
 
   llvm::Constant *literal = fields.finishAndCreateGlobal(
   "__block_literal_global", blockInfo.BlockAlign,
-  /*constant*/ true, llvm::GlobalVariable::InternalLinkage, AddrSpace);
+  /*constant*/ !IsWindows, llvm::GlobalVariable::InternalLinkage, 
AddrSpace);
+
+  // Windows does not allow globals to be initialised to point to globals in
+  // different DLLs.  Any such variables must run code to initialise them.
+  if (IsWindows) {
+auto *Init = llvm::Function::Create(llvm::FunctionType::get(CGM.VoidTy,
+  {}), llvm::GlobalValue::InternalLinkage, ".block_isa_init",
+&CGM.getModule());
+llvm::IRBuilder<> b(llvm::BasicBlock::Create(CGM.getLLVMContext(), "entry",
+  Init));
+b.CreateAlignedStore(CGM.getNSConcreteGlobalBlock(),
+b.CreateStructGEP(literal, 0), CGM.getPointerAlign().getQuantity());
+b.CreateRetVoid();
+// We can't use the normal LLVM global initialisation array, because we
+// need to specify that this runs early in library initialisation.
+auto *InitVar = new llvm::GlobalVariable(CGM.getModule(), Init->getType(),
+/*isConstant*/true, llvm::GlobalValue::InternalLinkage,
+Init, ".block_isa_init_ptr");
+InitVar->setSection(".CRT$XCLa");
+CGM.addUsedGlobal(InitVar);
+  }
 
   // Return a constant of the appropriately-casted type.
   llvm::Type *RequiredType =


Index: test/CodeGen/global-blocks-win32.c
===
--- /dev/null
+++ test/CodeGen/global-blocks-win32.c
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -fblocks -triple i386-pc-windows-msvc %s -emit-llvm -o - -fblocks | FileCheck %s
+
+
+int (^x)(void) = ^() { return 21; };
+
+
+// Check that the block literal is emitted with a null isa pointer
+// CHECK: @__block_literal_global = internal global { i8**, i32, i32, i8*, %struct.__block_descriptor* } { i8** null, 
+
+// Check that _NSConcreteGlobalBlock has the correct dllimport specifier.
+// CHECK: @_NSConcreteGlobalBlock = external dllimport global i8*
+// Check that we create an initialiser pointer in the correct section (early library initialisation).
+// CHECK: @.block_isa_init_ptr = internal constant void ()* @.block_isa_init, section ".CRT$XCLa"
+
+// Check that we emit an initialiser for it.
+// CHECK: define internal void @.block_isa_init() {
+// CHECK: store i8** @_NSConcreteGlobalBlock, i8*** getelementptr inbounds ({ i8**, i32, i32, i8*, %struct.__block_descriptor* }, { i8**, i32, i32, i8*, %struct.__block_descriptor* }* @__block_literal_global, i32 0, i32 0), align 4
+
Index: lib/CodeGen/CGBlocks.cpp
===

[PATCH] D50144: Add Windows support for the GNUstep Objective-C ABI V2.

[David Chisnall via Phabricator via cfe-commits]

theraven added inline comments.



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",

rjmccall wrote:
> 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.
Split into D50436.



Comment at: lib/CodeGen/CGObjCGNU.cpp:915
+return name;
+  }
   /// The GCC ABI superclass message lookup function.  Takes a pointer to a

rjmccall wrote:
> Can this section-names cleanup also just be in a separate patch?
This is difficult to extract, because it's mostly needed for the COFF part 
where we need to modify the section names.  For ELF, it was fine to keep them 
as separate `const char*`s



Comment at: lib/CodeGen/CGObjCGNU.cpp:2262
 llvm::Constant *CGObjCGNUstep::GetEHType(QualType T) {
+  if (CGM.getContext().getTargetInfo().getTriple().isWindowsMSVCEnvironment())
+return CGM.getCXXABI().getAddrOfRTTIDescriptor(T);

rjmccall wrote:
> 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?
For the EH logic, we are using DWARF exceptions if we are targeting a Windows 
environment that uses DWARF EH and SEH-based exceptions if we are targeting a 
Windows environment that uses MSVC-compatible SEH.

The section code is specific to PE/COFF, where we don't get to use some of the 
ELF tricks.

The blocks code is specific to Windows, because it relates to Windows run-time 
linking.  Hypothetically, a PE/COFF platform could provide dynamic relocations 
that would eliminate the need for that check.

It's possible that some of the PE/COFF vs Windows distinctions are wrong.  For 
example, UEFI images are PE/COFF binaries and if anyone wanted to use blocks in 
a UEFI firmware image then they may find that they need the Windows code path 
(but I do not have a good way of testing this, so restricted it to Windows 
initially).  Similarly, some of the section initialisation code in CGObjCGNU 
may actually be Windows-only, but it's probably a good starting point for 
anyone who actually wants to use Objective-C in UEFI firmware (though the final 
destination for such people is likely to involve padded cells). 



Comment at: lib/CodeGen/CGObjCGNU.cpp:3817
+  if (isRethrow && CGF.CGM.getTarget().getTriple().isWindowsMSVCEnvironment()) 
{
+CGF.EmitRuntimeCallOrInvoke(ExceptionReThrowFn).setDoesNotReturn();
+  }

rjmccall wrote:
> You're sure here that the static information aligns with the dynamic?
I'm not sure I understand this question.



Comment at: lib/CodeGen/CGObjCRuntime.cpp:125
 llvm::Constant *TypeInfo;
+unsigned Flags;
   };

rjmccall wrote:
> Please add some sort of comment about the meaning and source of these flags.
These are not well documented anywhere, including in the Clang Microsoft C++ 
ABI code that I read to see why exceptions weren't working, but I've added a 
small comment that explains why they exist.  I have no idea where the values 
come from though.


Repository:
  rC Clang

https://reviews.llvm.org/D50144



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

[PATCH] D50448: [CGObjCGNU] Rename GetSelector helper method to fix -Woverloaded-virtual warning (PR38210)

[David Chisnall via Phabricator via cfe-commits]

theraven accepted this revision.
theraven added a comment.
This revision is now accepted and ready to land.

Looks good to me.  This method should probably take a StringRef rather than a 
`const std::string&`, but I can make that change separately.


Repository:
  rC Clang

https://reviews.llvm.org/D50448



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

[PATCH] D50436: Correctly initialise global blocks on Windows.

[David Chisnall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL339317: Correctly initialise global blocks on Windows. 
(authored by theraven, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D50436

Files:
  cfe/trunk/lib/CodeGen/CGBlocks.cpp
  cfe/trunk/test/CodeGen/global-blocks-win32.c


Index: cfe/trunk/test/CodeGen/global-blocks-win32.c
===
--- cfe/trunk/test/CodeGen/global-blocks-win32.c
+++ cfe/trunk/test/CodeGen/global-blocks-win32.c
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -fblocks -triple i386-pc-windows-msvc %s -emit-llvm -o - 
-fblocks | FileCheck %s
+
+
+int (^x)(void) = ^() { return 21; };
+
+
+// Check that the block literal is emitted with a null isa pointer
+// CHECK: @__block_literal_global = internal global { i8**, i32, i32, i8*, 
%struct.__block_descriptor* } { i8** null, 
+
+// Check that _NSConcreteGlobalBlock has the correct dllimport specifier.
+// CHECK: @_NSConcreteGlobalBlock = external dllimport global i8*
+// Check that we create an initialiser pointer in the correct section (early 
library initialisation).
+// CHECK: @.block_isa_init_ptr = internal constant void ()* @.block_isa_init, 
section ".CRT$XCLa"
+
+// Check that we emit an initialiser for it.
+// CHECK: define internal void @.block_isa_init() {
+// CHECK: store i8** @_NSConcreteGlobalBlock, i8*** getelementptr inbounds ({ 
i8**, i32, i32, i8*, %struct.__block_descriptor* }, { i8**, i32, i32, i8*, 
%struct.__block_descriptor* }* @__block_literal_global, i32 0, i32 0), align 4
+
Index: cfe/trunk/lib/CodeGen/CGBlocks.cpp
===
--- cfe/trunk/lib/CodeGen/CGBlocks.cpp
+++ cfe/trunk/lib/CodeGen/CGBlocks.cpp
@@ -1213,9 +1213,13 @@
   auto fields = builder.beginStruct();
 
   bool IsOpenCL = CGM.getLangOpts().OpenCL;
+  bool IsWindows = CGM.getTarget().getTriple().isOSWindows();
   if (!IsOpenCL) {
 // isa
-fields.add(CGM.getNSConcreteGlobalBlock());
+if (IsWindows)
+  fields.addNullPointer(CGM.Int8PtrPtrTy);
+else
+  fields.add(CGM.getNSConcreteGlobalBlock());
 
 // __flags
 BlockFlags flags = BLOCK_IS_GLOBAL | BLOCK_HAS_SIGNATURE;
@@ -1250,7 +1254,27 @@
 
   llvm::Constant *literal = fields.finishAndCreateGlobal(
   "__block_literal_global", blockInfo.BlockAlign,
-  /*constant*/ true, llvm::GlobalVariable::InternalLinkage, AddrSpace);
+  /*constant*/ !IsWindows, llvm::GlobalVariable::InternalLinkage, 
AddrSpace);
+
+  // Windows does not allow globals to be initialised to point to globals in
+  // different DLLs.  Any such variables must run code to initialise them.
+  if (IsWindows) {
+auto *Init = llvm::Function::Create(llvm::FunctionType::get(CGM.VoidTy,
+  {}), llvm::GlobalValue::InternalLinkage, ".block_isa_init",
+&CGM.getModule());
+llvm::IRBuilder<> b(llvm::BasicBlock::Create(CGM.getLLVMContext(), "entry",
+  Init));
+b.CreateAlignedStore(CGM.getNSConcreteGlobalBlock(),
+b.CreateStructGEP(literal, 0), CGM.getPointerAlign().getQuantity());
+b.CreateRetVoid();
+// We can't use the normal LLVM global initialisation array, because we
+// need to specify that this runs early in library initialisation.
+auto *InitVar = new llvm::GlobalVariable(CGM.getModule(), Init->getType(),
+/*isConstant*/true, llvm::GlobalValue::InternalLinkage,
+Init, ".block_isa_init_ptr");
+InitVar->setSection(".CRT$XCLa");
+CGM.addUsedGlobal(InitVar);
+  }
 
   // Return a constant of the appropriately-casted type.
   llvm::Type *RequiredType =


Index: cfe/trunk/test/CodeGen/global-blocks-win32.c
===
--- cfe/trunk/test/CodeGen/global-blocks-win32.c
+++ cfe/trunk/test/CodeGen/global-blocks-win32.c
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -fblocks -triple i386-pc-windows-msvc %s -emit-llvm -o - -fblocks | FileCheck %s
+
+
+int (^x)(void) = ^() { return 21; };
+
+
+// Check that the block literal is emitted with a null isa pointer
+// CHECK: @__block_literal_global = internal global { i8**, i32, i32, i8*, %struct.__block_descriptor* } { i8** null, 
+
+// Check that _NSConcreteGlobalBlock has the correct dllimport specifier.
+// CHECK: @_NSConcreteGlobalBlock = external dllimport global i8*
+// Check that we create an initialiser pointer in the correct section (early library initialisation).
+// CHECK: @.block_isa_init_ptr = internal constant void ()* @.block_isa_init, section ".CRT$XCLa"
+
+// Check that we emit an initialiser for it.
+// CHECK: define internal void @.block_isa_init() {
+// CHECK: store i8** @_NSConcreteGlobalBlock, i8*** getelementptr inbounds ({ i8**, i32, i32, i8*, %struct.__block_descriptor* }, { i8**, i32, i32, i8*, %struct.__block_descriptor* }* @__block_literal_global, i32 0, i32 0), align 4
+
Index: cfe/trunk/lib/CodeGen/CGBlocks.cpp
==

[PATCH] D50144: Add Windows support for the GNUstep Objective-C ABI V2.

[David Chisnall via Phabricator via cfe-commits]

theraven updated this revision to Diff 159868.
theraven added a comment.

- Address rjmcall's review comments.
- Revert blocks part of the patch to put in a separate review.


Repository:
  rC Clang

https://reviews.llvm.org/D50144

Files:
  include/clang/Driver/Options.td
  lib/AST/MicrosoftMangle.cpp
  lib/CodeGen/CGException.cpp
  lib/CodeGen/CGObjCGNU.cpp
  lib/CodeGen/CGObjCRuntime.cpp
  lib/CodeGen/CGObjCRuntime.h
  lib/CodeGen/CodeGenFunction.h
  lib/Driver/ToolChains/Clang.cpp
  test/CodeGenObjC/gnu-init.m
  test/CodeGenObjC/gnustep2-proto.m
  test/CodeGenObjCXX/arc-marker-funclet.mm
  test/CodeGenObjCXX/microsoft-abi-arc-param-order.mm
  test/CodeGenObjCXX/msabi-objc-extensions.mm
  test/CodeGenObjCXX/msabi-objc-types.mm

Index: test/CodeGenObjCXX/msabi-objc-types.mm
===
--- test/CodeGenObjCXX/msabi-objc-types.mm
+++ test/CodeGenObjCXX/msabi-objc-types.mm
@@ -3,166 +3,166 @@
 @class I;
 
 id kid;
-// CHECK: @"?kid@@3PAUobjc_object@@A" =  dso_local global
+// CHECK: @"?kid@@3PAU.objc_object@@A" =  dso_local global
 
 Class klass;
-// CHECK: @"?klass@@3PAUobjc_class@@A" = dso_local global
+// CHECK: @"?klass@@3PAU.objc_class@@A" = dso_local global
 
 I *kI;
-// CHECK: @"?kI@@3PAUI@@A" = dso_local global
+// CHECK: @"?kI@@3PAU.objc_cls_I@@A" = dso_local global
 
 void f(I *) {}
-// CHECK-LABEL: "?f@@YAXPAUI@@@Z"
+// CHECK-LABEL: "?f@@YAXPAU.objc_cls_I@@@Z"
 
 void f(const I *) {}
-// CHECK-LABEL: "?f@@YAXPBUI@@@Z"
+// CHECK-LABEL: "?f@@YAXPBU.objc_cls_I@@@Z"
 
 void f(I &) {}
-// CHECK-LABEL: "?f@@YAXAAUI@@@Z"
+// CHECK-LABEL: "?f@@YAXAAU.objc_cls_I@@@Z"
 
 void f(const I &) {}
-// CHECK-LABEL: "?f@@YAXABUI@@@Z"
+// CHECK-LABEL: "?f@@YAXABU.objc_cls_I@@@Z"
 
 void f(const I &&) {}
-// CHECK-LABEL: "?f@@YAX$$QBUI@@@Z"
+// CHECK-LABEL: "?f@@YAX$$QBU.objc_cls_I@@@Z"
 
 void g(id) {}
-// CHECK-LABEL: "?g@@YAXPAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAXPAU.objc_object@@@Z"
 
 void g(id &) {}
-// CHECK-LABEL: "?g@@YAXAAPAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAXAAPAU.objc_object@@@Z"
 
 void g(const id &) {}
-// CHECK-LABEL: "?g@@YAXABQAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAXABQAU.objc_object@@@Z"
 
 void g(id &&) {}
-// CHECK-LABEL: "?g@@YAX$$QAPAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAX$$QAPAU.objc_object@@@Z"
 
 void h(Class) {}
-// CHECK-LABEL: "?h@@YAXPAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAXPAU.objc_class@@@Z"
 
 void h(Class &) {}
-// CHECK-LABEL: "?h@@YAXAAPAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAXAAPAU.objc_class@@@Z"
 
 void h(const Class &) {}
-// CHECK-LABEL: "?h@@YAXABQAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAXABQAU.objc_class@@@Z"
 
 void h(Class &&) {}
-// CHECK-LABEL: "?h@@YAX$$QAPAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAX$$QAPAU.objc_class@@@Z"
 
 I *i() { return nullptr; }
-// CHECK-LABEL: "?i@@YAPAUI@@XZ"
+// CHECK-LABEL: "?i@@YAPAU.objc_cls_I@@XZ"
 
 const I *j() { return nullptr; }
-// CHECK-LABEL: "?j@@YAPBUI@@XZ"
+// CHECK-LABEL: "?j@@YAPBU.objc_cls_I@@XZ"
 
 I &k() { return *kI; }
-// CHECK-LABEL: "?k@@YAAAUI@@XZ"
+// CHECK-LABEL: "?k@@YAAAU.objc_cls_I@@XZ"
 
 const I &l() { return *kI; }
-// CHECK-LABEL: "?l@@YAABUI@@XZ"
+// CHECK-LABEL: "?l@@YAABU.objc_cls_I@@XZ"
 
 void m(const id) {}
-// CHECK-LABEL: "?m@@YAXQAUobjc_object@@@Z"
+// CHECK-LABEL: "?m@@YAXQAU.objc_object@@@Z"
 
 void m(const I *) {}
-// CHECK-LABEL: "?m@@YAXPBUI@@@Z"
+// CHECK-LABEL: "?m@@YAXPBU.objc_cls_I@@@Z"
 
 void n(SEL) {}
-// CHECK-LABEL: "?n@@YAXPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXPAU.objc_selector@@@Z"
 
 void n(SEL *) {}
-// CHECK-LABEL: "?n@@YAXPAPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXPAPAU.objc_selector@@@Z"
 
 void n(const SEL *) {}
-// CHECK-LABEL: "?n@@YAXPBQAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXPBQAU.objc_selector@@@Z"
 
 void n(SEL &) {}
-// CHECK-LABEL: "?n@@YAXAAPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXAAPAU.objc_selector@@@Z"
 
 void n(const SEL &) {}
-// CHECK-LABEL: "?n@@YAXABQAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXABQAU.objc_selector@@@Z"
 
 void n(SEL &&) {}
-// CHECK-LABEL: "?n@@YAX$$QAPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAX$$QAPAU.objc_selector@@@Z"
 
 struct __declspec(dllexport) s {
   struct s &operator=(const struct s &) = delete;
 
   void m(I *) {}
-  // CHECK-LABEL: "?m@s@@QAAXPAUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXPAU.objc_cls_I@@@Z"
 
   void m(const I *) {}
-  // CHECK-LABEL: "?m@s@@QAAXPBUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXPBU.objc_cls_I@@@Z"
 
   void m(I &) {}
-  // CHECK-LABEL: "?m@s@@QAAXAAUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXAAU.objc_cls_I@@@Z"
 
   void m(const I &) {}
-  // CHECK-LABEL: "?m@s@@QAAXABUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXABU.objc_cls_I@@@Z"
 
   void m(I &&) {}
-  // CHECK-LABEL: "?m@s@@QAAX$$QAUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAX$$QAU.objc_cls_I@@@Z"
 
   void m(const I &&) {}
-  // CHECK-LABEL: "?m@s@@QAAX$$QBUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAX$$QBU.objc_cls_I@@@Z"
 
   void m(id) {}
-  // CHECK-LABEL: "?m@s@@QAAXPAUobj

[PATCH] D50144: Add Windows support for the GNUstep Objective-C ABI V2.

[David Chisnall via Phabricator via cfe-commits]

theraven updated this revision to Diff 159887.
theraven added a comment.

- Address Dustin's review comments.
- Fix an issue in protocol generation.
- Fix failing test.
- Address rjmcall's review comments.
- Add some missing comments and factor out SEH check.


Repository:
  rC Clang

https://reviews.llvm.org/D50144

Files:
  include/clang/Driver/Options.td
  lib/AST/MicrosoftMangle.cpp
  lib/CodeGen/CGException.cpp
  lib/CodeGen/CGObjCGNU.cpp
  lib/CodeGen/CGObjCRuntime.cpp
  lib/CodeGen/CGObjCRuntime.h
  lib/CodeGen/CodeGenFunction.h
  lib/Driver/ToolChains/Clang.cpp
  test/CodeGenObjC/gnu-init.m
  test/CodeGenObjC/gnustep2-proto.m
  test/CodeGenObjCXX/arc-marker-funclet.mm
  test/CodeGenObjCXX/microsoft-abi-arc-param-order.mm
  test/CodeGenObjCXX/msabi-objc-extensions.mm
  test/CodeGenObjCXX/msabi-objc-types.mm

Index: test/CodeGenObjCXX/msabi-objc-types.mm
===
--- test/CodeGenObjCXX/msabi-objc-types.mm
+++ test/CodeGenObjCXX/msabi-objc-types.mm
@@ -3,166 +3,166 @@
 @class I;
 
 id kid;
-// CHECK: @"?kid@@3PAUobjc_object@@A" =  dso_local global
+// CHECK: @"?kid@@3PAU.objc_object@@A" =  dso_local global
 
 Class klass;
-// CHECK: @"?klass@@3PAUobjc_class@@A" = dso_local global
+// CHECK: @"?klass@@3PAU.objc_class@@A" = dso_local global
 
 I *kI;
-// CHECK: @"?kI@@3PAUI@@A" = dso_local global
+// CHECK: @"?kI@@3PAU.objc_cls_I@@A" = dso_local global
 
 void f(I *) {}
-// CHECK-LABEL: "?f@@YAXPAUI@@@Z"
+// CHECK-LABEL: "?f@@YAXPAU.objc_cls_I@@@Z"
 
 void f(const I *) {}
-// CHECK-LABEL: "?f@@YAXPBUI@@@Z"
+// CHECK-LABEL: "?f@@YAXPBU.objc_cls_I@@@Z"
 
 void f(I &) {}
-// CHECK-LABEL: "?f@@YAXAAUI@@@Z"
+// CHECK-LABEL: "?f@@YAXAAU.objc_cls_I@@@Z"
 
 void f(const I &) {}
-// CHECK-LABEL: "?f@@YAXABUI@@@Z"
+// CHECK-LABEL: "?f@@YAXABU.objc_cls_I@@@Z"
 
 void f(const I &&) {}
-// CHECK-LABEL: "?f@@YAX$$QBUI@@@Z"
+// CHECK-LABEL: "?f@@YAX$$QBU.objc_cls_I@@@Z"
 
 void g(id) {}
-// CHECK-LABEL: "?g@@YAXPAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAXPAU.objc_object@@@Z"
 
 void g(id &) {}
-// CHECK-LABEL: "?g@@YAXAAPAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAXAAPAU.objc_object@@@Z"
 
 void g(const id &) {}
-// CHECK-LABEL: "?g@@YAXABQAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAXABQAU.objc_object@@@Z"
 
 void g(id &&) {}
-// CHECK-LABEL: "?g@@YAX$$QAPAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAX$$QAPAU.objc_object@@@Z"
 
 void h(Class) {}
-// CHECK-LABEL: "?h@@YAXPAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAXPAU.objc_class@@@Z"
 
 void h(Class &) {}
-// CHECK-LABEL: "?h@@YAXAAPAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAXAAPAU.objc_class@@@Z"
 
 void h(const Class &) {}
-// CHECK-LABEL: "?h@@YAXABQAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAXABQAU.objc_class@@@Z"
 
 void h(Class &&) {}
-// CHECK-LABEL: "?h@@YAX$$QAPAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAX$$QAPAU.objc_class@@@Z"
 
 I *i() { return nullptr; }
-// CHECK-LABEL: "?i@@YAPAUI@@XZ"
+// CHECK-LABEL: "?i@@YAPAU.objc_cls_I@@XZ"
 
 const I *j() { return nullptr; }
-// CHECK-LABEL: "?j@@YAPBUI@@XZ"
+// CHECK-LABEL: "?j@@YAPBU.objc_cls_I@@XZ"
 
 I &k() { return *kI; }
-// CHECK-LABEL: "?k@@YAAAUI@@XZ"
+// CHECK-LABEL: "?k@@YAAAU.objc_cls_I@@XZ"
 
 const I &l() { return *kI; }
-// CHECK-LABEL: "?l@@YAABUI@@XZ"
+// CHECK-LABEL: "?l@@YAABU.objc_cls_I@@XZ"
 
 void m(const id) {}
-// CHECK-LABEL: "?m@@YAXQAUobjc_object@@@Z"
+// CHECK-LABEL: "?m@@YAXQAU.objc_object@@@Z"
 
 void m(const I *) {}
-// CHECK-LABEL: "?m@@YAXPBUI@@@Z"
+// CHECK-LABEL: "?m@@YAXPBU.objc_cls_I@@@Z"
 
 void n(SEL) {}
-// CHECK-LABEL: "?n@@YAXPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXPAU.objc_selector@@@Z"
 
 void n(SEL *) {}
-// CHECK-LABEL: "?n@@YAXPAPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXPAPAU.objc_selector@@@Z"
 
 void n(const SEL *) {}
-// CHECK-LABEL: "?n@@YAXPBQAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXPBQAU.objc_selector@@@Z"
 
 void n(SEL &) {}
-// CHECK-LABEL: "?n@@YAXAAPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXAAPAU.objc_selector@@@Z"
 
 void n(const SEL &) {}
-// CHECK-LABEL: "?n@@YAXABQAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXABQAU.objc_selector@@@Z"
 
 void n(SEL &&) {}
-// CHECK-LABEL: "?n@@YAX$$QAPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAX$$QAPAU.objc_selector@@@Z"
 
 struct __declspec(dllexport) s {
   struct s &operator=(const struct s &) = delete;
 
   void m(I *) {}
-  // CHECK-LABEL: "?m@s@@QAAXPAUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXPAU.objc_cls_I@@@Z"
 
   void m(const I *) {}
-  // CHECK-LABEL: "?m@s@@QAAXPBUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXPBU.objc_cls_I@@@Z"
 
   void m(I &) {}
-  // CHECK-LABEL: "?m@s@@QAAXAAUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXAAU.objc_cls_I@@@Z"
 
   void m(const I &) {}
-  // CHECK-LABEL: "?m@s@@QAAXABUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXABU.objc_cls_I@@@Z"
 
   void m(I &&) {}
-  // CHECK-LABEL: "?m@s@@QAAX$$QAUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAX$$QAU.objc_cls_I@@@Z"
 
   void m(const I &&) {}
-  // CHECK-LABEL: "?m@s@@QAAX$$QBUI@@@Z"
+  // CHECK-LABEL: "?

[PATCH] D50144: Add Windows support for the GNUstep Objective-C ABI V2.

[David Chisnall via Phabricator via cfe-commits]

theraven marked an inline comment as done.
theraven added a comment.

I believe that this is now ready to land.




Comment at: lib/CodeGen/CGObjCGNU.cpp:915
+return name;
+  }
   /// The GCC ABI superclass message lookup function.  Takes a pointer to a

rjmccall wrote:
> theraven wrote:
> > rjmccall wrote:
> > > Can this section-names cleanup also just be in a separate patch?
> > This is difficult to extract, because it's mostly needed for the COFF part 
> > where we need to modify the section names.  For ELF, it was fine to keep 
> > them as separate `const char*`s
> I don't mind if the extracted patch seems unmotivated on its own, but I do 
> think it should be a separate patch.
None of the changes to do this are in a separate commit, and they touch enough 
of the code that it's very difficult to separate them without ending up with 
conflicts in this branch (which touches the code surrounding the changes in 
multiple places).  I cannot extract the code in such a way that this patch 
would cleanly rebase on top, because various things (e.g. the null section 
code) needed restructuring to work with Windows and touch this logic.



Comment at: lib/CodeGen/CGObjCGNU.cpp:3817
+  if (isRethrow && CGF.CGM.getTarget().getTriple().isWindowsMSVCEnvironment()) 
{
+CGF.EmitRuntimeCallOrInvoke(ExceptionReThrowFn).setDoesNotReturn();
+  }

rjmccall wrote:
> theraven wrote:
> > rjmccall wrote:
> > > You're sure here that the static information aligns with the dynamic?
> > I'm not sure I understand this question.
> Your new code path no longer uses `ExceptionAsObject`, which is our static 
> notion of the current exception value.  Instead, you call a runtime function 
> which presumably relies on a dynamically-stored current exception value.  I'm 
> just asking if, in this lexical position, you're definitely rethrowing the 
> right exception.
Ah, I see.  This code path is hit only as a `@throw;`, not a `@throw obj` (in 
the latter case, there is a non-null return from `S.getThrowExpr()`).  In this 
case, the value of ExceptionAsObject may not be useful. In the case of a 
catchall, this is a load from a stack location with no corresponding store.  
The Windows unwind logic keeps the object on the stack during funclet execution 
and then continues the unwind at the end, without ever providing this frame's 
funclets with a pointer to it.  That's probably not very obvious, so I've added 
an explanatory comment.



Comment at: lib/CodeGen/CGObjCGNU.cpp:3542
+  allSelectors.push_back(entry.first);
+std::sort(allSelectors.begin(), allSelectors.end());
 

rjmccall wrote:
> mgrang wrote:
> > Please use llvm::sort instead of std::sort. See 
> > https://llvm.org/docs/CodingStandards.html#beware-of-non-deterministic-sorting-order-of-equal-elements.
> Also, why is the sort necessary?  Should this change be separately committed 
> and tested?
Ooops, this was a fix for PR35277, which was meant to be a separate review.  I 
forgot to remove it from this branch after cherry-picking it to another one.  
Removed.


Repository:
  rC Clang

https://reviews.llvm.org/D50144



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

[PATCH] D50144: Add Windows support for the GNUstep Objective-C ABI V2.

[David Chisnall via Phabricator via cfe-commits]

theraven updated this revision to Diff 160091.
theraven added a comment.

Squashed into a single commit.


Repository:
  rC Clang

https://reviews.llvm.org/D50144

Files:
  include/clang/Driver/Options.td
  lib/AST/MicrosoftMangle.cpp
  lib/CodeGen/CGException.cpp
  lib/CodeGen/CGObjCGNU.cpp
  lib/CodeGen/CGObjCRuntime.cpp
  lib/CodeGen/CGObjCRuntime.h
  lib/CodeGen/CodeGenFunction.h
  lib/Driver/ToolChains/Clang.cpp
  test/CodeGenObjC/gnu-init.m
  test/CodeGenObjC/gnustep2-proto.m
  test/CodeGenObjCXX/arc-marker-funclet.mm
  test/CodeGenObjCXX/microsoft-abi-arc-param-order.mm
  test/CodeGenObjCXX/msabi-objc-extensions.mm
  test/CodeGenObjCXX/msabi-objc-types.mm

Index: test/CodeGenObjCXX/msabi-objc-types.mm
===
--- test/CodeGenObjCXX/msabi-objc-types.mm
+++ test/CodeGenObjCXX/msabi-objc-types.mm
@@ -3,166 +3,166 @@
 @class I;
 
 id kid;
-// CHECK: @"?kid@@3PAUobjc_object@@A" =  dso_local global
+// CHECK: @"?kid@@3PAU.objc_object@@A" =  dso_local global
 
 Class klass;
-// CHECK: @"?klass@@3PAUobjc_class@@A" = dso_local global
+// CHECK: @"?klass@@3PAU.objc_class@@A" = dso_local global
 
 I *kI;
-// CHECK: @"?kI@@3PAUI@@A" = dso_local global
+// CHECK: @"?kI@@3PAU.objc_cls_I@@A" = dso_local global
 
 void f(I *) {}
-// CHECK-LABEL: "?f@@YAXPAUI@@@Z"
+// CHECK-LABEL: "?f@@YAXPAU.objc_cls_I@@@Z"
 
 void f(const I *) {}
-// CHECK-LABEL: "?f@@YAXPBUI@@@Z"
+// CHECK-LABEL: "?f@@YAXPBU.objc_cls_I@@@Z"
 
 void f(I &) {}
-// CHECK-LABEL: "?f@@YAXAAUI@@@Z"
+// CHECK-LABEL: "?f@@YAXAAU.objc_cls_I@@@Z"
 
 void f(const I &) {}
-// CHECK-LABEL: "?f@@YAXABUI@@@Z"
+// CHECK-LABEL: "?f@@YAXABU.objc_cls_I@@@Z"
 
 void f(const I &&) {}
-// CHECK-LABEL: "?f@@YAX$$QBUI@@@Z"
+// CHECK-LABEL: "?f@@YAX$$QBU.objc_cls_I@@@Z"
 
 void g(id) {}
-// CHECK-LABEL: "?g@@YAXPAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAXPAU.objc_object@@@Z"
 
 void g(id &) {}
-// CHECK-LABEL: "?g@@YAXAAPAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAXAAPAU.objc_object@@@Z"
 
 void g(const id &) {}
-// CHECK-LABEL: "?g@@YAXABQAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAXABQAU.objc_object@@@Z"
 
 void g(id &&) {}
-// CHECK-LABEL: "?g@@YAX$$QAPAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAX$$QAPAU.objc_object@@@Z"
 
 void h(Class) {}
-// CHECK-LABEL: "?h@@YAXPAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAXPAU.objc_class@@@Z"
 
 void h(Class &) {}
-// CHECK-LABEL: "?h@@YAXAAPAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAXAAPAU.objc_class@@@Z"
 
 void h(const Class &) {}
-// CHECK-LABEL: "?h@@YAXABQAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAXABQAU.objc_class@@@Z"
 
 void h(Class &&) {}
-// CHECK-LABEL: "?h@@YAX$$QAPAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAX$$QAPAU.objc_class@@@Z"
 
 I *i() { return nullptr; }
-// CHECK-LABEL: "?i@@YAPAUI@@XZ"
+// CHECK-LABEL: "?i@@YAPAU.objc_cls_I@@XZ"
 
 const I *j() { return nullptr; }
-// CHECK-LABEL: "?j@@YAPBUI@@XZ"
+// CHECK-LABEL: "?j@@YAPBU.objc_cls_I@@XZ"
 
 I &k() { return *kI; }
-// CHECK-LABEL: "?k@@YAAAUI@@XZ"
+// CHECK-LABEL: "?k@@YAAAU.objc_cls_I@@XZ"
 
 const I &l() { return *kI; }
-// CHECK-LABEL: "?l@@YAABUI@@XZ"
+// CHECK-LABEL: "?l@@YAABU.objc_cls_I@@XZ"
 
 void m(const id) {}
-// CHECK-LABEL: "?m@@YAXQAUobjc_object@@@Z"
+// CHECK-LABEL: "?m@@YAXQAU.objc_object@@@Z"
 
 void m(const I *) {}
-// CHECK-LABEL: "?m@@YAXPBUI@@@Z"
+// CHECK-LABEL: "?m@@YAXPBU.objc_cls_I@@@Z"
 
 void n(SEL) {}
-// CHECK-LABEL: "?n@@YAXPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXPAU.objc_selector@@@Z"
 
 void n(SEL *) {}
-// CHECK-LABEL: "?n@@YAXPAPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXPAPAU.objc_selector@@@Z"
 
 void n(const SEL *) {}
-// CHECK-LABEL: "?n@@YAXPBQAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXPBQAU.objc_selector@@@Z"
 
 void n(SEL &) {}
-// CHECK-LABEL: "?n@@YAXAAPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXAAPAU.objc_selector@@@Z"
 
 void n(const SEL &) {}
-// CHECK-LABEL: "?n@@YAXABQAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXABQAU.objc_selector@@@Z"
 
 void n(SEL &&) {}
-// CHECK-LABEL: "?n@@YAX$$QAPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAX$$QAPAU.objc_selector@@@Z"
 
 struct __declspec(dllexport) s {
   struct s &operator=(const struct s &) = delete;
 
   void m(I *) {}
-  // CHECK-LABEL: "?m@s@@QAAXPAUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXPAU.objc_cls_I@@@Z"
 
   void m(const I *) {}
-  // CHECK-LABEL: "?m@s@@QAAXPBUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXPBU.objc_cls_I@@@Z"
 
   void m(I &) {}
-  // CHECK-LABEL: "?m@s@@QAAXAAUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXAAU.objc_cls_I@@@Z"
 
   void m(const I &) {}
-  // CHECK-LABEL: "?m@s@@QAAXABUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXABU.objc_cls_I@@@Z"
 
   void m(I &&) {}
-  // CHECK-LABEL: "?m@s@@QAAX$$QAUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAX$$QAU.objc_cls_I@@@Z"
 
   void m(const I &&) {}
-  // CHECK-LABEL: "?m@s@@QAAX$$QBUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAX$$QBU.objc_cls_I@@@Z"
 
   void m(id) {}
-  // CHECK-LABEL: "?m@s@@QAAXPAUobjc_object@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXPAU.objc_object@@@Z"
 
  

[PATCH] D50144: Add Windows support for the GNUstep Objective-C ABI V2.

[David Chisnall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC339428: Add Windows support for the GNUstep Objective-C ABI 
V2. (authored by theraven, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D50144?vs=160091&id=160095#toc

Repository:
  rC Clang

https://reviews.llvm.org/D50144

Files:
  include/clang/Driver/Options.td
  lib/AST/MicrosoftMangle.cpp
  lib/CodeGen/CGException.cpp
  lib/CodeGen/CGObjCGNU.cpp
  lib/CodeGen/CGObjCRuntime.cpp
  lib/CodeGen/CGObjCRuntime.h
  lib/CodeGen/CodeGenFunction.h
  lib/Driver/ToolChains/Clang.cpp
  test/CodeGenObjC/gnu-init.m
  test/CodeGenObjC/gnustep2-proto.m
  test/CodeGenObjCXX/arc-marker-funclet.mm
  test/CodeGenObjCXX/microsoft-abi-arc-param-order.mm
  test/CodeGenObjCXX/msabi-objc-extensions.mm
  test/CodeGenObjCXX/msabi-objc-types.mm

Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1488,7 +1488,7 @@
   HelpText<"Enable ARC-style weak references in Objective-C">;
 
 // Objective-C ABI options.
-def fobjc_runtime_EQ : Joined<["-"], "fobjc-runtime=">, Group, Flags<[CC1Option]>,
+def fobjc_runtime_EQ : Joined<["-"], "fobjc-runtime=">, Group, Flags<[CC1Option, CoreOption]>,
   HelpText<"Specify the target Objective-C runtime kind and version">;
 def fobjc_abi_version_EQ : Joined<["-"], "fobjc-abi-version=">, Group;
 def fobjc_nonfragile_abi_version_EQ : Joined<["-"], "fobjc-nonfragile-abi-version=">, Group;
Index: test/CodeGenObjCXX/microsoft-abi-arc-param-order.mm
===
--- test/CodeGenObjCXX/microsoft-abi-arc-param-order.mm
+++ test/CodeGenObjCXX/microsoft-abi-arc-param-order.mm
@@ -9,7 +9,7 @@
 
 // Verify that we destruct things from left to right in the MS C++ ABI: a, b, c, d.
 //
-// CHECK-LABEL: define dso_local void @"?test_arc_order@@YAXUA@@PAUobjc_object@@01@Z"
+// CHECK-LABEL: define dso_local void @"?test_arc_order@@YAXUA@@PAU.objc_object@@01@Z"
 // CHECK:   (<{ %struct.A, i8*, %struct.A, i8* }>* inalloca)
 void test_arc_order(A a, id __attribute__((ns_consumed)) b , A c, id __attribute__((ns_consumed)) d) {
   // CHECK: call x86_thiscallcc void @"??1A@@QAE@XZ"(%struct.A* %{{.*}})
Index: test/CodeGenObjCXX/msabi-objc-types.mm
===
--- test/CodeGenObjCXX/msabi-objc-types.mm
+++ test/CodeGenObjCXX/msabi-objc-types.mm
@@ -3,166 +3,166 @@
 @class I;
 
 id kid;
-// CHECK: @"?kid@@3PAUobjc_object@@A" =  dso_local global
+// CHECK: @"?kid@@3PAU.objc_object@@A" =  dso_local global
 
 Class klass;
-// CHECK: @"?klass@@3PAUobjc_class@@A" = dso_local global
+// CHECK: @"?klass@@3PAU.objc_class@@A" = dso_local global
 
 I *kI;
-// CHECK: @"?kI@@3PAUI@@A" = dso_local global
+// CHECK: @"?kI@@3PAU.objc_cls_I@@A" = dso_local global
 
 void f(I *) {}
-// CHECK-LABEL: "?f@@YAXPAUI@@@Z"
+// CHECK-LABEL: "?f@@YAXPAU.objc_cls_I@@@Z"
 
 void f(const I *) {}
-// CHECK-LABEL: "?f@@YAXPBUI@@@Z"
+// CHECK-LABEL: "?f@@YAXPBU.objc_cls_I@@@Z"
 
 void f(I &) {}
-// CHECK-LABEL: "?f@@YAXAAUI@@@Z"
+// CHECK-LABEL: "?f@@YAXAAU.objc_cls_I@@@Z"
 
 void f(const I &) {}
-// CHECK-LABEL: "?f@@YAXABUI@@@Z"
+// CHECK-LABEL: "?f@@YAXABU.objc_cls_I@@@Z"
 
 void f(const I &&) {}
-// CHECK-LABEL: "?f@@YAX$$QBUI@@@Z"
+// CHECK-LABEL: "?f@@YAX$$QBU.objc_cls_I@@@Z"
 
 void g(id) {}
-// CHECK-LABEL: "?g@@YAXPAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAXPAU.objc_object@@@Z"
 
 void g(id &) {}
-// CHECK-LABEL: "?g@@YAXAAPAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAXAAPAU.objc_object@@@Z"
 
 void g(const id &) {}
-// CHECK-LABEL: "?g@@YAXABQAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAXABQAU.objc_object@@@Z"
 
 void g(id &&) {}
-// CHECK-LABEL: "?g@@YAX$$QAPAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAX$$QAPAU.objc_object@@@Z"
 
 void h(Class) {}
-// CHECK-LABEL: "?h@@YAXPAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAXPAU.objc_class@@@Z"
 
 void h(Class &) {}
-// CHECK-LABEL: "?h@@YAXAAPAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAXAAPAU.objc_class@@@Z"
 
 void h(const Class &) {}
-// CHECK-LABEL: "?h@@YAXABQAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAXABQAU.objc_class@@@Z"
 
 void h(Class &&) {}
-// CHECK-LABEL: "?h@@YAX$$QAPAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAX$$QAPAU.objc_class@@@Z"
 
 I *i() { return nullptr; }
-// CHECK-LABEL: "?i@@YAPAUI@@XZ"
+// CHECK-LABEL: "?i@@YAPAU.objc_cls_I@@XZ"
 
 const I *j() { return nullptr; }
-// CHECK-LABEL: "?j@@YAPBUI@@XZ"
+// CHECK-LABEL: "?j@@YAPBU.objc_cls_I@@XZ"
 
 I &k() { return *kI; }
-// CHECK-LABEL: "?k@@YAAAUI@@XZ"
+// CHECK-LABEL: "?k@@YAAAU.objc_cls_I@@XZ"
 
 const I &l() { return *kI; }
-// CHECK-LABEL: "?l@@YAABUI@@XZ"
+// CHECK-LABEL: "?l@@YAABU.objc_cls_I@@XZ"
 
 void m(const id) {}
-// CHECK-LABEL: "?m@@YAXQAUobjc_object@@@Z"
+// CHECK-LABEL: "?m@@YAXQAU.objc_object@@@Z"
 
 void m(const I *) {}
-// CHECK-LABEL: "?m@@YAXPBUI@@@Z"

[PATCH] D50559: [gnu-objc] Make selector order deterministic.

[David Chisnall via Phabricator via cfe-commits]

theraven created this revision.
theraven added a reviewer: rjmccall.
Herald added subscribers: cfe-commits, mgrang.

This probably fixes PR35277, though there may be other sources of
nondeterminism (this was the only case of iterating over a DenseMap).

It's difficult to provide a test case for this, because it shows up only
on systems with ASLR enabled.


Repository:
  rC Clang

https://reviews.llvm.org/D50559

Files:
  lib/CodeGen/CGObjCGNU.cpp


Index: lib/CodeGen/CGObjCGNU.cpp
===
--- lib/CodeGen/CGObjCGNU.cpp
+++ lib/CodeGen/CGObjCGNU.cpp
@@ -3541,12 +3541,16 @@
 ConstantInitBuilder builder(CGM);
 auto selectors = builder.beginArray(selStructTy);
 auto &table = SelectorTable; // MSVC workaround
-for (auto &entry : table) {
+std::vector allSelectors;
+for (auto &entry : table)
+  allSelectors.push_back(entry.first);
+llvm::sort(allSelectors.begin(), allSelectors.end());
 
-  std::string selNameStr = entry.first.getAsString();
+for (auto &untypedSel : allSelectors) {
+  std::string selNameStr = untypedSel.getAsString();
   llvm::Constant *selName = ExportUniqueString(selNameStr, 
".objc_sel_name");
 
-  for (TypedSelector &sel : entry.second) {
+  for (TypedSelector &sel : table[untypedSel]) {
 llvm::Constant *selectorTypeEncoding = NULLPtr;
 if (!sel.first.empty())
   selectorTypeEncoding =


Index: lib/CodeGen/CGObjCGNU.cpp
===
--- lib/CodeGen/CGObjCGNU.cpp
+++ lib/CodeGen/CGObjCGNU.cpp
@@ -3541,12 +3541,16 @@
 ConstantInitBuilder builder(CGM);
 auto selectors = builder.beginArray(selStructTy);
 auto &table = SelectorTable; // MSVC workaround
-for (auto &entry : table) {
+std::vector allSelectors;
+for (auto &entry : table)
+  allSelectors.push_back(entry.first);
+llvm::sort(allSelectors.begin(), allSelectors.end());
 
-  std::string selNameStr = entry.first.getAsString();
+for (auto &untypedSel : allSelectors) {
+  std::string selNameStr = untypedSel.getAsString();
   llvm::Constant *selName = ExportUniqueString(selNameStr, ".objc_sel_name");
 
-  for (TypedSelector &sel : entry.second) {
+  for (TypedSelector &sel : table[untypedSel]) {
 llvm::Constant *selectorTypeEncoding = NULLPtr;
 if (!sel.first.empty())
   selectorTypeEncoding =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50559: [gnu-objc] Make selector order deterministic.

[David Chisnall via Phabricator via cfe-commits]

theraven added inline comments.



Comment at: lib/CodeGen/CGObjCGNU.cpp:3547
+  allSelectors.push_back(entry.first);
+llvm::sort(allSelectors.begin(), allSelectors.end());
 

bmwiedemann wrote:
> compilation failed here:
> ../tools/clang/lib/CodeGen/CGObjCGNU.cpp:2444:11: error: 'sort' is not a 
> member of 'llvm'
> 
> it suggested std::sort
I'm not sure `llvm::sort` was part of the 6.0 release, it was added in April 
and so should be in 7.0.  I don't expect a patch against the 8 branch to apply 
cleanly to 6...


Repository:
  rC Clang

https://reviews.llvm.org/D50559



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

[PATCH] D50559: [gnu-objc] Make selector order deterministic.

[David Chisnall via Phabricator via cfe-commits]

theraven updated this revision to Diff 160312.
theraven added a comment.

- Add a test case.


Repository:
  rC Clang

https://reviews.llvm.org/D50559

Files:
  lib/CodeGen/CGObjCGNU.cpp
  test/CodeGenObjC/gnu-deterministic-selectors.m


Index: test/CodeGenObjC/gnu-deterministic-selectors.m
===
--- /dev/null
+++ test/CodeGenObjC/gnu-deterministic-selectors.m
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -fobjc-runtime=gnustep-1.5 
%s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -fobjc-runtime=gcc %s 
-emit-llvm -o - | FileCheck %s
+
+// Check that these selectors are emitted in alphabetical order.
+// The order doesn't actually matter, only that it doesn't vary across runs.
+// Clang sorts them when targeting a GCC-like ABI to guarantee determinism.
+// CHECK: @.objc_selector_list = internal global [6 x { i8*, i8* }] [{ i8*, 
i8* } { i8* getelementptr inbounds ([2 x i8], [2 x i8]* @.objc_sel_namea, i64 
0, i64 0), i8* null }, { i8*, i8* } { i8* getelementptr inbounds ([2 x i8], [2 
x i8]* @.objc_sel_nameg, i64 0, i64 0), i8* null }, { i8*, i8* } { i8* 
getelementptr inbounds ([2 x i8], [2 x i8]* @.objc_sel_namej, i64 0, i64 0), 
i8* null }, { i8*, i8* } { i8* getelementptr inbounds ([2 x i8], [2 x i8]* 
@.objc_sel_namel, i64 0, i64 0), i8* null }, { i8*, i8* } { i8* getelementptr 
inbounds ([2 x i8], [2 x i8]* @.objc_sel_namez, i64 0, i64 0), i8* null }, { 
i8*, i8* } zeroinitializer], align 8
+
+
+void f() {
+   SEL a = @selector(z);
+   SEL b = @selector(a);
+   SEL c = @selector(g);
+   SEL d = @selector(l);
+   SEL e = @selector(j);
+}
Index: lib/CodeGen/CGObjCGNU.cpp
===
--- lib/CodeGen/CGObjCGNU.cpp
+++ lib/CodeGen/CGObjCGNU.cpp
@@ -3541,12 +3541,16 @@
 ConstantInitBuilder builder(CGM);
 auto selectors = builder.beginArray(selStructTy);
 auto &table = SelectorTable; // MSVC workaround
-for (auto &entry : table) {
+std::vector allSelectors;
+for (auto &entry : table)
+  allSelectors.push_back(entry.first);
+llvm::sort(allSelectors.begin(), allSelectors.end());
 
-  std::string selNameStr = entry.first.getAsString();
+for (auto &untypedSel : allSelectors) {
+  std::string selNameStr = untypedSel.getAsString();
   llvm::Constant *selName = ExportUniqueString(selNameStr, 
".objc_sel_name");
 
-  for (TypedSelector &sel : entry.second) {
+  for (TypedSelector &sel : table[untypedSel]) {
 llvm::Constant *selectorTypeEncoding = NULLPtr;
 if (!sel.first.empty())
   selectorTypeEncoding =


Index: test/CodeGenObjC/gnu-deterministic-selectors.m
===
--- /dev/null
+++ test/CodeGenObjC/gnu-deterministic-selectors.m
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -fobjc-runtime=gnustep-1.5 %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -fobjc-runtime=gcc %s -emit-llvm -o - | FileCheck %s
+
+// Check that these selectors are emitted in alphabetical order.
+// The order doesn't actually matter, only that it doesn't vary across runs.
+// Clang sorts them when targeting a GCC-like ABI to guarantee determinism.
+// CHECK: @.objc_selector_list = internal global [6 x { i8*, i8* }] [{ i8*, i8* } { i8* getelementptr inbounds ([2 x i8], [2 x i8]* @.objc_sel_namea, i64 0, i64 0), i8* null }, { i8*, i8* } { i8* getelementptr inbounds ([2 x i8], [2 x i8]* @.objc_sel_nameg, i64 0, i64 0), i8* null }, { i8*, i8* } { i8* getelementptr inbounds ([2 x i8], [2 x i8]* @.objc_sel_namej, i64 0, i64 0), i8* null }, { i8*, i8* } { i8* getelementptr inbounds ([2 x i8], [2 x i8]* @.objc_sel_namel, i64 0, i64 0), i8* null }, { i8*, i8* } { i8* getelementptr inbounds ([2 x i8], [2 x i8]* @.objc_sel_namez, i64 0, i64 0), i8* null }, { i8*, i8* } zeroinitializer], align 8
+
+
+void f() {
+	SEL a = @selector(z);
+	SEL b = @selector(a);
+	SEL c = @selector(g);
+	SEL d = @selector(l);
+	SEL e = @selector(j);
+}
Index: lib/CodeGen/CGObjCGNU.cpp
===
--- lib/CodeGen/CGObjCGNU.cpp
+++ lib/CodeGen/CGObjCGNU.cpp
@@ -3541,12 +3541,16 @@
 ConstantInitBuilder builder(CGM);
 auto selectors = builder.beginArray(selStructTy);
 auto &table = SelectorTable; // MSVC workaround
-for (auto &entry : table) {
+std::vector allSelectors;
+for (auto &entry : table)
+  allSelectors.push_back(entry.first);
+llvm::sort(allSelectors.begin(), allSelectors.end());
 
-  std::string selNameStr = entry.first.getAsString();
+for (auto &untypedSel : allSelectors) {
+  std::string selNameStr = untypedSel.getAsString();
   llvm::Constant *selName = ExportUniqueString(selNameStr, ".objc_sel_name");
 
-  for (TypedSelector &sel : entry.second) {
+  for (TypedSelec

[PATCH] D50559: [gnu-objc] Make selector order deterministic.

[David Chisnall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC339668: [gnu-objc] Make selector order deterministic. 
(authored by theraven, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D50559?vs=160312&id=160545#toc

Repository:
  rC Clang

https://reviews.llvm.org/D50559

Files:
  lib/CodeGen/CGObjCGNU.cpp
  test/CodeGenObjC/gnu-deterministic-selectors.m


Index: test/CodeGenObjC/gnu-deterministic-selectors.m
===
--- test/CodeGenObjC/gnu-deterministic-selectors.m
+++ test/CodeGenObjC/gnu-deterministic-selectors.m
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -fobjc-runtime=gnustep-1.5 
%s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -fobjc-runtime=gcc %s 
-emit-llvm -o - | FileCheck %s
+
+// Check that these selectors are emitted in alphabetical order.
+// The order doesn't actually matter, only that it doesn't vary across runs.
+// Clang sorts them when targeting a GCC-like ABI to guarantee determinism.
+// CHECK: @.objc_selector_list = internal global [6 x { i8*, i8* }] [{ i8*, 
i8* } { i8* getelementptr inbounds ([2 x i8], [2 x i8]* @.objc_sel_namea, i64 
0, i64 0), i8* null }, { i8*, i8* } { i8* getelementptr inbounds ([2 x i8], [2 
x i8]* @.objc_sel_nameg, i64 0, i64 0), i8* null }, { i8*, i8* } { i8* 
getelementptr inbounds ([2 x i8], [2 x i8]* @.objc_sel_namej, i64 0, i64 0), 
i8* null }, { i8*, i8* } { i8* getelementptr inbounds ([2 x i8], [2 x i8]* 
@.objc_sel_namel, i64 0, i64 0), i8* null }, { i8*, i8* } { i8* getelementptr 
inbounds ([2 x i8], [2 x i8]* @.objc_sel_namez, i64 0, i64 0), i8* null }, { 
i8*, i8* } zeroinitializer], align 8
+
+
+void f() {
+   SEL a = @selector(z);
+   SEL b = @selector(a);
+   SEL c = @selector(g);
+   SEL d = @selector(l);
+   SEL e = @selector(j);
+}
Index: lib/CodeGen/CGObjCGNU.cpp
===
--- lib/CodeGen/CGObjCGNU.cpp
+++ lib/CodeGen/CGObjCGNU.cpp
@@ -3541,12 +3541,16 @@
 ConstantInitBuilder builder(CGM);
 auto selectors = builder.beginArray(selStructTy);
 auto &table = SelectorTable; // MSVC workaround
-for (auto &entry : table) {
+std::vector allSelectors;
+for (auto &entry : table)
+  allSelectors.push_back(entry.first);
+llvm::sort(allSelectors.begin(), allSelectors.end());
 
-  std::string selNameStr = entry.first.getAsString();
+for (auto &untypedSel : allSelectors) {
+  std::string selNameStr = untypedSel.getAsString();
   llvm::Constant *selName = ExportUniqueString(selNameStr, 
".objc_sel_name");
 
-  for (TypedSelector &sel : entry.second) {
+  for (TypedSelector &sel : table[untypedSel]) {
 llvm::Constant *selectorTypeEncoding = NULLPtr;
 if (!sel.first.empty())
   selectorTypeEncoding =


Index: test/CodeGenObjC/gnu-deterministic-selectors.m
===
--- test/CodeGenObjC/gnu-deterministic-selectors.m
+++ test/CodeGenObjC/gnu-deterministic-selectors.m
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -fobjc-runtime=gnustep-1.5 %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -fobjc-runtime=gcc %s -emit-llvm -o - | FileCheck %s
+
+// Check that these selectors are emitted in alphabetical order.
+// The order doesn't actually matter, only that it doesn't vary across runs.
+// Clang sorts them when targeting a GCC-like ABI to guarantee determinism.
+// CHECK: @.objc_selector_list = internal global [6 x { i8*, i8* }] [{ i8*, i8* } { i8* getelementptr inbounds ([2 x i8], [2 x i8]* @.objc_sel_namea, i64 0, i64 0), i8* null }, { i8*, i8* } { i8* getelementptr inbounds ([2 x i8], [2 x i8]* @.objc_sel_nameg, i64 0, i64 0), i8* null }, { i8*, i8* } { i8* getelementptr inbounds ([2 x i8], [2 x i8]* @.objc_sel_namej, i64 0, i64 0), i8* null }, { i8*, i8* } { i8* getelementptr inbounds ([2 x i8], [2 x i8]* @.objc_sel_namel, i64 0, i64 0), i8* null }, { i8*, i8* } { i8* getelementptr inbounds ([2 x i8], [2 x i8]* @.objc_sel_namez, i64 0, i64 0), i8* null }, { i8*, i8* } zeroinitializer], align 8
+
+
+void f() {
+	SEL a = @selector(z);
+	SEL b = @selector(a);
+	SEL c = @selector(g);
+	SEL d = @selector(l);
+	SEL e = @selector(j);
+}
Index: lib/CodeGen/CGObjCGNU.cpp
===
--- lib/CodeGen/CGObjCGNU.cpp
+++ lib/CodeGen/CGObjCGNU.cpp
@@ -3541,12 +3541,16 @@
 ConstantInitBuilder builder(CGM);
 auto selectors = builder.beginArray(selStructTy);
 auto &table = SelectorTable; // MSVC workaround
-for (auto &entry : table) {
+std::vector allSelectors;
+for (auto &entry : table)
+  allSelectors.push_back(entry.first);
+llvm::sort(allSelectors.begin(), allSelectors.end());
 
-  std::string selNameStr = entry.first.getAsString();
+for

[PATCH] D50144: Add Windows support for the GNUstep Objective-C ABI V2.

[David Chisnall via Phabricator via cfe-commits]

theraven added inline comments.



Comment at: lib/AST/MicrosoftMangle.cpp:1886
   case BuiltinType::ObjCId:
-mangleArtificalTagType(TTK_Struct, "objc_object");
+mangleArtificalTagType(TTK_Struct, ".objc_object");
 break;

compnerd wrote:
> DHowett-MSFT wrote:
> > I'm interested in @smeenai's take on this, as well.
> Hmm, doesn't this break ObjC/ObjC++ interoperability?  I don't think that we 
> should be changing the decoration here for the MS ABI case.
No, this fixes ObjC++ interop.  Throwing an exception from a `@throw` and 
catching it with `catch` now works and we avoid the problem that a C++ 
compilation unit declares a `struct` that happens to have the same name as an 
Objective-C class (which is very common for wrapper code) may catch things of 
the wrong type.


Repository:
  rC Clang

https://reviews.llvm.org/D50144



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

[PATCH] D50144: Add Windows support for the GNUstep Objective-C ABI V2.

[David Chisnall via Phabricator via cfe-commits]

theraven added inline comments.



Comment at: lib/AST/MicrosoftMangle.cpp:1886
   case BuiltinType::ObjCId:
-mangleArtificalTagType(TTK_Struct, "objc_object");
+mangleArtificalTagType(TTK_Struct, ".objc_object");
 break;

smeenai wrote:
> theraven wrote:
> > compnerd wrote:
> > > DHowett-MSFT wrote:
> > > > I'm interested in @smeenai's take on this, as well.
> > > Hmm, doesn't this break ObjC/ObjC++ interoperability?  I don't think that 
> > > we should be changing the decoration here for the MS ABI case.
> > No, this fixes ObjC++ interop.  Throwing an exception from a `@throw` and 
> > catching it with `catch` now works and we avoid the problem that a C++ 
> > compilation unit declares a `struct` that happens to have the same name as 
> > an Objective-C class (which is very common for wrapper code) may catch 
> > things of the wrong type.
> I've seen a lot of code which does something like this in a header
> 
> ```lang=cpp
> #ifdef __OBJC__
> @class I;
> #else
> struct I;
> #endif
> 
> void f(I *);
> ```
> 
> You want `f` to be mangled consistently across C++ and Objective-C++, 
> otherwise you'll get link errors if `f` is e.g. defined in a C++ TU and 
> referenced in an Objective-C++ TU. This was the case before your change and 
> isn't the case after your change, which is what @compnerd was pointing out. 
> They are mangled consistently in the Itanium ABI, and we want them to be 
> mangled consistently in the MS ABI as well.
> 
> Not wanting the catch mix-up is completely reasonable, of course, but it can 
> be done in a more targeted way. I'll put up a patch that undoes this part of 
> the change while still preventing incorrect catching.
With a non-fragile ABI, there is not guarantee that `struct I` and `@class I` 
have the same layout.  This is why `@defs` has gone away.  Any code that does 
this and treats `struct I` as anything other than an opaque type is broken.  No 
other platform supports throwing an Objective-C object and catching it as a 
struct, and this is an intentional design decision.  I would be strongly 
opposed to a change that intentionally supported this, because it is breaking 
correct code in order to make incorrect code do something that may or may not 
work.


Repository:
  rC Clang

https://reviews.llvm.org/D50144



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

[PATCH] D50144: Add Windows support for the GNUstep Objective-C ABI V2.

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

In https://reviews.llvm.org/D50144#1209758, @smeenai wrote:

> I'm confused by the funclet-based unwinding support. Do you intend GNUStep to 
> be used with windows-msvc triples? If so, how is the runtime throwing 
> exceptions? We took the approach of having the Obj-C runtime wrap Obj-C 
> exceptions in C++ exceptions and then have vcruntime handle the rest (see 
> https://reviews.llvm.org/D47233 and https://reviews.llvm.org/D47988), but I 
> don't see any of the associated RTTI emission changes in this patch.


I'm assuming 'we' here is Apple?  Microsoft has a friendly fork of the GNUstep 
runtime in the WinObjC project and I am slowly working on pulling in the 
WinObjC downstream changes, incorporating them with the v2 ABI, and making it 
easy for WinObjC to use an unmodified version of the runtime.  The v2 ABI still 
has a few things that don't work with the Windows linkage model, which I'm 
working out how best to address.  The code for throwing an exception is here:

https://github.com/gnustep/libobjc2/blob/master/eh_win32_msvc.cc

The runtime constructs an SEH exception on the stack.  The low-level NT 
exception type knows all of the possible types that might be allowed in a 
catch, so the runtime dynamically constructs a list of all of these, one per 
superclass in the hierarchy, on the stack.  The funclet-based unwinding 
mechanism makes it safe to do this, because the throwing stack frame remains 
valid until after the exception is caught.  The Windows C++ personality 
function then checks the catches the exceptions.  Clang/CX (Clang with the 
Visual Studio compiler back end) has supported this for a while, but it never 
made it upstream to Clang/LLVM and I'm trying to rectify this.

On 64-bit Windows, we must provide 32-bit offsets to some arbitrary base.  For 
C++, this is the base load address of the DLL.  That's fine for C++, where the 
throw types are static (and the entire class hierarchy is available at compile 
time), but our stack may be more than 4GB away from any library.  Instead, we 
use an offset from some arbitrary on-stack location (and if we end up being 
more than 4GB away from the base of our stack frame, then something's gone very 
badly wrong!).




Comment at: lib/AST/MicrosoftMangle.cpp:1886
   case BuiltinType::ObjCId:
-mangleArtificalTagType(TTK_Struct, "objc_object");
+mangleArtificalTagType(TTK_Struct, ".objc_object");
 break;

smeenai wrote:
> theraven wrote:
> > smeenai wrote:
> > > theraven wrote:
> > > > compnerd wrote:
> > > > > DHowett-MSFT wrote:
> > > > > > I'm interested in @smeenai's take on this, as well.
> > > > > Hmm, doesn't this break ObjC/ObjC++ interoperability?  I don't think 
> > > > > that we should be changing the decoration here for the MS ABI case.
> > > > No, this fixes ObjC++ interop.  Throwing an exception from a `@throw` 
> > > > and catching it with `catch` now works and we avoid the problem that a 
> > > > C++ compilation unit declares a `struct` that happens to have the same 
> > > > name as an Objective-C class (which is very common for wrapper code) 
> > > > may catch things of the wrong type.
> > > I've seen a lot of code which does something like this in a header
> > > 
> > > ```lang=cpp
> > > #ifdef __OBJC__
> > > @class I;
> > > #else
> > > struct I;
> > > #endif
> > > 
> > > void f(I *);
> > > ```
> > > 
> > > You want `f` to be mangled consistently across C++ and Objective-C++, 
> > > otherwise you'll get link errors if `f` is e.g. defined in a C++ TU and 
> > > referenced in an Objective-C++ TU. This was the case before your change 
> > > and isn't the case after your change, which is what @compnerd was 
> > > pointing out. They are mangled consistently in the Itanium ABI, and we 
> > > want them to be mangled consistently in the MS ABI as well.
> > > 
> > > Not wanting the catch mix-up is completely reasonable, of course, but it 
> > > can be done in a more targeted way. I'll put up a patch that undoes this 
> > > part of the change while still preventing incorrect catching.
> > With a non-fragile ABI, there is not guarantee that `struct I` and `@class 
> > I` have the same layout.  This is why `@defs` has gone away.  Any code that 
> > does this and treats `struct I` as anything other than an opaque type is 
> > broken.  No other platform supports throwing an Objective-C object and 
> > catching it as a struct, and this is an intentional design decision.  I 
> > would be strongly opposed to a change that intentionally supported this, 
> > because it is breaking correct code in order to make incorrect code do 
> > something that may or may not work.
> To be clear, I'm aware of the layout differences. My plan was to revert the 
> mangling here to what it was before (which would also be consistent with 
> Itanium), and then adjust the RTTI emission for Obj-C exception types (which 
> I'm handling in https://reviews.llvm.org/D47233) to differentiate between C++ 
>

[PATCH] D46052: GNUstep Objective-C ABI version 2

[David Chisnall via Phabricator via cfe-commits]

theraven marked 3 inline comments as done.
theraven added inline comments.



Comment at: lib/CodeGen/CGObjCGNU.cpp:439
+ ArrayRef IvarOffsets,
+ ArrayRef IvarAlign,
+ ArrayRef IvarOwnership);

rjmccall wrote:
> theraven wrote:
> > DHowett-MSFT wrote:
> > > While we're here, is there value in storing the ivar size in layout as 
> > > well, so that the runtime doesn't need to calculate it from the distance 
> > > to the next ivar/end of the instance?
> > Normally the runtime calculates it from the type encoding, if it's 
> > required.  I agree that it might be nice to have though.  Do you have 
> > strong feelings about needing it in the 2.0 ABI?  The looser coupling means 
> > that it would be easy to add in the 2.1 ABI if we want it later.  Are you 
> > seeing cases where the runtime is calculating it incorrectly because of 
> > insufficient information in the type encoding, or where calculating it is 
> > causing performance problems?
> The distance between ivar offsets wouldn't be correct anyway because of 
> alignment padding.  A set-ivar function might be able to get away with 
> copying too many bytes from an input buffer (although I wouldn't recommend 
> it!), but a get-ivar function definitely should not copy too many bytes into 
> an output buffer.
> 
> And all that's assuming that the runtime promises not to reorder ivars 
> dynamically.  I don't know what the GNU runtime says about that.  (Static 
> reordering is fine, since the runtime can reasonably demand that the compiler 
> emit ivars in layout order.)
The runtime was calculating the size from the type encoding (which can also be 
wrong in a few cases, such as packed structures).  I agree it makes sense to 
add the size though, and have done..



Comment at: lib/CodeGen/CGObjCGNU.cpp:1172
+if (isWeak) {
+  // Placeholder for the real symbol.
+  ClassSymbol->setInitializer(new llvm::GlobalVariable(TheModule,

rjmccall wrote:
> I would suggest clarifying in what sense this is a placeholder.  Does the 
> runtime recognize it specially?  If so, how?  Is it replaced statically by a 
> later pass in IRGen?
This comment was inherited from the old version and is actually nonsense now.  
I have replaced it with one that actually makes sense.


Repository:
  rC Clang

https://reviews.llvm.org/D46052



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

[PATCH] D46052: GNUstep Objective-C ABI version 2

[David Chisnall via Phabricator via cfe-commits]

theraven added inline comments.



Comment at: lib/CodeGen/CGObjCGNU.cpp:1056
+char c = Str[i];
+if (isalpha(c) || isnumber(c))
+  StringName += c;

Ka-Ka wrote:
> The isnumber() function was added to cctype.h by Apple. I don't think it can 
> be used in llvm.
> 
> According to
> https://stackoverflow.com/questions/39204080/what-is-the-difference-between-isdigit-and-isnumber
> 
Ah, isnumber is from 4.4BSD, I assumed it worked everywhere.  Changing it to 
isdigit is fine.


Repository:
  rC Clang

https://reviews.llvm.org/D46052



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

[PATCH] D46052: GNUstep Objective-C ABI version 2

[David Chisnall via Phabricator via cfe-commits]

theraven added inline comments.



Comment at: lib/CodeGen/CGObjCGNU.cpp:512
   /// used to return an untyped selector (with the types field set to NULL).
-  llvm::Value *GetSelector(CodeGenFunction &CGF, Selector Sel,
+  virtual llvm::Value *GetSelector(CodeGenFunction &CGF, Selector Sel,
const std::string &TypeEncoding);

xbolva00 wrote:
> This causes 
> CGObjCGNU.cpp:589:16: warning: ‘virtual llvm::Value* 
> {anonymous}::CGObjCGNU::GetSelector(clang::CodeGen::CodeGenFunction&, const 
> clang::ObjCMethodDecl*)’ was hidden [-Woverloaded-virtual]
>llvm::Value *GetSelector(CodeGenFunction &CGF,
> 
I can't reproduce this, and I'm not sure what the issue is.  The two lines have 
different overloads, so one shouldn't be hiding the other.


Repository:
  rC Clang

https://reviews.llvm.org/D46052



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

[PATCH] D46052: GNUstep Objective-C ABI version 2

[David Chisnall via Phabricator via cfe-commits]

theraven added inline comments.



Comment at: lib/CodeGen/CGObjCGNU.cpp:512
   /// used to return an untyped selector (with the types field set to NULL).
-  llvm::Value *GetSelector(CodeGenFunction &CGF, Selector Sel,
+  virtual llvm::Value *GetSelector(CodeGenFunction &CGF, Selector Sel,
const std::string &TypeEncoding);

xbolva00 wrote:
> theraven wrote:
> > xbolva00 wrote:
> > > This causes 
> > > CGObjCGNU.cpp:589:16: warning: ‘virtual llvm::Value* 
> > > {anonymous}::CGObjCGNU::GetSelector(clang::CodeGen::CodeGenFunction&, 
> > > const clang::ObjCMethodDecl*)’ was hidden [-Woverloaded-virtual]
> > >llvm::Value *GetSelector(CodeGenFunction &CGF,
> > > 
> > I can't reproduce this, and I'm not sure what the issue is.  The two lines 
> > have different overloads, so one shouldn't be hiding the other.
> It is a warning from GCC 7.2
Sounds like it's a spurious one.  Any idea how to silence it?


Repository:
  rC Clang

https://reviews.llvm.org/D46052



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

[PATCH] D50783: [CodeGen] Merge identical block descriptor global variables

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

This revision broke blocks on all ELF targets.  The block descriptors' symbol 
names can now include the @ character, which is reserved on ELF platforms as a 
separator between symbol name and symbol version.  As a result, nothing 
containing a block that has an Objective-C object argument will link.  Please 
add mangling similar to that in the GNUstep runtime to avoid this.


Repository:
  rC Clang

https://reviews.llvm.org/D50783



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

[PATCH] D50783: [CodeGen] Merge identical block descriptor global variables

[David Chisnall via Phabricator via cfe-commits]

theraven added inline comments.



Comment at: lib/CodeGen/CGBlocks.cpp:163
+  std::string TypeAtEncoding =
+  CGM.getContext().getObjCEncodingForBlock(BlockInfo.getBlockExpr());
+  Name += "e" + llvm::to_string(TypeAtEncoding.size()) + "_" + TypeAtEncoding;

Specifically, this line.  It is unsafe to use an Objective-C encoding directly 
as a symbol name on ELF.


Repository:
  rC Clang

https://reviews.llvm.org/D50783



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

[PATCH] D50783: [CodeGen] Merge identical block descriptor global variables

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

In https://reviews.llvm.org/D50783#1221147, @ahatanak wrote:

> The GNUstep documentation I found replaces '@' with '\1'. Would that fix the 
> problem?
>
> https://github.com/gnustep/libobjc2/blob/master/ABIDoc/abi.tex


Yup.  If this mangling is needed outside of CGObjCGNU, it would be good to take 
the code that does this mangling and move it somewhere that can be reused.  I 
don't know if there are any characters that are valid in Objective-C type 
encoding but aren't valid in Windows PE/COFF symbols, but having a hook to fix 
them in a single place sounds like a good idea.


Repository:
  rC Clang

https://reviews.llvm.org/D50783



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

[PATCH] D50144: Add Windows support for the GNUstep Objective-C ABI V2.

[David Chisnall via Phabricator via cfe-commits]

theraven created this revision.
theraven added reviewers: rjmccall, DHowett-MSFT.
Herald added a subscriber: cfe-commits.

Introduces funclet-based unwinding for Objective-C and fixes an issue
where global blocks can't have their isa pointers initialised on
Windows.

After discussion with Dustin, this changes the name mangling of
Objective-C types to prevent a C++ catch statement of type struct X*
from catching an Objective-C object of type X*.


Repository:
  rC Clang

https://reviews.llvm.org/D50144

Files:
  include/clang/Driver/Options.td
  lib/AST/MicrosoftMangle.cpp
  lib/CodeGen/CGBlocks.cpp
  lib/CodeGen/CGException.cpp
  lib/CodeGen/CGObjCGNU.cpp
  lib/CodeGen/CGObjCRuntime.cpp
  lib/CodeGen/CGObjCRuntime.h
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/EHScopeStack.h
  lib/Driver/ToolChains/Clang.cpp
  test/CodeGenObjC/gnu-init.m
  test/CodeGenObjCXX/arc-marker-funclet.mm
  test/CodeGenObjCXX/microsoft-abi-arc-param-order.mm
  test/CodeGenObjCXX/msabi-objc-extensions.mm
  test/CodeGenObjCXX/msabi-objc-types.mm

Index: test/CodeGenObjCXX/msabi-objc-types.mm
===
--- test/CodeGenObjCXX/msabi-objc-types.mm
+++ test/CodeGenObjCXX/msabi-objc-types.mm
@@ -3,166 +3,166 @@
 @class I;
 
 id kid;
-// CHECK: @"?kid@@3PAUobjc_object@@A" =  dso_local global
+// CHECK: @"?kid@@3PAU.objc_object@@A" =  dso_local global
 
 Class klass;
-// CHECK: @"?klass@@3PAUobjc_class@@A" = dso_local global
+// CHECK: @"?klass@@3PAU.objc_class@@A" = dso_local global
 
 I *kI;
-// CHECK: @"?kI@@3PAUI@@A" = dso_local global
+// CHECK: @"?kI@@3PAU.objc_cls_I@@A" = dso_local global
 
 void f(I *) {}
-// CHECK-LABEL: "?f@@YAXPAUI@@@Z"
+// CHECK-LABEL: "?f@@YAXPAU.objc_cls_I@@@Z"
 
 void f(const I *) {}
-// CHECK-LABEL: "?f@@YAXPBUI@@@Z"
+// CHECK-LABEL: "?f@@YAXPBU.objc_cls_I@@@Z"
 
 void f(I &) {}
-// CHECK-LABEL: "?f@@YAXAAUI@@@Z"
+// CHECK-LABEL: "?f@@YAXAAU.objc_cls_I@@@Z"
 
 void f(const I &) {}
-// CHECK-LABEL: "?f@@YAXABUI@@@Z"
+// CHECK-LABEL: "?f@@YAXABU.objc_cls_I@@@Z"
 
 void f(const I &&) {}
-// CHECK-LABEL: "?f@@YAX$$QBUI@@@Z"
+// CHECK-LABEL: "?f@@YAX$$QBU.objc_cls_I@@@Z"
 
 void g(id) {}
-// CHECK-LABEL: "?g@@YAXPAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAXPAU.objc_object@@@Z"
 
 void g(id &) {}
-// CHECK-LABEL: "?g@@YAXAAPAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAXAAPAU.objc_object@@@Z"
 
 void g(const id &) {}
-// CHECK-LABEL: "?g@@YAXABQAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAXABQAU.objc_object@@@Z"
 
 void g(id &&) {}
-// CHECK-LABEL: "?g@@YAX$$QAPAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAX$$QAPAU.objc_object@@@Z"
 
 void h(Class) {}
-// CHECK-LABEL: "?h@@YAXPAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAXPAU.objc_class@@@Z"
 
 void h(Class &) {}
-// CHECK-LABEL: "?h@@YAXAAPAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAXAAPAU.objc_class@@@Z"
 
 void h(const Class &) {}
-// CHECK-LABEL: "?h@@YAXABQAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAXABQAU.objc_class@@@Z"
 
 void h(Class &&) {}
-// CHECK-LABEL: "?h@@YAX$$QAPAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAX$$QAPAU.objc_class@@@Z"
 
 I *i() { return nullptr; }
-// CHECK-LABEL: "?i@@YAPAUI@@XZ"
+// CHECK-LABEL: "?i@@YAPAU.objc_cls_I@@XZ"
 
 const I *j() { return nullptr; }
-// CHECK-LABEL: "?j@@YAPBUI@@XZ"
+// CHECK-LABEL: "?j@@YAPBU.objc_cls_I@@XZ"
 
 I &k() { return *kI; }
-// CHECK-LABEL: "?k@@YAAAUI@@XZ"
+// CHECK-LABEL: "?k@@YAAAU.objc_cls_I@@XZ"
 
 const I &l() { return *kI; }
-// CHECK-LABEL: "?l@@YAABUI@@XZ"
+// CHECK-LABEL: "?l@@YAABU.objc_cls_I@@XZ"
 
 void m(const id) {}
-// CHECK-LABEL: "?m@@YAXQAUobjc_object@@@Z"
+// CHECK-LABEL: "?m@@YAXQAU.objc_object@@@Z"
 
 void m(const I *) {}
-// CHECK-LABEL: "?m@@YAXPBUI@@@Z"
+// CHECK-LABEL: "?m@@YAXPBU.objc_cls_I@@@Z"
 
 void n(SEL) {}
-// CHECK-LABEL: "?n@@YAXPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXPAU.objc_selector@@@Z"
 
 void n(SEL *) {}
-// CHECK-LABEL: "?n@@YAXPAPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXPAPAU.objc_selector@@@Z"
 
 void n(const SEL *) {}
-// CHECK-LABEL: "?n@@YAXPBQAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXPBQAU.objc_selector@@@Z"
 
 void n(SEL &) {}
-// CHECK-LABEL: "?n@@YAXAAPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXAAPAU.objc_selector@@@Z"
 
 void n(const SEL &) {}
-// CHECK-LABEL: "?n@@YAXABQAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXABQAU.objc_selector@@@Z"
 
 void n(SEL &&) {}
-// CHECK-LABEL: "?n@@YAX$$QAPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAX$$QAPAU.objc_selector@@@Z"
 
 struct __declspec(dllexport) s {
   struct s &operator=(const struct s &) = delete;
 
   void m(I *) {}
-  // CHECK-LABEL: "?m@s@@QAAXPAUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXPAU.objc_cls_I@@@Z"
 
   void m(const I *) {}
-  // CHECK-LABEL: "?m@s@@QAAXPBUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXPBU.objc_cls_I@@@Z"
 
   void m(I &) {}
-  // CHECK-LABEL: "?m@s@@QAAXAAUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXAAU.objc_cls_I@@@Z"
 
   void m(const I &) {}
-  // CHECK-LABEL: "?m@s@@QAAXABUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXABU.objc_cls_I@@

[PATCH] D50144: Add Windows support for the GNUstep Objective-C ABI V2.

[David Chisnall via Phabricator via cfe-commits]

theraven updated this revision to Diff 158997.
theraven added a comment.

- Address Dustin's review comments.


Repository:
  rC Clang

https://reviews.llvm.org/D50144

Files:
  include/clang/Driver/Options.td
  lib/AST/MicrosoftMangle.cpp
  lib/CodeGen/CGBlocks.cpp
  lib/CodeGen/CGException.cpp
  lib/CodeGen/CGObjCGNU.cpp
  lib/CodeGen/CGObjCRuntime.cpp
  lib/CodeGen/CGObjCRuntime.h
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/EHScopeStack.h
  lib/Driver/ToolChains/Clang.cpp
  test/CodeGenObjC/gnu-init.m
  test/CodeGenObjCXX/arc-marker-funclet.mm
  test/CodeGenObjCXX/microsoft-abi-arc-param-order.mm
  test/CodeGenObjCXX/msabi-objc-extensions.mm
  test/CodeGenObjCXX/msabi-objc-types.mm

Index: test/CodeGenObjCXX/msabi-objc-types.mm
===
--- test/CodeGenObjCXX/msabi-objc-types.mm
+++ test/CodeGenObjCXX/msabi-objc-types.mm
@@ -3,166 +3,166 @@
 @class I;
 
 id kid;
-// CHECK: @"?kid@@3PAUobjc_object@@A" =  dso_local global
+// CHECK: @"?kid@@3PAU.objc_object@@A" =  dso_local global
 
 Class klass;
-// CHECK: @"?klass@@3PAUobjc_class@@A" = dso_local global
+// CHECK: @"?klass@@3PAU.objc_class@@A" = dso_local global
 
 I *kI;
-// CHECK: @"?kI@@3PAUI@@A" = dso_local global
+// CHECK: @"?kI@@3PAU.objc_cls_I@@A" = dso_local global
 
 void f(I *) {}
-// CHECK-LABEL: "?f@@YAXPAUI@@@Z"
+// CHECK-LABEL: "?f@@YAXPAU.objc_cls_I@@@Z"
 
 void f(const I *) {}
-// CHECK-LABEL: "?f@@YAXPBUI@@@Z"
+// CHECK-LABEL: "?f@@YAXPBU.objc_cls_I@@@Z"
 
 void f(I &) {}
-// CHECK-LABEL: "?f@@YAXAAUI@@@Z"
+// CHECK-LABEL: "?f@@YAXAAU.objc_cls_I@@@Z"
 
 void f(const I &) {}
-// CHECK-LABEL: "?f@@YAXABUI@@@Z"
+// CHECK-LABEL: "?f@@YAXABU.objc_cls_I@@@Z"
 
 void f(const I &&) {}
-// CHECK-LABEL: "?f@@YAX$$QBUI@@@Z"
+// CHECK-LABEL: "?f@@YAX$$QBU.objc_cls_I@@@Z"
 
 void g(id) {}
-// CHECK-LABEL: "?g@@YAXPAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAXPAU.objc_object@@@Z"
 
 void g(id &) {}
-// CHECK-LABEL: "?g@@YAXAAPAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAXAAPAU.objc_object@@@Z"
 
 void g(const id &) {}
-// CHECK-LABEL: "?g@@YAXABQAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAXABQAU.objc_object@@@Z"
 
 void g(id &&) {}
-// CHECK-LABEL: "?g@@YAX$$QAPAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAX$$QAPAU.objc_object@@@Z"
 
 void h(Class) {}
-// CHECK-LABEL: "?h@@YAXPAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAXPAU.objc_class@@@Z"
 
 void h(Class &) {}
-// CHECK-LABEL: "?h@@YAXAAPAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAXAAPAU.objc_class@@@Z"
 
 void h(const Class &) {}
-// CHECK-LABEL: "?h@@YAXABQAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAXABQAU.objc_class@@@Z"
 
 void h(Class &&) {}
-// CHECK-LABEL: "?h@@YAX$$QAPAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAX$$QAPAU.objc_class@@@Z"
 
 I *i() { return nullptr; }
-// CHECK-LABEL: "?i@@YAPAUI@@XZ"
+// CHECK-LABEL: "?i@@YAPAU.objc_cls_I@@XZ"
 
 const I *j() { return nullptr; }
-// CHECK-LABEL: "?j@@YAPBUI@@XZ"
+// CHECK-LABEL: "?j@@YAPBU.objc_cls_I@@XZ"
 
 I &k() { return *kI; }
-// CHECK-LABEL: "?k@@YAAAUI@@XZ"
+// CHECK-LABEL: "?k@@YAAAU.objc_cls_I@@XZ"
 
 const I &l() { return *kI; }
-// CHECK-LABEL: "?l@@YAABUI@@XZ"
+// CHECK-LABEL: "?l@@YAABU.objc_cls_I@@XZ"
 
 void m(const id) {}
-// CHECK-LABEL: "?m@@YAXQAUobjc_object@@@Z"
+// CHECK-LABEL: "?m@@YAXQAU.objc_object@@@Z"
 
 void m(const I *) {}
-// CHECK-LABEL: "?m@@YAXPBUI@@@Z"
+// CHECK-LABEL: "?m@@YAXPBU.objc_cls_I@@@Z"
 
 void n(SEL) {}
-// CHECK-LABEL: "?n@@YAXPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXPAU.objc_selector@@@Z"
 
 void n(SEL *) {}
-// CHECK-LABEL: "?n@@YAXPAPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXPAPAU.objc_selector@@@Z"
 
 void n(const SEL *) {}
-// CHECK-LABEL: "?n@@YAXPBQAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXPBQAU.objc_selector@@@Z"
 
 void n(SEL &) {}
-// CHECK-LABEL: "?n@@YAXAAPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXAAPAU.objc_selector@@@Z"
 
 void n(const SEL &) {}
-// CHECK-LABEL: "?n@@YAXABQAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXABQAU.objc_selector@@@Z"
 
 void n(SEL &&) {}
-// CHECK-LABEL: "?n@@YAX$$QAPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAX$$QAPAU.objc_selector@@@Z"
 
 struct __declspec(dllexport) s {
   struct s &operator=(const struct s &) = delete;
 
   void m(I *) {}
-  // CHECK-LABEL: "?m@s@@QAAXPAUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXPAU.objc_cls_I@@@Z"
 
   void m(const I *) {}
-  // CHECK-LABEL: "?m@s@@QAAXPBUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXPBU.objc_cls_I@@@Z"
 
   void m(I &) {}
-  // CHECK-LABEL: "?m@s@@QAAXAAUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXAAU.objc_cls_I@@@Z"
 
   void m(const I &) {}
-  // CHECK-LABEL: "?m@s@@QAAXABUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXABU.objc_cls_I@@@Z"
 
   void m(I &&) {}
-  // CHECK-LABEL: "?m@s@@QAAX$$QAUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAX$$QAU.objc_cls_I@@@Z"
 
   void m(const I &&) {}
-  // CHECK-LABEL: "?m@s@@QAAX$$QBUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAX$$QBU.objc_cls_I@@@Z"
 
   void m(id) {}
-  // CHECK-LABEL: "?m@s@@QAAXPAUobjc_object@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAX

[PATCH] D50144: Add Windows support for the GNUstep Objective-C ABI V2.

[David Chisnall via Phabricator via cfe-commits]

theraven added inline comments.



Comment at: include/clang/Driver/Options.td:1491
 // Objective-C ABI options.
-def fobjc_runtime_EQ : Joined<["-"], "fobjc-runtime=">, Group, 
Flags<[CC1Option]>,
+def fobjc_runtime_EQ : Joined<["-"], "fobjc-runtime=">, Group, 
Flags<[CC1Option, CoreOption]>,
   HelpText<"Specify the target Objective-C runtime kind and version">;

DHowett-MSFT wrote:
> Is this so it's exposed to clang-cl?
Yup.  Unfortunately, the normal work around of passing the argument with 
`-Xclang` (so `-Xclang -fobjc-runtime=gnustep-2.0`) doesn't work because the 
driver makes various other decisions about what flags to pass to the front end 
based on the Objective-C runtime and makes all of those assuming 
`-fobjc-runtime=gcc`.  It is not possible to compile ARC code without this.



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",

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?


Repository:
  rC Clang

https://reviews.llvm.org/D50144



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

[PATCH] D50144: Add Windows support for the GNUstep Objective-C ABI V2.

[David Chisnall via Phabricator via cfe-commits]

theraven updated this revision to Diff 159293.
theraven added a comment.

- Fix an issue in protocol generation.


Repository:
  rC Clang

https://reviews.llvm.org/D50144

Files:
  include/clang/Driver/Options.td
  lib/AST/MicrosoftMangle.cpp
  lib/CodeGen/CGBlocks.cpp
  lib/CodeGen/CGException.cpp
  lib/CodeGen/CGObjCGNU.cpp
  lib/CodeGen/CGObjCRuntime.cpp
  lib/CodeGen/CGObjCRuntime.h
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/EHScopeStack.h
  lib/Driver/ToolChains/Clang.cpp
  test/CodeGenObjC/gnu-init.m
  test/CodeGenObjCXX/arc-marker-funclet.mm
  test/CodeGenObjCXX/microsoft-abi-arc-param-order.mm
  test/CodeGenObjCXX/msabi-objc-extensions.mm
  test/CodeGenObjCXX/msabi-objc-types.mm

Index: test/CodeGenObjCXX/msabi-objc-types.mm
===
--- test/CodeGenObjCXX/msabi-objc-types.mm
+++ test/CodeGenObjCXX/msabi-objc-types.mm
@@ -3,166 +3,166 @@
 @class I;
 
 id kid;
-// CHECK: @"?kid@@3PAUobjc_object@@A" =  dso_local global
+// CHECK: @"?kid@@3PAU.objc_object@@A" =  dso_local global
 
 Class klass;
-// CHECK: @"?klass@@3PAUobjc_class@@A" = dso_local global
+// CHECK: @"?klass@@3PAU.objc_class@@A" = dso_local global
 
 I *kI;
-// CHECK: @"?kI@@3PAUI@@A" = dso_local global
+// CHECK: @"?kI@@3PAU.objc_cls_I@@A" = dso_local global
 
 void f(I *) {}
-// CHECK-LABEL: "?f@@YAXPAUI@@@Z"
+// CHECK-LABEL: "?f@@YAXPAU.objc_cls_I@@@Z"
 
 void f(const I *) {}
-// CHECK-LABEL: "?f@@YAXPBUI@@@Z"
+// CHECK-LABEL: "?f@@YAXPBU.objc_cls_I@@@Z"
 
 void f(I &) {}
-// CHECK-LABEL: "?f@@YAXAAUI@@@Z"
+// CHECK-LABEL: "?f@@YAXAAU.objc_cls_I@@@Z"
 
 void f(const I &) {}
-// CHECK-LABEL: "?f@@YAXABUI@@@Z"
+// CHECK-LABEL: "?f@@YAXABU.objc_cls_I@@@Z"
 
 void f(const I &&) {}
-// CHECK-LABEL: "?f@@YAX$$QBUI@@@Z"
+// CHECK-LABEL: "?f@@YAX$$QBU.objc_cls_I@@@Z"
 
 void g(id) {}
-// CHECK-LABEL: "?g@@YAXPAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAXPAU.objc_object@@@Z"
 
 void g(id &) {}
-// CHECK-LABEL: "?g@@YAXAAPAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAXAAPAU.objc_object@@@Z"
 
 void g(const id &) {}
-// CHECK-LABEL: "?g@@YAXABQAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAXABQAU.objc_object@@@Z"
 
 void g(id &&) {}
-// CHECK-LABEL: "?g@@YAX$$QAPAUobjc_object@@@Z"
+// CHECK-LABEL: "?g@@YAX$$QAPAU.objc_object@@@Z"
 
 void h(Class) {}
-// CHECK-LABEL: "?h@@YAXPAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAXPAU.objc_class@@@Z"
 
 void h(Class &) {}
-// CHECK-LABEL: "?h@@YAXAAPAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAXAAPAU.objc_class@@@Z"
 
 void h(const Class &) {}
-// CHECK-LABEL: "?h@@YAXABQAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAXABQAU.objc_class@@@Z"
 
 void h(Class &&) {}
-// CHECK-LABEL: "?h@@YAX$$QAPAUobjc_class@@@Z"
+// CHECK-LABEL: "?h@@YAX$$QAPAU.objc_class@@@Z"
 
 I *i() { return nullptr; }
-// CHECK-LABEL: "?i@@YAPAUI@@XZ"
+// CHECK-LABEL: "?i@@YAPAU.objc_cls_I@@XZ"
 
 const I *j() { return nullptr; }
-// CHECK-LABEL: "?j@@YAPBUI@@XZ"
+// CHECK-LABEL: "?j@@YAPBU.objc_cls_I@@XZ"
 
 I &k() { return *kI; }
-// CHECK-LABEL: "?k@@YAAAUI@@XZ"
+// CHECK-LABEL: "?k@@YAAAU.objc_cls_I@@XZ"
 
 const I &l() { return *kI; }
-// CHECK-LABEL: "?l@@YAABUI@@XZ"
+// CHECK-LABEL: "?l@@YAABU.objc_cls_I@@XZ"
 
 void m(const id) {}
-// CHECK-LABEL: "?m@@YAXQAUobjc_object@@@Z"
+// CHECK-LABEL: "?m@@YAXQAU.objc_object@@@Z"
 
 void m(const I *) {}
-// CHECK-LABEL: "?m@@YAXPBUI@@@Z"
+// CHECK-LABEL: "?m@@YAXPBU.objc_cls_I@@@Z"
 
 void n(SEL) {}
-// CHECK-LABEL: "?n@@YAXPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXPAU.objc_selector@@@Z"
 
 void n(SEL *) {}
-// CHECK-LABEL: "?n@@YAXPAPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXPAPAU.objc_selector@@@Z"
 
 void n(const SEL *) {}
-// CHECK-LABEL: "?n@@YAXPBQAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXPBQAU.objc_selector@@@Z"
 
 void n(SEL &) {}
-// CHECK-LABEL: "?n@@YAXAAPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXAAPAU.objc_selector@@@Z"
 
 void n(const SEL &) {}
-// CHECK-LABEL: "?n@@YAXABQAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAXABQAU.objc_selector@@@Z"
 
 void n(SEL &&) {}
-// CHECK-LABEL: "?n@@YAX$$QAPAUobjc_selector@@@Z"
+// CHECK-LABEL: "?n@@YAX$$QAPAU.objc_selector@@@Z"
 
 struct __declspec(dllexport) s {
   struct s &operator=(const struct s &) = delete;
 
   void m(I *) {}
-  // CHECK-LABEL: "?m@s@@QAAXPAUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXPAU.objc_cls_I@@@Z"
 
   void m(const I *) {}
-  // CHECK-LABEL: "?m@s@@QAAXPBUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXPBU.objc_cls_I@@@Z"
 
   void m(I &) {}
-  // CHECK-LABEL: "?m@s@@QAAXAAUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXAAU.objc_cls_I@@@Z"
 
   void m(const I &) {}
-  // CHECK-LABEL: "?m@s@@QAAXABUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAXABU.objc_cls_I@@@Z"
 
   void m(I &&) {}
-  // CHECK-LABEL: "?m@s@@QAAX$$QAUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAX$$QAU.objc_cls_I@@@Z"
 
   void m(const I &&) {}
-  // CHECK-LABEL: "?m@s@@QAAX$$QBUI@@@Z"
+  // CHECK-LABEL: "?m@s@@QAAX$$QBU.objc_cls_I@@@Z"
 
   void m(id) {}
-  // CHECK-LABEL: "?m@s@@QAAXPAUobjc_object@@@Z"
+  // CHECK-LABEL: "?m@s@@Q

[PATCH] D44580: Sema: allow comparison between blocks & block-compatible objc types

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

We seem to be missing tests for the assignment part of this patch.




Comment at: lib/Sema/SemaExpr.cpp:7749
+// id (or strictly compatible object type) -> T^
+if (getLangOpts().ObjC1 && 
RHSType->isBlockCompatibleObjCPointerType(Context)) {
   Kind = CK_AnyPointerToBlockPointerCast;

Do we want to allow implicit casts for all object types to block types for 
assignment, or only for null pointers?  We definitely want to allow `nil` to be 
assigned to a block type, but I would lean slightly to requiring an implicit 
cast.

Ideally, I think we'd allow this but warn, because casting from an arbitrary 
ObjC type to a block incorrectly can cause exciting security vulnerabilities if 
it's done incorrectly and we should encourage people to check these casts 
(`nil` is always safe though - as long as somewhere else checks the nullability 
attributes).


Repository:
  rC Clang

https://reviews.llvm.org/D44580



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

[PATCH] D53547: NFC: Remove the ObjC1/ObjC2 distinction from clang (and related projects)

[David Chisnall via Phabricator via cfe-commits]

theraven accepted this revision.
theraven added a comment.
This revision is now accepted and ready to land.

Looks good for me, and removing all of the code describing Objective-C 4 as 
ObjC1 makes me happy.




Comment at: 
clang-tools-extra/clang-tidy/google/AvoidThrowingObjCExceptionCheck.cpp:23
   // this check should only be applied to ObjC sources.
-  if (!getLangOpts().ObjC1 && !getLangOpts().ObjC2) {
+  if (!getLangOpts().ObjC) {
 return;

LLVM style wants to get rid of the braces here (and the next few instances of 
this).  Since you're touching this code, would you mind fixing that as well?


https://reviews.llvm.org/D53547



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

[PATCH] D54539: [CodeGen] Replace '@' characters in block descriptors' symbol names with '\1' on ELF targets.

[David Chisnall via Phabricator via cfe-commits]

theraven added inline comments.



Comment at: lib/CodeGen/CGObjCRuntime.h:288
+  /// descriptor's symbol name.
+  virtual std::string getObjCEncodingForBlock(const BlockExpr *BE) const;
+

I'm not sure that this actually belongs in CGObjCRuntime.  The runtime doesn't 
care about the symbol namings, and the need for this mangling applies to any 
ELF platform (it would also apply to Mach-O if Mach-O adopted GNU-style symbol 
versioning).  It probably isn't relevant, for example, with WebAssembly, 
irrespective of the runtime, and it would be relevant for the Apple family of 
runtimes if anyone wanted to use them on ELF.  

If it does go in CGObjCRuntime, I think I'd prefer to have the implementation 
there as well and make it conditional on the object format, not on the runtime. 
 Having a function that maps Objective-C type encodings to valid symbol names 
in CGObjCRuntime would let us do a little bit of cleanup in CGObjCGNU* and 
could also be used in the blocks code.


Repository:
  rC Clang

https://reviews.llvm.org/D54539



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

[PATCH] D55544: Warning: objc-encodings-larger-than=

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

It would probably be a good idea to have a similar check on properties, as 
property encoding strings contain the type encoding (plus extra stuff).

I wonder if we also want an option in code generation to replace very long type 
encodings (or encodings of specifically annotated ivars?) with `"?"` ('unknown 
type')?




Comment at: lib/Sema/SemaDeclObjC.cpp:3881
+  unsigned long encodingSize = LangOpts.ObjCLargeEncodingSize;
+  if (encodingSize == 0) return;
+  for (ObjCIvarDecl *ivar = ID->getClassInterface()->all_declared_ivar_begin();

I missed this early exit on the first pass, please can you add some spacing to 
make it more obvious?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55544



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

[PATCH] D58724: [gnustep-objc] Make the GNUstep v2 ABI work for Windows DLLs.

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

This review is mostly so that @DHowett-MSFT can check that I didn't break his 
patches too badly in the cleanup and test that they still do allow building 
Objective-C DLLs on Windows.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58724



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

[PATCH] D58724: [gnustep-objc] Make the GNUstep v2 ABI work for Windows DLLs.

[David Chisnall via Phabricator via cfe-commits]

theraven created this revision.
theraven added a reviewer: DHowett-MSFT.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
theraven added a comment.

This review is mostly so that @DHowett-MSFT can check that I didn't break his 
patches too badly in the cleanup and test that they still do allow building 
Objective-C DLLs on Windows.


Based on a patch by Dustin Howett, modified to not change the ABI for
ELF platforms.

[gnustep-objc] Use more Windows-like section names.

This also makes things more readable by PE/COFF debug tools that assume
sections fit in the first header.

Patch by Dustin Howett!


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D58724

Files:
  clang/lib/CodeGen/CGObjCGNU.cpp

Index: clang/lib/CodeGen/CGObjCGNU.cpp
===
--- clang/lib/CodeGen/CGObjCGNU.cpp
+++ clang/lib/CodeGen/CGObjCGNU.cpp
@@ -185,12 +185,16 @@
   (R.getVersion() >= VersionTuple(major, minor));
   }
 
+  StringRef SymbolPrefix() {
+return CGM.getTriple().isOSBinFormatCOFF() ? "_" : "._";
+  }
+
   std::string SymbolForProtocol(StringRef Name) {
-return (StringRef("._OBJC_PROTOCOL_") + Name).str();
+return (SymbolPrefix() + "OBJC_PROTOCOL_" + Name).str();
   }
 
   std::string SymbolForProtocolRef(StringRef Name) {
-return (StringRef("._OBJC_REF_PROTOCOL_") + Name).str();
+return (SymbolPrefix() + "OBJC_REF_PROTOCOL_" + Name).str();
   }
 
 
@@ -906,12 +910,15 @@
 ConstantStringSection
   };
   static const char *const SectionsBaseNames[8];
+  static const char *const PECOFFSectionsBaseNames[8];
   template
   std::string sectionName() {
-std::string name(SectionsBaseNames[K]);
-if (CGM.getTriple().isOSBinFormatCOFF())
+if (CGM.getTriple().isOSBinFormatCOFF()) {
+  std::string name(PECOFFSectionsBaseNames[K]);
   name += "$m";
-return name;
+  return name;
+}
+return SectionsBaseNames[K];
   }
   /// The GCC ABI superclass message lookup function.  Takes a pointer to a
   /// structure describing the receiver and the class, and a selector as
@@ -932,15 +939,19 @@
   bool EmittedClass = false;
   /// Generate the name of a symbol for a reference to a class.  Accesses to
   /// classes should be indirected via this.
+
+  typedef std::pair> EarlyInitPair;
+  std::vector EarlyInitList;
+
   std::string SymbolForClassRef(StringRef Name, bool isWeak) {
 if (isWeak)
-  return (StringRef("._OBJC_WEAK_REF_CLASS_") + Name).str();
+  return (SymbolPrefix() + "OBJC_WEAK_REF_CLASS_" + Name).str();
 else
-  return (StringRef("._OBJC_REF_CLASS_") + Name).str();
+  return (SymbolPrefix() + "OBJC_REF_CLASS_" + Name).str();
   }
   /// Generate the name of a class symbol.
   std::string SymbolForClass(StringRef Name) {
-return (StringRef("._OBJC_CLASS_") + Name).str();
+return (SymbolPrefix() + "OBJC_CLASS_" + Name).str();
   }
   void CallRuntimeFunction(CGBuilderTy &B, StringRef FunctionName,
   ArrayRef Args) {
@@ -994,10 +1005,13 @@
 
 llvm::Constant *isa = TheModule.getNamedGlobal(Sym);
 
-if (!isa)
+if (!isa) {
   isa = new llvm::GlobalVariable(TheModule, IdTy, /* isConstant */false,
   llvm::GlobalValue::ExternalLinkage, nullptr, Sym);
-else if (isa->getType() != PtrToIdTy)
+  if (CGM.getTriple().isOSBinFormatCOFF()) {
+cast(isa)->setDLLStorageClass(llvm::GlobalValue::DLLImportStorageClass);
+  }
+} else if (isa->getType() != PtrToIdTy)
   isa = llvm::ConstantExpr::getBitCast(isa, PtrToIdTy);
 
 //  struct
@@ -1012,7 +1026,11 @@
 
 ConstantInitBuilder Builder(CGM);
 auto Fields = Builder.beginStruct();
-Fields.add(isa);
+if (!CGM.getTriple().isOSBinFormatCOFF()) {
+  Fields.add(isa);
+} else {
+  Fields.addNullPointer(PtrTy);
+}
 // For now, all non-ASCII strings are represented as UTF-16.  As such, the
 // number of bytes is simply double the number of UTF-16 codepoints.  In
 // ASCII strings, the number of bytes is equal to the number of non-ASCII
@@ -1083,6 +1101,10 @@
   ObjCStrGV->setComdat(TheModule.getOrInsertComdat(StringName));
   ObjCStrGV->setVisibility(llvm::GlobalValue::HiddenVisibility);
 }
+if (CGM.getTriple().isOSBinFormatCOFF()) {
+  std::pair v{ObjCStrGV, 0};
+  EarlyInitList.emplace_back(Sym, v);
+}
 llvm::Constant *ObjCStr = llvm::ConstantExpr::getBitCast(ObjCStrGV, IdTy);
 ObjCStrings[Str] = ObjCStr;
 ConstantStrings.push_back(ObjCStr);
@@ -1196,6 +1218,26 @@
   ClassSymbol->setInitializer(new llvm::GlobalVariable(TheModule,
   Int8Ty, false, llvm::GlobalValue::ExternalWeakLinkage,
   nullptr, SymbolForClass(Name)));
+else {
+  if (CGM.getTriple().isOSBinFormatCOFF()) {
+IdentifierInfo &II = CGM.getContext().Idents.get(Name);
+TranslationUnitDecl *TUDecl = CGM.getContext().getTranslationUnitDecl();
+DeclContext *DC = Tra

[PATCH] D58724: [gnustep-objc] Make the GNUstep v2 ABI work for Windows DLLs.

[David Chisnall via Phabricator via cfe-commits]

theraven marked 2 inline comments as done.
theraven added inline comments.



Comment at: clang/lib/CodeGen/CGObjCGNU.cpp:188
 
+  StringRef SymbolPrefix() {
+return CGM.getTriple().isOSBinFormatCOFF() ? "_" : "._";

DHowett-MSFT wrote:
> Should this be `SymbolPrefix()` or something more like 
> `MangleSymbol(StringRef sym)`? I know that right now we only need to prepend 
> `_` or `._`, but is it more future-proof to make it generic?
I have refactored this, and then tried adding a $ instead of the . for 
mangling.  On further testing, the latest link.exe from VS 2017 no longer 
complains about symbols starting with a dot, so I'm inclined to revert that 
part of it entirely (lld-link.exe was always happy).  I'd prefer to minimise 
differences between COFF and ELF and this means that we have different section 
names, but aside from needing the extra global initialisation on COFF, 
everything else is the same.  



Comment at: clang/lib/CodeGen/CGObjCGNU.cpp:1528
 InitStructBuilder.addInt(Int64Ty, 0);
-for (auto *s : SectionsBaseNames) {
-  auto bounds = GetSectionBounds(s);
-  InitStructBuilder.add(bounds.first);
-  InitStructBuilder.add(bounds.second);
-};
+if (CGM.getTriple().isOSBinFormatCOFF()) {
+  for (auto *s : PECOFFSectionsBaseNames) {

DHowett-MSFT wrote:
> We may be able to reduce the duplication here (aside: it is very cool that 
> Phabricator shows duplicated lines) by capturing `auto sectionVec = 
> &SectionsBaseNames;` and switching which is in use based on bin format.
I thought about that and didn't do it because if the two arrays are different 
lengths, they're different types.  But, of course, they're the same length and 
if they're ever different lengths in the future then that's probably a bug.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58724



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

[PATCH] D58724: [gnustep-objc] Make the GNUstep v2 ABI work for Windows DLLs.

[David Chisnall via Phabricator via cfe-commits]

theraven marked 2 inline comments as done.
theraven added inline comments.



Comment at: clang/lib/CodeGen/CGObjCGNU.cpp:188
 
+  StringRef SymbolPrefix() {
+return CGM.getTriple().isOSBinFormatCOFF() ? "_" : "._";

DHowett-MSFT wrote:
> theraven wrote:
> > DHowett-MSFT wrote:
> > > Should this be `SymbolPrefix()` or something more like 
> > > `MangleSymbol(StringRef sym)`? I know that right now we only need to 
> > > prepend `_` or `._`, but is it more future-proof to make it generic?
> > I have refactored this, and then tried adding a $ instead of the . for 
> > mangling.  On further testing, the latest link.exe from VS 2017 no longer 
> > complains about symbols starting with a dot, so I'm inclined to revert that 
> > part of it entirely (lld-link.exe was always happy).  I'd prefer to 
> > minimise differences between COFF and ELF and this means that we have 
> > different section names, but aside from needing the extra global 
> > initialisation on COFF, everything else is the same.  
> The issue I had with symbols starting with `.` was in `.DEF` files 
> specifically.
> 
> Linking a shared object containing:
> ```
> @.exp_with_dot = dllexport global i32 0, align 4
> ```
> 
> without a def file, yields:
> 
> ```
> ordinal hint RVA  name
> 
>   10 00012900 .exp_with_dot
> ```
> 
> However, linking the same library with a definition file:
> 
> ###test.def
> ```
> EXPORTS
>  .exp_with_dot
> ```
> 
> yields
> 
> ###output
> ```
> test.def : fatal error LNK1242: '.exp_with_dot' is an invalid export symbol 
> name
> ```
> 
> This still reproduces with 15.9.8, sadly. LINK version 14.16.27027.1.
Does it work with a $ instead of a .?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58724



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

[PATCH] D58807: [CodeGen] COMDAT-fold block descriptors

[David Chisnall via Phabricator via cfe-commits]

theraven accepted this revision.
theraven added a comment.
This revision is now accepted and ready to land.

Looks good to me.  On ELF and Mach-O, I think we get the equivalent behaviour 
automatically from the ODR linkage type.  I'd really like to see linkage type 
and COMDAT made orthogonal, but that's a bigger change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58807



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

[PATCH] D58807: [CodeGen] COMDAT-fold block descriptors

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

It would probably benefit from a test so that we don't regress.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58807



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

[PATCH] D58724: [gnustep-objc] Make the GNUstep v2 ABI work for Windows DLLs.

[David Chisnall via Phabricator via cfe-commits]

theraven updated this revision to Diff 15.
theraven marked an inline comment as done.
theraven added a comment.

- Address Dustin's review comments.
- [objc-gnustep] Use $ in symbol names on Windows.
- Add fix from Dustin.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58724

Files:
  clang/lib/CodeGen/CGObjCGNU.cpp

Index: clang/lib/CodeGen/CGObjCGNU.cpp
===
--- clang/lib/CodeGen/CGObjCGNU.cpp
+++ clang/lib/CodeGen/CGObjCGNU.cpp
@@ -185,12 +185,16 @@
   (R.getVersion() >= VersionTuple(major, minor));
   }
 
+  Twine ManglePublicSymbol(StringRef Name) {
+return StringRef(CGM.getTriple().isOSBinFormatCOFF() ? "$_" : "._") + Name;
+  }
+
   std::string SymbolForProtocol(StringRef Name) {
-return (StringRef("._OBJC_PROTOCOL_") + Name).str();
+return (ManglePublicSymbol("OBJC_PROTOCOL_") + Name).str();
   }
 
   std::string SymbolForProtocolRef(StringRef Name) {
-return (StringRef("._OBJC_REF_PROTOCOL_") + Name).str();
+return (ManglePublicSymbol("OBJC_REF_PROTOCOL_") + Name).str();
   }
 
 
@@ -906,12 +910,15 @@
 ConstantStringSection
   };
   static const char *const SectionsBaseNames[8];
+  static const char *const PECOFFSectionsBaseNames[8];
   template
   std::string sectionName() {
-std::string name(SectionsBaseNames[K]);
-if (CGM.getTriple().isOSBinFormatCOFF())
+if (CGM.getTriple().isOSBinFormatCOFF()) {
+  std::string name(PECOFFSectionsBaseNames[K]);
   name += "$m";
-return name;
+  return name;
+}
+return SectionsBaseNames[K];
   }
   /// The GCC ABI superclass message lookup function.  Takes a pointer to a
   /// structure describing the receiver and the class, and a selector as
@@ -932,15 +939,19 @@
   bool EmittedClass = false;
   /// Generate the name of a symbol for a reference to a class.  Accesses to
   /// classes should be indirected via this.
+
+  typedef std::pair> EarlyInitPair;
+  std::vector EarlyInitList;
+
   std::string SymbolForClassRef(StringRef Name, bool isWeak) {
 if (isWeak)
-  return (StringRef("._OBJC_WEAK_REF_CLASS_") + Name).str();
+  return (ManglePublicSymbol("OBJC_WEAK_REF_CLASS_") + Name).str();
 else
-  return (StringRef("._OBJC_REF_CLASS_") + Name).str();
+  return (ManglePublicSymbol("OBJC_REF_CLASS_") + Name).str();
   }
   /// Generate the name of a class symbol.
   std::string SymbolForClass(StringRef Name) {
-return (StringRef("._OBJC_CLASS_") + Name).str();
+return (ManglePublicSymbol("OBJC_CLASS_") + Name).str();
   }
   void CallRuntimeFunction(CGBuilderTy &B, StringRef FunctionName,
   ArrayRef Args) {
@@ -994,10 +1005,13 @@
 
 llvm::Constant *isa = TheModule.getNamedGlobal(Sym);
 
-if (!isa)
+if (!isa) {
   isa = new llvm::GlobalVariable(TheModule, IdTy, /* isConstant */false,
   llvm::GlobalValue::ExternalLinkage, nullptr, Sym);
-else if (isa->getType() != PtrToIdTy)
+  if (CGM.getTriple().isOSBinFormatCOFF()) {
+cast(isa)->setDLLStorageClass(llvm::GlobalValue::DLLImportStorageClass);
+  }
+} else if (isa->getType() != PtrToIdTy)
   isa = llvm::ConstantExpr::getBitCast(isa, PtrToIdTy);
 
 //  struct
@@ -1012,7 +1026,11 @@
 
 ConstantInitBuilder Builder(CGM);
 auto Fields = Builder.beginStruct();
-Fields.add(isa);
+if (!CGM.getTriple().isOSBinFormatCOFF()) {
+  Fields.add(isa);
+} else {
+  Fields.addNullPointer(PtrTy);
+}
 // For now, all non-ASCII strings are represented as UTF-16.  As such, the
 // number of bytes is simply double the number of UTF-16 codepoints.  In
 // ASCII strings, the number of bytes is equal to the number of non-ASCII
@@ -1083,6 +1101,10 @@
   ObjCStrGV->setComdat(TheModule.getOrInsertComdat(StringName));
   ObjCStrGV->setVisibility(llvm::GlobalValue::HiddenVisibility);
 }
+if (CGM.getTriple().isOSBinFormatCOFF()) {
+  std::pair v{ObjCStrGV, 0};
+  EarlyInitList.emplace_back(Sym, v);
+}
 llvm::Constant *ObjCStr = llvm::ConstantExpr::getBitCast(ObjCStrGV, IdTy);
 ObjCStrings[Str] = ObjCStr;
 ConstantStrings.push_back(ObjCStr);
@@ -1196,6 +1218,33 @@
   ClassSymbol->setInitializer(new llvm::GlobalVariable(TheModule,
   Int8Ty, false, llvm::GlobalValue::ExternalWeakLinkage,
   nullptr, SymbolForClass(Name)));
+else {
+  if (CGM.getTriple().isOSBinFormatCOFF()) {
+IdentifierInfo &II = CGM.getContext().Idents.get(Name);
+TranslationUnitDecl *TUDecl = CGM.getContext().getTranslationUnitDecl();
+DeclContext *DC = TranslationUnitDecl::castToDeclContext(TUDecl);
+
+const ObjCInterfaceDecl *OID = nullptr;
+for (const auto &Result : DC->lookup(&II))
+  if ((OID = dyn_cast(Result)))
+break;
+
+// The first Interface we find may be a @class,
+// which

[PATCH] D58724: [gnustep-objc] Make the GNUstep v2 ABI work for Windows DLLs.

[David Chisnall via Phabricator via cfe-commits]

theraven updated this revision to Diff 193002.
theraven marked 2 inline comments as done.
theraven added a comment.

- Fix ownership with Twine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58724

Files:
  clang/lib/CodeGen/CGBlocks.cpp
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/test/CodeGenObjC/block-desc-str.m
  clang/test/CodeGenObjC/gnu-init.m

Index: clang/test/CodeGenObjC/gnu-init.m
===
--- clang/test/CodeGenObjC/gnu-init.m
+++ clang/test/CodeGenObjC/gnu-init.m
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-freebsd -S -emit-llvm -fobjc-runtime=gnustep-2.0 -o - %s | FileCheck %s -check-prefix=CHECK-NEW
 // RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -S -emit-llvm -fobjc-runtime=gnustep-2.0 -o - %s | FileCheck %s -check-prefix=CHECK-WIN
 // RUN: %clang_cc1 -triple x86_64-unknown-freebsd -S -emit-llvm -fobjc-runtime=gnustep-1.8 -o - %s | FileCheck %s -check-prefix=CHECK-OLD
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -fuse-init-array -S -emit-llvm -fobjc-runtime=gnustep-2.0 -o - %s | FileCheck %s -check-prefix=CHECK-INIT_ARRAY
 
 // Almost minimal Objective-C file, check that it emits calls to the correct
 // runtime entry points.
@@ -39,6 +40,7 @@
 
 // Check that the load function is manually inserted into .ctors.
 // CHECK-NEW: @.objc_ctor = linkonce hidden constant void ()* @.objcv2_load_function, section ".ctors", comdat
+// CHECK-INIT_ARRAY: @.objc_ctor = linkonce hidden constant void ()* @.objcv2_load_function, section ".init_array", comdat
 
 
 // Make sure that we provide null versions of everything so the __start /
Index: clang/test/CodeGenObjC/block-desc-str.m
===
--- clang/test/CodeGenObjC/block-desc-str.m
+++ clang/test/CodeGenObjC/block-desc-str.m
@@ -1,9 +1,13 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -emit-llvm -fobjc-runtime=gnustep-1.7 -fblocks -o - %s | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-llvm -fobjc-runtime=gcc -fblocks -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -emit-llvm -fobjc-runtime=gnustep-1.7 -fblocks -o - %s | FileCheck --check-prefix=CHECK-COMDAT %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-llvm -fobjc-runtime=gcc -fblocks -o - %s | FileCheck --check-prefix=CHECK-COMDAT %s
 // RUN: %clang_cc1 -triple=x86_64-apple-darwin10 -emit-llvm -fblocks -o - %s | FileCheck %s
 
 // Test that descriptor symbol names don't include '@'.
 
+// CHECK-COMDAT: $"__block_descriptor_40_8_32o_e5_v8\01?0l" = comdat any
+// CHECK-COMDAT: @[[STR:.*]] = private unnamed_addr constant [6 x i8] c"v8@?0\00"
+// CHECK-COMDAT: @"__block_descriptor_40_8_32o_e5_v8\01?0l" = linkonce_odr hidden unnamed_addr constant { i64, i64, i8*, i8*, i8*, {{.*}} } { i64 0, i64 40, i8* bitcast ({{.*}} to i8*), i8* bitcast ({{.*}} to i8*), i8* getelementptr inbounds ([6 x i8], [6 x i8]* @[[STR]], i32 0, i32 0), {{.*}} }, comdat, align 8
+
 // CHECK: @[[STR:.*]] = private unnamed_addr constant [6 x i8] c"v8@?0\00"
 // CHECK: @"__block_descriptor_40_8_32o_e5_v8\01?0l" = linkonce_odr hidden unnamed_addr constant { i64, i64, i8*, i8*, i8*, {{.*}} } { i64 0, i64 40, i8* bitcast ({{.*}} to i8*), i8* bitcast ({{.*}} to i8*), i8* getelementptr inbounds ([6 x i8], [6 x i8]* @[[STR]], i32 0, i32 0), {{.*}} }, align 8
 
Index: clang/lib/CodeGen/CGObjCGNU.cpp
===
--- clang/lib/CodeGen/CGObjCGNU.cpp
+++ clang/lib/CodeGen/CGObjCGNU.cpp
@@ -185,12 +185,16 @@
   (R.getVersion() >= VersionTuple(major, minor));
   }
 
+  Twine ManglePublicSymbol(Twine Name) {
+return StringRef(CGM.getTriple().isOSBinFormatCOFF() ? "$_" : "._") + Name;
+  }
+
   std::string SymbolForProtocol(StringRef Name) {
-return (StringRef("._OBJC_PROTOCOL_") + Name).str();
+return (ManglePublicSymbol("OBJC_PROTOCOL_") + Name).str();
   }
 
   std::string SymbolForProtocolRef(StringRef Name) {
-return (StringRef("._OBJC_REF_PROTOCOL_") + Name).str();
+return (ManglePublicSymbol("OBJC_REF_PROTOCOL_") + Name).str();
   }
 
 
@@ -906,12 +910,15 @@
 ConstantStringSection
   };
   static const char *const SectionsBaseNames[8];
+  static const char *const PECOFFSectionsBaseNames[8];
   template
   std::string sectionName() {
-std::string name(SectionsBaseNames[K]);
-if (CGM.getTriple().isOSBinFormatCOFF())
+if (CGM.getTriple().isOSBinFormatCOFF()) {
+  std::string name(PECOFFSectionsBaseNames[K]);
   name += "$m";
-return name;
+  return name;
+}
+return SectionsBaseNames[K];
   }
   /// The GCC ABI superclass message lookup function.  Takes a pointer to a
   /// structure describing the receiver and the class, and a selector as
@@ -932,15 +939,19 @@
   bool EmittedClass = false;
   /// Generate the name of a symbol for a reference to 

[PATCH] D58807: [CodeGen] COMDAT-fold block descriptors

[David Chisnall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL357363: COMDAT-fold block descriptors. (authored by 
theraven, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58807?vs=189193&id=193007#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58807

Files:
  cfe/trunk/lib/CodeGen/CGBlocks.cpp
  cfe/trunk/test/CodeGenObjC/block-desc-str.m


Index: cfe/trunk/lib/CodeGen/CGBlocks.cpp
===
--- cfe/trunk/lib/CodeGen/CGBlocks.cpp
+++ cfe/trunk/lib/CodeGen/CGBlocks.cpp
@@ -274,6 +274,8 @@
  /*constant*/ true, linkage, AddrSpace);
 
   if (linkage == llvm::GlobalValue::LinkOnceODRLinkage) {
+if (CGM.supportsCOMDAT())
+  global->setComdat(CGM.getModule().getOrInsertComdat(descName));
 global->setVisibility(llvm::GlobalValue::HiddenVisibility);
 global->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
   }
Index: cfe/trunk/test/CodeGenObjC/block-desc-str.m
===
--- cfe/trunk/test/CodeGenObjC/block-desc-str.m
+++ cfe/trunk/test/CodeGenObjC/block-desc-str.m
@@ -1,9 +1,13 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -emit-llvm 
-fobjc-runtime=gnustep-1.7 -fblocks -o - %s | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-llvm -fobjc-runtime=gcc 
-fblocks -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -emit-llvm 
-fobjc-runtime=gnustep-1.7 -fblocks -o - %s | FileCheck 
--check-prefix=CHECK-COMDAT %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-llvm -fobjc-runtime=gcc 
-fblocks -o - %s | FileCheck --check-prefix=CHECK-COMDAT %s
 // RUN: %clang_cc1 -triple=x86_64-apple-darwin10 -emit-llvm -fblocks -o - %s | 
FileCheck %s
 
 // Test that descriptor symbol names don't include '@'.
 
+// CHECK-COMDAT: $"__block_descriptor_40_8_32o_e5_v8\01?0l" = comdat any
+// CHECK-COMDAT: @[[STR:.*]] = private unnamed_addr constant [6 x i8] 
c"v8@?0\00"
+// CHECK-COMDAT: @"__block_descriptor_40_8_32o_e5_v8\01?0l" = linkonce_odr 
hidden unnamed_addr constant { i64, i64, i8*, i8*, i8*, {{.*}} } { i64 0, i64 
40, i8* bitcast ({{.*}} to i8*), i8* bitcast ({{.*}} to i8*), i8* getelementptr 
inbounds ([6 x i8], [6 x i8]* @[[STR]], i32 0, i32 0), {{.*}} }, comdat, align 8
+
 // CHECK: @[[STR:.*]] = private unnamed_addr constant [6 x i8] c"v8@?0\00"
 // CHECK: @"__block_descriptor_40_8_32o_e5_v8\01?0l" = linkonce_odr hidden 
unnamed_addr constant { i64, i64, i8*, i8*, i8*, {{.*}} } { i64 0, i64 40, i8* 
bitcast ({{.*}} to i8*), i8* bitcast ({{.*}} to i8*), i8* getelementptr 
inbounds ([6 x i8], [6 x i8]* @[[STR]], i32 0, i32 0), {{.*}} }, align 8
 


Index: cfe/trunk/lib/CodeGen/CGBlocks.cpp
===
--- cfe/trunk/lib/CodeGen/CGBlocks.cpp
+++ cfe/trunk/lib/CodeGen/CGBlocks.cpp
@@ -274,6 +274,8 @@
  /*constant*/ true, linkage, AddrSpace);
 
   if (linkage == llvm::GlobalValue::LinkOnceODRLinkage) {
+if (CGM.supportsCOMDAT())
+  global->setComdat(CGM.getModule().getOrInsertComdat(descName));
 global->setVisibility(llvm::GlobalValue::HiddenVisibility);
 global->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
   }
Index: cfe/trunk/test/CodeGenObjC/block-desc-str.m
===
--- cfe/trunk/test/CodeGenObjC/block-desc-str.m
+++ cfe/trunk/test/CodeGenObjC/block-desc-str.m
@@ -1,9 +1,13 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -emit-llvm -fobjc-runtime=gnustep-1.7 -fblocks -o - %s | FileCheck %s
-// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-llvm -fobjc-runtime=gcc -fblocks -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -emit-llvm -fobjc-runtime=gnustep-1.7 -fblocks -o - %s | FileCheck --check-prefix=CHECK-COMDAT %s
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-llvm -fobjc-runtime=gcc -fblocks -o - %s | FileCheck --check-prefix=CHECK-COMDAT %s
 // RUN: %clang_cc1 -triple=x86_64-apple-darwin10 -emit-llvm -fblocks -o - %s | FileCheck %s
 
 // Test that descriptor symbol names don't include '@'.
 
+// CHECK-COMDAT: $"__block_descriptor_40_8_32o_e5_v8\01?0l" = comdat any
+// CHECK-COMDAT: @[[STR:.*]] = private unnamed_addr constant [6 x i8] c"v8@?0\00"
+// CHECK-COMDAT: @"__block_descriptor_40_8_32o_e5_v8\01?0l" = linkonce_odr hidden unnamed_addr constant { i64, i64, i8*, i8*, i8*, {{.*}} } { i64 0, i64 40, i8* bitcast ({{.*}} to i8*), i8* bitcast ({{.*}} to i8*), i8* getelementptr inbounds ([6 x i8], [6 x i8]* @[[STR]], i32 0, i32 0), {{.*}} }, comdat, align 8
+
 // CHECK: @[[STR:.*]] = private unnamed_addr constant [6 x i8] c"v8@?0\00"
 // CHECK: @"__block_descriptor_40_8_32o_e5_v8\01?0l" = 

[PATCH] D58724: [gnustep-objc] Make the GNUstep v2 ABI work for Windows DLLs.

[David Chisnall via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rC357364: [gnustep-objc] Make the GNUstep v2 ABI work for 
Windows DLLs. (authored by theraven, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D58724?vs=193002&id=193008#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D58724

Files:
  lib/CodeGen/CGObjCGNU.cpp
  test/CodeGenObjC/gnu-init.m

Index: test/CodeGenObjC/gnu-init.m
===
--- test/CodeGenObjC/gnu-init.m
+++ test/CodeGenObjC/gnu-init.m
@@ -73,29 +73,28 @@
 
 
 // Make sure all of our section boundary variables are emitted correctly.
-// CHECK-WIN-DAG: @__start___objc_selectors = linkonce_odr hidden global %.objc_section_sentinel zeroinitializer, section "__objc_selectors$a", comdat, align 1
-// CHECK-WIN-DAG: @__stop__objc_selectors = linkonce_odr hidden global %.objc_section_sentinel zeroinitializer, section "__objc_selectors$z", comdat, align 1
-// CHECK-WIN-DAG: @__start___objc_classes = linkonce_odr hidden global %.objc_section_sentinel zeroinitializer, section "__objc_classes$a", comdat, align 1
-// CHECK-WIN-DAG: @__stop__objc_classes = linkonce_odr hidden global %.objc_section_sentinel zeroinitializer, section "__objc_classes$z", comdat, align 1
-// CHECK-WIN-DAG: @__start___objc_class_refs = linkonce_odr hidden global %.objc_section_sentinel zeroinitializer, section "__objc_class_refs$a", comdat, align 1
-// CHECK-WIN-DAG: @__stop__objc_class_refs = linkonce_odr hidden global %.objc_section_sentinel zeroinitializer, section "__objc_class_refs$z", comdat, align 1
-// CHECK-WIN-DAG: @__start___objc_cats = linkonce_odr hidden global %.objc_section_sentinel zeroinitializer, section "__objc_cats$a", comdat, align 1
-// CHECK-WIN-DAG: @__stop__objc_cats = linkonce_odr hidden global %.objc_section_sentinel zeroinitializer, section "__objc_cats$z", comdat, align 1
-// CHECK-WIN-DAG: @__start___objc_protocols = linkonce_odr hidden global %.objc_section_sentinel zeroinitializer, section "__objc_protocols$a", comdat, align 1
-// CHECK-WIN-DAG: @__stop__objc_protocols = linkonce_odr hidden global %.objc_section_sentinel zeroinitializer, section "__objc_protocols$z", comdat, align 1
-// CHECK-WIN-DAG: @__start___objc_protocol_refs = linkonce_odr hidden global %.objc_section_sentinel zeroinitializer, section "__objc_protocol_refs$a", comdat, align 1
-// CHECK-WIN-DAG: @__stop__objc_protocol_refs = linkonce_odr hidden global %.objc_section_sentinel zeroinitializer, section "__objc_protocol_refs$z", comdat, align 1
-// CHECK-WIN-DAG: @__start___objc_class_aliases = linkonce_odr hidden global %.objc_section_sentinel zeroinitializer, section "__objc_class_aliases$a", comdat, align 1
-// CHECK-WIN-DAG: @__stop__objc_class_aliases = linkonce_odr hidden global %.objc_section_sentinel zeroinitializer, section "__objc_class_aliases$z", comdat, align 1
-// CHECK-WIN-DAG: @__start___objc_constant_string = linkonce_odr hidden global %.objc_section_sentinel zeroinitializer, section "__objc_constant_string$a", comdat, align 1
-// CHECK-WIN-DAG: @__stop__objc_constant_string = linkonce_odr hidden global %.objc_section_sentinel zeroinitializer, section "__objc_constant_string$z", comdat, align 1
-// CHECK-WIN: @.objc_init = linkonce_odr hidden global { i64, %.objc_section_sentinel*, %.objc_section_sentinel*, %.objc_section_sentinel*, %.objc_section_sentinel*, %.objc_section_sentinel*, %.objc_section_sentinel*, %.objc_section_sentinel*, %.objc_section_sentinel*, %.objc_section_sentinel*, %.objc_section_sentinel*, %.objc_section_sentinel*, %.objc_section_sentinel*, %.objc_section_sentinel*, %.objc_section_sentinel*, %.objc_section_sentinel*, %.objc_section_sentinel* } { i64 0, %.objc_section_sentinel* @__start___objc_selectors, %.objc_section_sentinel* @__stop__objc_selectors, %.objc_section_sentinel* @__start___objc_classes, %.objc_section_sentinel* @__stop__objc_classes, %.objc_section_sentinel* @__start___objc_class_refs, %.objc_section_sentinel* @__stop__objc_class_refs, %.objc_section_sentinel* @__start___objc_cats, %.objc_section_sentinel* @__stop__objc_cats, %.objc_section_sentinel* @__start___objc_protocols, %.objc_section_sentinel* @__stop__objc_protocols, %.objc_section_sentinel* @__start___objc_protocol_refs, %.objc_section_sentinel* @__stop__objc_protocol_refs, %.objc_section_sentinel* @__start___objc_class_aliases, %.objc_section_sentinel* @__stop__objc_class_aliases, %.objc_section_sentinel* @__start___objc_constant_string, %.objc_section_sentinel* @__stop__objc_constant_string }, comdat, align 8
+// CHECK-WIN-DAG: @"__stop.objcrt$SEL" = linkonce_odr hidden global %.objc_section_sentinel zeroinitializer, section ".objcrt$SEL$z", comdat, align 8
+// CHECK-WIN-DAG: @"__start_.objcrt$CLS" = linkonce_odr hidden global %.objc_section_senti

[PATCH] D61634: [clang/llvm] Allow efficient implementation of libc's memory functions in C/C++

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

I wonder if a list of comma-separated names is the right approach.  Would it 
make more sense to add a new attribute for each of the helpers, such as 
`#no-runtime-for-memcpy`? That should make querying simpler (one lookup in the 
table, rather than a lookup and a string scan) and also make it easier to add 
and remove attributes (merging is now just a matter of trying to add all of 
them, with the existing logic to deduplicate repeated attributes working).

We probably need to also carefully audit all of the transform passes to find 
ones that insert calls.  Last time I looked, there were four places in LLVM 
where memcpy could be expanded, I wonder if there are a similar number where it 
can be synthesised...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61634



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

[PATCH] D52344: [Clang][CodeGen][ObjC]: Fix non-bridged CoreFoundation builds on ELF targets that use `-fconstant-cfstrings`.

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

In https://reviews.llvm.org/D52344#1242530, @rjmccall wrote:

> In https://reviews.llvm.org/D52344#1242451, @kristina wrote:
>
> > Would you be okay with me renaming `cfstring` to `.cfstring` for ELF so 
> > it's in line with ELF section naming conventions (I'm not entirely sure if 
> > that could cause issues with ObjC stuff)? And yes I think it's best to 
> > avoid that code-path altogether if it turns out to be a constant as that's 
> > likely from the declaration of the type, I'll write a FileCheck test in a 
> > moment and check that I can apply the same logic to ELF aside from the DLL 
> > stuff as CoreFoundation needs to export the symbol somehow while preserving 
> > the implicit extern attribute for every other TU that comes from the 
> > builtin that `CFSTR` expands to. Is what I'm suggesting making sense? If 
> > not, let me know, I may be misunderstanding what's happening here.
>
>
> Following the ELF conventions seems like the right thing to do, assuming it 
> doesn't cause compatibility problems.  David Chisnall might know best here.


I don't believe it will have any compatibility issues.  We expose builtins for 
creating CF- and NSString objects from C code.  For Apple targets, these are 
equivalent.  For targets that don't care about Objective-C interop, the CF 
version is sensible because it doesn't introduce any dependencies on an 
Objective-C runtime.  For code targeting a non-Apple runtime and wanting CF / 
Foundation interop, the `CFSTR` macro expands to the NS version.

As others have pointed out, the section must be a valid C identifier for the 
magic `__start_` and `__stop_` symbols to be created.  I don't really like this 
limitation and it would be nice if we could improve lld (and encourage binutils 
to do the same) so that `__start_.cfstring` and `__stop_.cfstring` could be 
used to create symbols pointing to the start and end of a section because it's 
useful to name things in such a way that they can't conflict with any valid C 
identifier.  With PE/COFF, we have a mechanism for more precise control and can 
give sections any names that we want (and also arrange for start and end 
symbols for subsections).


Repository:
  rC Clang

https://reviews.llvm.org/D52344



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

[PATCH] D47233: [CodeGen] Emit MSVC RTTI for Obj-C EH types

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

In https://reviews.llvm.org/D47233#1129810, @DHowett-MSFT wrote:

> We ran into some critical issues with this approach on x64. The pointers in 
> the exception record are supposed to be image-relative instead of absolute, 
> so I tried to make them absolute to libobjc2's load address, but I never 
> quite solved it.
>
> A slightly better-documented and cleaner version of the code you linked is 
> here 
> .


(For the reference of other people, since Dustin and I discussed this offline a 
while back)

This is fixed upstream.  We use a stack address as our base address and 
construct stack-variable-relative offsets.  This is safe to do because the only 
requirement for Win64 EH is that the pointers be relative to some arbitrary 
base and in the SEH model the throwing stack frame remains live until the last 
catch.


Repository:
  rC Clang

https://reviews.llvm.org/D47233



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

[PATCH] D52581: [AST] Revert mangling changes from r339428

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

> I would have done the same for the GNUstep RTTI here, except I don't actually
>  see the code for that anywhere, and no tests seem to break either, so I
>  believe it's not upstreamed yet.

I'm not sure I understand this comment.  The compiler code is in LLVM and the 
tests that you've modified are the ones that would fail.  The corresponding 
code is upstreamed in the runtime, to generate EH metadata for the throw with 
the same mangling.  If you are going to change the mangling, please make sure 
that the existing mangling is preserved for EH when compiling for the GNUstep / 
Microsoft ABI on Windows.

I'm also deeply unconvinced by the idea that it's okay to have a `struct I` in 
one compilation unit and a `@class I` in another.  The `struct` version will 
not do any of the correct things with respect to memory management.  Code that 
has this idiom is very likely to be broken.


Repository:
  rC Clang

https://reviews.llvm.org/D52581



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

[PATCH] D52581: [AST] Revert mangling changes from r339428

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

In https://reviews.llvm.org/D52581#1248306, @smeenai wrote:

> The simplest option is something like https://reviews.llvm.org/P8109, where 
> we add a `.objc` discriminator when mangling the RTTI itself. It would 
> require the GNUStep runtime for Windows to be altered accordingly (e.g. 
> https://github.com/gnustep/libobjc2/blob/master/eh_win32_msvc.cc#L85 would 
> have to use `".objc.PAU"` instead of `".PAU.objc_cls_"`); would that be 
> acceptable, @theraven and @DHowett-MSFT?


I think that's fine.  We have shipped the new ABI for ELF platforms, but not 
yet for Windows (and we have a few other patches still needed in Clang), and 
haven't finished the WinObjC integration, so we don't yet have a stable ABI 
that anyone cares about preserving for this - we explicitly disabled the 
Windows code paths in clang 7.0, so no one can use them with an unstable ABI 
without patching clang.  As long as we come up with some consistent mangling 
and it's sensibly documented, I'm happy to make the runtime changes for it.


Repository:
  rC Clang

https://reviews.llvm.org/D52581



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

[PATCH] D55802: Change CGObjC to use objc intrinsics instead of runtime methods

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.
Herald added a project: LLVM.

After some bisection, it appears that this is the revision that introduced the 
regression in the GNUstep Objective-C runtime test suite that I reported on the 
list a few weeks ago.  In this is the test (compiled with 
`-fobjc-runtime=gnustep-2.0 -O3` and an ELF triple):

https://github.com/gnustep/libobjc2/blob/master/Test/AssociatedObject.m

After this change, Early CSE w/ MemorySSA is determining that the second load 
of `deallocCalled` is redundant.  The code goes from:

%7 = load i1, i1* @deallocCalled, align 1
br i1 %7, label %8, label %9
  
  ; :8:  ; preds = %0
call void @__assert(i8* getelementptr inbounds ([5 x i8], [5 x i8]* 
@__func__.main, i64 0, i64 0), i8* getelementptr inbounds ([27 x i8], [27 x 
i8]* @.str, i64 0, i64 0), i32 26, i8* getelementptr inbounds ([15 x i8], [15 x 
i8]* @.str.1, i64 0, i64 0)) #5
unreachable
  
  ; :9:  ; preds = %0
call void @llvm.objc.autoreleasePoolPop(i8* %1)
%10 = load i1, i1* @deallocCalled, align 1
br i1 %10, label %12, label %11
  
  ; :11: ; preds = %9
call void @__assert(i8* getelementptr inbounds ([5 x i8], [5 x i8]* 
@__func__.main, i64 0, i64 0), i8* getelementptr inbounds ([27 x i8], [27 x 
i8]* @.str, i64 0, i64 0), i32 29, i8* getelementptr inbounds ([14 x i8], [14 x 
i8]* @.str.2, i64 0, i64 0)) #5
unreachable

to:

%7 = load i1, i1* @deallocCalled, align 1
br i1 %7, label %8, label %9
  
  ; :8:  ; preds = %0
call void @__assert(i8* getelementptr inbounds ([5 x i8], [5 x i8]* 
@__func__.main, i64 0, i64 0), i8* getelementptr inbounds ([27 x i8], [27 x 
i8]* @.str, i64 0, i64 0), i32 26, i8* getelementptr inbounds ([15 x i8], [15 x 
i8]* @.str.1, i64 0, i64 0)) #5
unreachable
  
  ; :9:  ; preds = %0
call void @llvm.objc.autoreleasePoolPop(i8* %1)
br i1 %7, label %11, label %10
  
  ; :10: ; preds = %9
call void @__assert(i8* getelementptr inbounds ([5 x i8], [5 x i8]* 
@__func__.main, i64 0, i64 0), i8* getelementptr inbounds ([27 x i8], [27 x 
i8]* @.str, i64 0, i64 0), i32 29, i8* getelementptr inbounds ([14 x i8], [14 x 
i8]* @.str.2, i64 0, i64 0)) #5
unreachable

Later optimisations then determine that, because the assert does not return, 
the only possible value for %7 is false and cause the second assert to fire 
unconditionally.

It appears that we are not correctly modelling the side effects of the 
`llvm.objc.autoreleasePoolPop` intrinsic, but it's not entirely clear why not.  
The same test compiled for the macos runtime does not appear to exhibit the 
same behaviour.  The previous revision, where we emitted a call to 
`objc_autoreleasePoolPop` and not the intrinsic worked correctly, but with this 
change the optimisers are assuming that no globals can be modified across an 
autorelease pool pop operation (at least, in some situations).

Looking at the definition of the intrinsic, I don't see anything wrong, so I 
still suspect that there is a MemorySSA bug that this has uncovered, rather 
than anything wrong in this series of commits.  Any suggestions as to where to 
look would be appreciated.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55802



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

[PATCH] D64493: [Driver] Don't pass --dynamic-linker to ld on Solaris

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

I think this was simply mirroring what gcc 4.something did on Solaris.


Repository:
  rC Clang

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

https://reviews.llvm.org/D64493



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

[PATCH] D61974: [ObjC] Fix encoding of ObjC pointer types that are pointers to typedefs

[David Chisnall via Phabricator via cfe-commits]

theraven requested changes to this revision.
theraven added inline comments.
This revision now requires changes to proceed.



Comment at: lib/AST/ASTContext.cpp:6973
 QualType PointeeTy = OPT->getPointeeType();
-if (!Options.EncodingProperty() &&
+if (getLangOpts().ObjCRuntime.isGNUFamily() &&
+!Options.EncodingProperty() &&

Please can we at least make this check just for the GCC runtime?  I'm not sure 
it's even needed there.  I've previously had to write code that works around 
this and have always considered it a bug in GCC, rather than a feature that I'd 
like us to copy, so I'd also be happy with just deleting this code path 
entirely.



Repository:
  rC Clang

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

https://reviews.llvm.org/D61974



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

[PATCH] D61974: [ObjC] Fix encoding of ObjC pointer types that are pointers to typedefs

[David Chisnall via Phabricator via cfe-commits]

theraven added inline comments.



Comment at: lib/AST/ASTContext.cpp:6973
 QualType PointeeTy = OPT->getPointeeType();
-if (!Options.EncodingProperty() &&
+if (getLangOpts().ObjCRuntime.isGNUFamily() &&
+!Options.EncodingProperty() &&

ahatanak wrote:
> theraven wrote:
> > Please can we at least make this check just for the GCC runtime?  I'm not 
> > sure it's even needed there.  I've previously had to write code that works 
> > around this and have always considered it a bug in GCC, rather than a 
> > feature that I'd like us to copy, so I'd also be happy with just deleting 
> > this code path entirely.
> > 
> Are we allowed to delete the code path entirely? That would clean up the code 
> a bit, but I assume it would also break GCC runtime users' code.
I'd be happy with deleting it entirely.  Nothing in the gcc runtime itself 
depends on it and all of the code that I've dealt with that works with the GCC 
runtime and this functionality has explicit work arounds for this behaviour in 
GCC.



Repository:
  rC Clang

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

https://reviews.llvm.org/D61974



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

[PATCH] D61974: [ObjC] Fix encoding of ObjC pointer types that are pointers to typedefs

[David Chisnall via Phabricator via cfe-commits]

theraven accepted this revision.
theraven added a comment.
This revision is now accepted and ready to land.

LGTM.  I wonder if we have any other ugly GCC bug compatibility parts in 
clang's Objective-C implementation...


Repository:
  rC Clang

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

https://reviews.llvm.org/D61974



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

[PATCH] D55229: [COFF] Statically link certain runtime library functions

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

If we're going to change the default, please at least add a flag to allow 
callers to opt into dlimport.  We can then make that dependent on 
`-static-objc` or similar.




Comment at: CodeGenObjC/gnu-init.m:103
+// Make sure we do not have dllimport on the load function
+// CHECK-WIN: declare dso_local void @__objc_load
 

compnerd wrote:
> rnk wrote:
> > compnerd wrote:
> > > rnk wrote:
> > > > mgrang wrote:
> > > > > I had to remove dllimport in this and the next unit test. I am not 
> > > > > sure of the wider implications of this change. Maybe controlling this 
> > > > > via a flag (like -flto-visibility-public-std) is a better/more 
> > > > > localized option?
> > > > These are the ones that I think @compnerd really cares about since the 
> > > > objc runtime is typically dynamically linked and frequently called. We 
> > > > might want to arrange things such that we have a separate codepath for 
> > > > ObjC runtime helpers. I'm surprised we don't already have such a code 
> > > > path.
> > > I think that @theraven would care more about this code path than I.  I 
> > > think that this change may be wrong, because the load here is supposed to 
> > > be in the ObjC runtime, and this becoming local to the module would break 
> > > the global registration.
> > We just set dso_local whenever something isn't dllimport, even if it's a 
> > lie sometimes. I think everything would work as intended with a linker 
> > thunk. It's "true" as far as LLVM is concerned for the purposes of emitting 
> > relocations.
> Ah, okay, then, this might be okay.  However, the use of `dllimport` here 
> would force that the runtime to be called.  Then again it is possible to 
> statically link ...
`__objc_load` is a function defined in objc.dll.  It absolutely does want to be 
dlimport, or the linker won't be able to find it.



Comment at: CodeGenObjCXX/msabi-stret.mm:16
 
-// CHECK: declare dllimport void @objc_msgSend_stret(i8*, i8*, ...)
+// CHECK: declare dso_local void @objc_msgSend_stret(i8*, i8*, ...)
 // CHECK-NOT: declare dllimport void @objc_msgSend(i8*, i8*, ...)

Similarly, the `objc_msgSend` family are defined in objc.dll and so need to be 
`dlimport`ed.


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

https://reviews.llvm.org/D55229



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

[PATCH] D55640: [clang-tidy] Implement a check for large Objective-C type encodings 🔍

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

I wonder if we want to have an option to elide ObjC type info for all non-POD 
C++ types.  Nothing that you do with the type encoding is likely to be correct 
(for example, you can see the pointer field in a `std::shared_ptr`, but you 
can't see that changes to it need to update reference counts) so it probably 
does more harm than good.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55640



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

[PATCH] D55869: Convert some ObjC retain/release msgSends to runtime calls

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

This should be fine for the GNUstep runtime (the GCC runtime doesn't support 
ARC at all).  My main concern is that it will break already-released versions 
of the runtime built with a newer version of clang.  I can easily enable a new 
flag in the next release, but doing so for older ones is more problematic.


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

https://reviews.llvm.org/D55869



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

[PATCH] D54539: [CodeGen] Replace '@' characters in block descriptors' symbol names with '\1' on ELF targets.

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

Please can we either merge this or revert the original change that introduced 
the bug that this is fixing?  It's now been 6 months since blocks generated 
invalid ELF symbols.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54539



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

[PATCH] D54539: [CodeGen] Replace '@' characters in block descriptors' symbol names with '\1' on ELF targets.

[David Chisnall via Phabricator via cfe-commits]

theraven accepted this revision.
theraven added a comment.
This revision is now accepted and ready to land.

Looks like a much cleaner change.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54539



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

[PATCH] D68108: Redeclare Objective-C property accessors inside the ObjCImplDecl in which they are synthesized.

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

The changes for the GNU runtimes look correct to me.  We're missing a bunch of 
tests there, unfortunately.


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

https://reviews.llvm.org/D68108



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

[PATCH] D45305: ObjCGNU: Fix empty v3 protocols being emitted two fields short

[David Chisnall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC329882: ObjCGNU: Fix empty v3 protocols being emitted two 
fields short (authored by theraven, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D45305

Files:
  lib/CodeGen/CGObjCGNU.cpp
  test/CodeGenObjC/gnu-empty-protocol-v3.m


Index: lib/CodeGen/CGObjCGNU.cpp
===
--- lib/CodeGen/CGObjCGNU.cpp
+++ lib/CodeGen/CGObjCGNU.cpp
@@ -1748,11 +1748,13 @@
   llvm::ConstantInt::get(Int32Ty, ProtocolVersion), IdTy));
 
   Elements.add(MakeConstantString(ProtocolName, ".objc_protocol_name"));
-  Elements.add(ProtocolList);
-  Elements.add(MethodList);
-  Elements.add(MethodList);
-  Elements.add(MethodList);
-  Elements.add(MethodList);
+  Elements.add(ProtocolList); /* .protocol_list */
+  Elements.add(MethodList);   /* .instance_methods */
+  Elements.add(MethodList);   /* .class_methods */
+  Elements.add(MethodList);   /* .optional_instance_methods */
+  Elements.add(MethodList);   /* .optional_class_methods */
+  Elements.add(NULLPtr);  /* .properties */
+  Elements.add(NULLPtr);  /* .optional_properties */
   return Elements.finishAndCreateGlobal(".objc_protocol",
 CGM.getPointerAlign());
 }
Index: test/CodeGenObjC/gnu-empty-protocol-v3.m
===
--- test/CodeGenObjC/gnu-empty-protocol-v3.m
+++ test/CodeGenObjC/gnu-empty-protocol-v3.m
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -triple i686-pc-linux-gnu -fobjc-runtime=gnustep-1.9 
-emit-llvm -o - %s | FileCheck %s
+
+@protocol X;
+
+__attribute__((objc_root_class))
+@interface Z 
+@end
+
+@implementation Z
+@end
+
+// CHECK:  @.objc_protocol_list = internal global { i8*, i32, [0 x i8*] } 
zeroinitializer, align 4
+// CHECK:  @.objc_method_list = internal global { i32, [0 x { i8*, i8* }] 
} zeroinitializer, align 4
+// CHECK:  @.objc_protocol_name = private unnamed_addr constant [2 x i8] 
c"X\00", align 1
+// CHECK:  @.objc_protocol = internal global { i8*, i8*, { i8*, i32, [0 x 
i8*] }*, { i32, [0 x { i8*, i8* }] }*, { i32, [0 x { i8*, i8* }] }*, { i32, [0 
x { i8*, i8* }] }*, { i32, [0 x { i8*, i8* }] }*, i8*, i8* } {
+// CHECK-SAME: i8* inttoptr (i32 3 to i8*),
+// CHECK-SAME: i8* getelementptr inbounds ([2 x i8], [2 x i8]* 
@.objc_protocol_name, i32 0, i32 0),
+// CHECK-SAME: { i8*, i32, [0 x i8*] }* @.objc_protocol_list,
+// CHECK-SAME: { i32, [0 x { i8*, i8* }] }* @.objc_method_list,
+// CHECK-SAME: { i32, [0 x { i8*, i8* }] }* @.objc_method_list,
+// CHECK-SAME: { i32, [0 x { i8*, i8* }] }* @.objc_method_list,
+// CHECK-SAME: { i32, [0 x { i8*, i8* }] }* @.objc_method_list,
+// CHECK-SAME: i8* null,
+// CHECK-SAME: i8* null
+// CHECK-SAME: }, align 4


Index: lib/CodeGen/CGObjCGNU.cpp
===
--- lib/CodeGen/CGObjCGNU.cpp
+++ lib/CodeGen/CGObjCGNU.cpp
@@ -1748,11 +1748,13 @@
   llvm::ConstantInt::get(Int32Ty, ProtocolVersion), IdTy));
 
   Elements.add(MakeConstantString(ProtocolName, ".objc_protocol_name"));
-  Elements.add(ProtocolList);
-  Elements.add(MethodList);
-  Elements.add(MethodList);
-  Elements.add(MethodList);
-  Elements.add(MethodList);
+  Elements.add(ProtocolList); /* .protocol_list */
+  Elements.add(MethodList);   /* .instance_methods */
+  Elements.add(MethodList);   /* .class_methods */
+  Elements.add(MethodList);   /* .optional_instance_methods */
+  Elements.add(MethodList);   /* .optional_class_methods */
+  Elements.add(NULLPtr);  /* .properties */
+  Elements.add(NULLPtr);  /* .optional_properties */
   return Elements.finishAndCreateGlobal(".objc_protocol",
 CGM.getPointerAlign());
 }
Index: test/CodeGenObjC/gnu-empty-protocol-v3.m
===
--- test/CodeGenObjC/gnu-empty-protocol-v3.m
+++ test/CodeGenObjC/gnu-empty-protocol-v3.m
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -triple i686-pc-linux-gnu -fobjc-runtime=gnustep-1.9 -emit-llvm -o - %s | FileCheck %s
+
+@protocol X;
+
+__attribute__((objc_root_class))
+@interface Z 
+@end
+
+@implementation Z
+@end
+
+// CHECK:  @.objc_protocol_list = internal global { i8*, i32, [0 x i8*] } zeroinitializer, align 4
+// CHECK:  @.objc_method_list = internal global { i32, [0 x { i8*, i8* }] } zeroinitializer, align 4
+// CHECK:  @.objc_protocol_name = private unnamed_addr constant [2 x i8] c"X\00", align 1
+// CHECK:  @.objc_protocol = internal global { i8*, i8*, { i8*, i32, [0 x i8*] }*, { i32, [0 x { i8*, i8* }] }*, { i32, [0 x { i8*, i8* }] }*, { i32, [0 x { i8*, i8* }] }*, { i32, [0 x { i8*, i8* }] }*, i8*, i8* } {
+// CHECK-SAME: i8* inttoptr (i32 3 to i8*),
+// CHECK-SAME: i8* getelementptr inbounds ([2 x i8], [2 x i8]* @.objc_protocol_name, i32 0, i32 0),
+// C

[PATCH] D46052: GNUstep Objective-C ABI version 2

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

In https://reviews.llvm.org/D46052#1078597, @rjmccall wrote:

> Are you asking for a code review or a design review of the ABI?


I don't think a design review is appropriate here, I am asking for a code 
review.

> The second would be much easier to do from a design document.

If you'd be willing to do a design review, I'd be very happy to send you a copy 
of the ABI document.


Repository:
  rC Clang

https://reviews.llvm.org/D46052



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

[PATCH] D46052: GNUstep Objective-C ABI version 2

[David Chisnall via Phabricator via cfe-commits]

theraven added inline comments.



Comment at: lib/CodeGen/CGObjCGNU.cpp:977
+if ((CGM.getTarget().getPointerWidth(0) == 64) &&
+(SL->getLength() < 9) && !isNonASCII) {
+  // Tiny strings are (roughly):

rjmccall wrote:
> Please hoist `SL->getLength()` into a variable.
> 
> Also, consider breaking this bit of code (maybe just building a `uint64_t`?) 
> into a separate function.
I don't think separating it would simplify the code much.  It's 8 lines of 
non-comment code and you'd probably need at least three of them in the caller.



Comment at: lib/CodeGen/CGObjCGNU.cpp:1077
+std::replace(StringName.begin(), StringName.end(),
+  '@', '\1');
+auto *ObjCStrGV =

rjmccall wrote:
> Is `@` really the only illegal character in ELF symbol names?  Surely at 
> least `\0` as well.  And your mangling will collide strings if they contain 
> `\1`.
> 
> I think you should probably just have this use internal linkage (and a 
> meaningless symbol name) for strings containing bad characters.
No, I copied the mangling for selectors without engaging my brain.  I've now 
replaced it with something much more conservative.  I might tweak it a bit 
before the next release, but this seems to give pretty good coverage.


Repository:
  rC Clang

https://reviews.llvm.org/D46052



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

[PATCH] D46052: GNUstep Objective-C ABI version 2

[David Chisnall via Phabricator via cfe-commits]

theraven added inline comments.



Comment at: lib/CodeGen/CGObjCGNU.cpp:439
+ ArrayRef IvarOffsets,
+ ArrayRef IvarAlign,
+ ArrayRef IvarOwnership);

DHowett-MSFT wrote:
> While we're here, is there value in storing the ivar size in layout as well, 
> so that the runtime doesn't need to calculate it from the distance to the 
> next ivar/end of the instance?
Normally the runtime calculates it from the type encoding, if it's required.  I 
agree that it might be nice to have though.  Do you have strong feelings about 
needing it in the 2.0 ABI?  The looser coupling means that it would be easy to 
add in the 2.1 ABI if we want it later.  Are you seeing cases where the runtime 
is calculating it incorrectly because of insufficient information in the type 
encoding, or where calculating it is causing performance problems?



Comment at: lib/CodeGen/CGObjCGNU.cpp:1402
+Stop->setVisibility(llvm::GlobalValue::HiddenVisibility);
+return { Start, Stop };
+  }

DHowett-MSFT wrote:
> This should be readily expandable for PE/COFF, but we'll need to change some 
> of the section names to fit better. Is it worth abstracting the names of the 
> sections across the target format somehow?
> 
> (pe/coff will need to emit COMDAT symbols pointing into lexicographically 
> sortable section names that the linker can fold away)
As I understand it, we can just append `$m` for the symbol names on PE/COFF and 
then create zero-size symbols with `$a` and `$z` suffixes for the end?  The 
symbol names should be able to be the same, but if that's not possible then it 
would be a good idea to change them so that ELF and PE/COFF binaries look the 
same.



Comment at: lib/CodeGen/CGObjCGNU.cpp:1446
+/*isConstant*/true, llvm::GlobalValue::LinkOnceAnyLinkage,
+LoadFunction, ".objc_ctor");
+// Check that this hasn't been renamed.  This shouldn't happen, because

DHowett-MSFT wrote:
> Is there a way to ensure that objc_load happens before other static 
> initializers across the entire set of linker inputs?
This part is quite ELF-specific, and on ELF unfortunately the order of 
constructor initialisation is undefined (it will be in whatever order the 
linker decides to put them in).  Running the Objective-C code first isn't 
necessarily correct: C++ static constructors and Objective-C `+load` methods 
could each depend on the other (and C++ on ObjC classes having been loaded).

Of course, if you have control over the linker (or possibly even a linker 
script) then you can adjust the ordering however you want...


Repository:
  rC Clang

https://reviews.llvm.org/D46052



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

[PATCH] D10480: Implement shared_mutex re: N4508

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

I missed this when it went in and coming across the code now I'm quite 
surprised that it did.  Why is `shared_mutex` not implemented as a wrapper 
around rwlocks (pthreads and Windows both provide this abstraction)?  The 
current implementation looks a lot less efficient than the system version on 
any operating system that I'm familiar with and has the added effect that 
native_handle isn't possible to implement, meaning that these can't easily 
interoperate with C APIs either.

I don't see a way of fixing this without an ABI break, unfortunately.  Perhaps 
we can provide a better implementation in the __v2 namespace and let users (and 
platforms that don't care about binary compatibility with older libc++) opt in?


https://reviews.llvm.org/D10480



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

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

[David Chisnall via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd157a9bc8ba1: Add Windows Control Flow Guard checks 
(/guard:cf). (authored by ajpaverd, committed by theraven).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65761

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/MSVC.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/cfguardtable.c
  clang/test/Driver/cl-fallback.c
  clang/test/Driver/cl-options.c
  llvm/docs/LangRef.rst
  llvm/docs/ReleaseNotes.rst
  llvm/include/llvm/CodeGen/MachineFunction.h
  llvm/include/llvm/CodeGen/Passes.h
  llvm/include/llvm/CodeGen/TargetCallingConv.h
  llvm/include/llvm/CodeGen/TargetLowering.h
  llvm/include/llvm/IR/CallingConv.h
  llvm/include/llvm/IR/InstrTypes.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/InitializePasses.h
  llvm/include/llvm/MC/MCObjectFileInfo.h
  llvm/include/llvm/Target/TargetCallingConv.td
  llvm/include/llvm/Transforms/CFGuard.h
  llvm/lib/AsmParser/LLLexer.cpp
  llvm/lib/AsmParser/LLParser.cpp
  llvm/lib/AsmParser/LLToken.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/AsmPrinter/WinCFGuard.cpp
  llvm/lib/CodeGen/AsmPrinter/WinCFGuard.h
  llvm/lib/CodeGen/CFGuardLongjmp.cpp
  llvm/lib/CodeGen/CMakeLists.txt
  llvm/lib/CodeGen/CodeGen.cpp
  llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
  llvm/lib/CodeGen/SelectionDAG/FastISel.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/IR/AsmWriter.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/lib/Target/AArch64/AArch64CallingConvention.h
  llvm/lib/Target/AArch64/AArch64CallingConvention.td
  llvm/lib/Target/AArch64/AArch64FastISel.cpp
  llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
  llvm/lib/Target/AArch64/AArch64RegisterInfo.cpp
  llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
  llvm/lib/Target/AArch64/LLVMBuild.txt
  llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp
  llvm/lib/Target/ARM/ARMCallingConv.h
  llvm/lib/Target/ARM/ARMCallingConv.td
  llvm/lib/Target/ARM/ARMFastISel.cpp
  llvm/lib/Target/ARM/ARMISelLowering.cpp
  llvm/lib/Target/ARM/ARMTargetMachine.cpp
  llvm/lib/Target/ARM/LLVMBuild.txt
  llvm/lib/Target/X86/LLVMBuild.txt
  llvm/lib/Target/X86/X86AsmPrinter.cpp
  llvm/lib/Target/X86/X86CallingConv.td
  llvm/lib/Target/X86/X86FastISel.cpp
  llvm/lib/Target/X86/X86RegisterInfo.cpp
  llvm/lib/Target/X86/X86TargetMachine.cpp
  llvm/lib/Transforms/CFGuard/CFGuard.cpp
  llvm/lib/Transforms/CFGuard/CMakeLists.txt
  llvm/lib/Transforms/CFGuard/LLVMBuild.txt
  llvm/lib/Transforms/CMakeLists.txt
  llvm/lib/Transforms/LLVMBuild.txt
  llvm/test/Bitcode/calling-conventions.3.2.ll
  llvm/test/Bitcode/calling-conventions.3.2.ll.bc
  llvm/test/Bitcode/operand-bundles-bc-analyzer.ll
  llvm/test/CodeGen/AArch64/cfguard-checks.ll
  llvm/test/CodeGen/AArch64/cfguard-module-flag.ll
  llvm/test/CodeGen/ARM/cfguard-checks.ll
  llvm/test/CodeGen/ARM/cfguard-module-flag.ll
  llvm/test/CodeGen/WinCFGuard/cfguard.ll
  llvm/test/CodeGen/X86/cfguard-checks.ll
  llvm/test/CodeGen/X86/cfguard-module-flag.ll
  llvm/test/CodeGen/X86/cfguard-x86-64-vectorcall.ll
  llvm/test/CodeGen/X86/cfguard-x86-vectorcall.ll

Index: llvm/test/CodeGen/X86/cfguard-x86-vectorcall.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/cfguard-x86-vectorcall.ll
@@ -0,0 +1,43 @@
+; RUN: llc < %s -mtriple=i686-pc-windows-msvc | FileCheck %s -check-prefix=X32
+; Control Flow Guard is currently only available on Windows
+
+
+; Test that Control Flow Guard checks are correctly added for x86 vector calls.
+define void @func_cf_vector_x86(void (%struct.HVA)* %0, %struct.HVA* %1) #0 {
+entry:
+  %2 = alloca %struct.HVA, align 8
+  %3 = bitcast %struct.HVA* %2 to i8*
+  %4 = bitcast %struct.HVA* %1 to i8*
+  call void @llvm.memcpy.p0i8.p0i8.i32(i8* align 8 %3, i8* align 8 %4, i32 32, i1 false)
+  %5 = load %struct.HVA, %struct.HVA* %2, align 8
+  call x86_vectorcallcc void %0(%struct.HVA inreg %5)
+  ret void
+
+  ; X32-LABEL: func_cf_vector_x86
+  ; X32: 	 movl 12(%ebp), %eax
+  ; X32: 	 movl 8(%ebp), %ecx
+  ; X32: 	 movsd 24(%eax), %xmm4 # xmm4 = mem[0],zero
+  ; X32: 	 movsd %xmm4, 24(%esp)
+  ; X32: 	 movsd 16(%eax), %xmm5 # xmm5 = mem[0],zero
+  ; X32: 	 movsd %xmm5, 16(%esp)
+  ; X32: 	 movsd (%eax), %xmm6   # xmm6 = mem[0],zero
+  ; X32: 	 movsd 8(%eax), %xmm7  # xmm7 = mem[0],zero
+  ; X32: 	 movsd %xmm7, 8(%esp)
+  ; X32: 	 movsd %xmm6, (%esp)
+  ; X32: 

[PATCH] D80225: [Driver] Recognize -fuse-ld={bfd, gold, lld} but don't prepend "ld." or "ld64." for other values

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

I'm a bit nervous about this.  I'm aware of at least one out-of-tree toolchain 
that uses this mechanism to select their proprietary linker.  I'd expect an RFC 
and for this to be highlighted in LLVM Weekly before I'm happy that this won't 
break downstream consumers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80225



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

[PATCH] D69991: Implement __attribute__((objc_direct)), __attribute__((objc_direct_members))

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.
Herald added a reviewer: aaron.ballman.
Herald added a reviewer: jdoerfert.

Sorry for getting to this late, I assumed it was a runtime-agnostic feature 
until someone filed a bug against the GNUstep runtime saying that we didn't 
implement it.  It would be polite to tag me in reviews for features that 
contain runtime-specific things.

The current implementation doesn't look as if it's runtime-specific.  Is there 
a reason that it's in CGObjCMacCommon rather than in the generic CGObjCRuntime 
class?

Looking at the code, it appears that it is correct only for objects created 
with `+alloc`.  Consider the following example:

  #import 
  #include 
  
  @interface X : NSObject
  - (void)test __attribute__((objc_direct));
  @end
  
  @implementation X
  + (void)initialize
  {
printf("+initialize called on %p\n", self);
  }
  - (void)test
  {
printf("Test called on %p\n", self);
  }
  @end
  
  int main()
  {
@autoreleasepool
{
X *x = class_createInstance(objc_getClass("X"), 0);
[x test];
}
  }

I don't believe this does the right thing (unfortunately, the latest XCode 
requires Catalina and it breaks too many things for me to upgrade from Mojave, 
so I can't test it and see if there are any tweaks to the runtime).  The API 
guarantee for `class_createInstance` does not require it to call `+initialize`. 
 I can see a few ways of fixing this:

Either:

- Unconditionally do the `[self self]` dance on all direct methods (not ideal, 
since it's likely to offset the performance win) for runtimes without knowledge 
of direct functions and one out of:
  - Set a metadata flag on any class with direct methods to require 
`class_createInstance` (and any other allocation paths that I haven't thought 
of) in the runtime to call `+initialize` on instantiation.
  - Modify the API contract of `class_createInstance` to send `+initialize` 
unconditionally.
  - Provide an `objc_msgSendDirect` that takes the target function in place of 
the selector and checks that the receiver is initialise, teach the optimisers 
to inline through this in cases where they can prove that the receiver is 
already initialised.
- Document that direct methods do not trigger `+initialize`, generalise that to 
class methods and remove the existing `+self` dance.

For direct class methods, avoiding the `+self` dance might best avoided by 
using a construct like the C++ static initialiser for a pointer to the class at 
the call site (rather than a direct reference to the class), which guarantees 
that it's initialised in the callee and skipping this when the receiver is 
`self` in an method (`+initialize` is guaranteed to have been called on `self` 
on entry to any method in this case).  The pseudocode is something like (where 
`+direct` is a direct method):

The commit message for this refers to `objc_opt_self`.  This is presumably a 
fast-path for a `self` message send, but the only reference to that anywhere in 
the LLVM tree is in LLDB.  We don't appear to have any tests in clang for it 
and neither upstream nor Apple clang appear to generate calls for it, so I'm 
not sure what 'an appropriate deployment target' is in the commit message.


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

https://reviews.llvm.org/D69991



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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

This feature looks generally useful.  A few small suggestions:

- This is really a way of transforming a formal protocol into an informal 
protocol.  Objective-C has had a convention of informal protocols since the 
'80s, but they're implemented as categories on the root class with no 
`@implementation`.  I'd suggest that `__attribute__((objc_informal_protocol))` 
or similar might be a better spelling for this, explicitly bringing the 
informal notion into the language.  A lot of the informal protocols in Cocoa 
could be better expressed using this and `@optional` methods than as categories 
on `NSObject`.
- Given that this doesn't depend on any features in the runtime (from the 
runtime's perspective, the protocol doesn't exist), I don't think it makes 
sense to have an `ObjCRuntime` method to query whether this is supported by the 
runtime.  We should enable it everywhere if it's going in anywhere.
- The changes required in CGObjcCGNU.cpp are fairly small and I agree that 
@rjmccall's proposal  for a callback-driven visitor would simplify the changes 
in both runtimes.
- The semantics are slightly confusing with the deep approach though.  
Normally, if you iterate over the protocols that a class conforms to, you only 
see the ones that it directly conforms to.  With this model, you'd see indirect 
ones.  We might want to set some metadata to allow programmers to differentiate 
the two, or we might want to have a warning (off by default?) if an informal 
protocol conforms to a formal one, or simply disallow it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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

[PATCH] D123898: Fix crash in ObjC codegen introduced with 5ab6ee75994d645725264e757d67bbb1c96fb2b6

[David Chisnall via Phabricator via cfe-commits]

theraven created this revision.
theraven added reviewers: rjmccall, triplef.
Herald added a project: All.
theraven requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

5ab6ee75994d645725264e757d67bbb1c96fb2b6 
 assumed 
that if `RValue::isScalar()` returns true then `RValue::getScalarVal` will 
return a valid value.  This is not the case when the return value is `void` and 
so void message returns would crash if they hit this path.  This is triggered 
only for cases where the nil-handling path needs to do something non-trivial 
(destroy arguments that should be consumed by the callee).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123898

Files:
  clang/lib/CodeGen/CGObjCGNU.cpp


Index: clang/lib/CodeGen/CGObjCGNU.cpp
===
--- clang/lib/CodeGen/CGObjCGNU.cpp
+++ clang/lib/CodeGen/CGObjCGNU.cpp
@@ -2837,11 +2837,13 @@
 // Enter the continuation block and emit a phi if required.
 CGF.EmitBlock(continueBB);
 if (msgRet.isScalar()) {
-  llvm::Value *v = msgRet.getScalarVal();
-  llvm::PHINode *phi = Builder.CreatePHI(v->getType(), 2);
-  phi->addIncoming(v, nonNilPathBB);
-  phi->addIncoming(CGM.EmitNullConstant(ResultType), nilPathBB);
-  msgRet = RValue::get(phi);
+  // If the return type is void, do nothing
+  if (llvm::Value *v = msgRet.getScalarVal()) {
+llvm::PHINode *phi = Builder.CreatePHI(v->getType(), 2);
+phi->addIncoming(v, nonNilPathBB);
+phi->addIncoming(CGM.EmitNullConstant(ResultType), nilPathBB);
+msgRet = RValue::get(phi);
+  }
 } else if (msgRet.isAggregate()) {
   // Aggregate zeroing is handled in nilCleanupBB when it's required.
 } else /* isComplex() */ {


Index: clang/lib/CodeGen/CGObjCGNU.cpp
===
--- clang/lib/CodeGen/CGObjCGNU.cpp
+++ clang/lib/CodeGen/CGObjCGNU.cpp
@@ -2837,11 +2837,13 @@
 // Enter the continuation block and emit a phi if required.
 CGF.EmitBlock(continueBB);
 if (msgRet.isScalar()) {
-  llvm::Value *v = msgRet.getScalarVal();
-  llvm::PHINode *phi = Builder.CreatePHI(v->getType(), 2);
-  phi->addIncoming(v, nonNilPathBB);
-  phi->addIncoming(CGM.EmitNullConstant(ResultType), nilPathBB);
-  msgRet = RValue::get(phi);
+  // If the return type is void, do nothing
+  if (llvm::Value *v = msgRet.getScalarVal()) {
+llvm::PHINode *phi = Builder.CreatePHI(v->getType(), 2);
+phi->addIncoming(v, nonNilPathBB);
+phi->addIncoming(CGM.EmitNullConstant(ResultType), nilPathBB);
+msgRet = RValue::get(phi);
+  }
 } else if (msgRet.isAggregate()) {
   // Aggregate zeroing is handled in nilCleanupBB when it's required.
 } else /* isComplex() */ {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123898: Fix crash in ObjC codegen introduced with 5ab6ee75994d645725264e757d67bbb1c96fb2b6

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

I'd like to.  The test case from the original bug report was very large but I'm 
not sure how to trigger this with anything comprehensibly small.  You need 
something where the callee is responsible for destroying the argument and I'm 
not sure what combination of properties / ABIs results in that.  The original 
test was `-(void)foo:(std::string)aString` on the MSVC ABI, but I don't know 
how much of the Visual Studio `std::string` implementation is necessary to 
trigger this behaviour.  Is it just a non-trivial destructor?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123898

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

[PATCH] D123898: Fix crash in ObjC codegen introduced with 5ab6ee75994d645725264e757d67bbb1c96fb2b6

[David Chisnall via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG94c3b169785c: Fix crash in ObjC codegen introduced with… 
(authored by theraven).

Changed prior to commit:
  https://reviews.llvm.org/D123898?vs=423241&id=447127#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123898

Files:
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/test/CodeGenObjC/gnustep2-nontrivial-destructor-argument.mm


Index: clang/test/CodeGenObjC/gnustep2-nontrivial-destructor-argument.mm
===
--- /dev/null
+++ clang/test/CodeGenObjC/gnustep2-nontrivial-destructor-argument.mm
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -triple x86_64-unknow-windows-msvc -S -emit-llvm 
-fobjc-runtime=gnustep-2.0 -o - %s 
+
+// Regression test.  Ensure that C++ arguments with non-trivial destructors
+// don't crash the compiler.
+
+struct X
+{
+  int a;
+  ~X();
+};
+
+@protocol Y
+- (void)foo: (X)bar;
+@end
+
+
+void test(id obj)
+{
+  X a{12};
+  [obj foo: a];
+}
+
Index: clang/lib/CodeGen/CGObjCGNU.cpp
===
--- clang/lib/CodeGen/CGObjCGNU.cpp
+++ clang/lib/CodeGen/CGObjCGNU.cpp
@@ -2837,11 +2837,13 @@
 // Enter the continuation block and emit a phi if required.
 CGF.EmitBlock(continueBB);
 if (msgRet.isScalar()) {
-  llvm::Value *v = msgRet.getScalarVal();
-  llvm::PHINode *phi = Builder.CreatePHI(v->getType(), 2);
-  phi->addIncoming(v, nonNilPathBB);
-  phi->addIncoming(CGM.EmitNullConstant(ResultType), nilPathBB);
-  msgRet = RValue::get(phi);
+  // If the return type is void, do nothing
+  if (llvm::Value *v = msgRet.getScalarVal()) {
+llvm::PHINode *phi = Builder.CreatePHI(v->getType(), 2);
+phi->addIncoming(v, nonNilPathBB);
+phi->addIncoming(CGM.EmitNullConstant(ResultType), nilPathBB);
+msgRet = RValue::get(phi);
+  }
 } else if (msgRet.isAggregate()) {
   // Aggregate zeroing is handled in nilCleanupBB when it's required.
 } else /* isComplex() */ {


Index: clang/test/CodeGenObjC/gnustep2-nontrivial-destructor-argument.mm
===
--- /dev/null
+++ clang/test/CodeGenObjC/gnustep2-nontrivial-destructor-argument.mm
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -triple x86_64-unknow-windows-msvc -S -emit-llvm -fobjc-runtime=gnustep-2.0 -o - %s 
+
+// Regression test.  Ensure that C++ arguments with non-trivial destructors
+// don't crash the compiler.
+
+struct X
+{
+  int a;
+  ~X();
+};
+
+@protocol Y
+- (void)foo: (X)bar;
+@end
+
+
+void test(id obj)
+{
+  X a{12};
+  [obj foo: a];
+}
+
Index: clang/lib/CodeGen/CGObjCGNU.cpp
===
--- clang/lib/CodeGen/CGObjCGNU.cpp
+++ clang/lib/CodeGen/CGObjCGNU.cpp
@@ -2837,11 +2837,13 @@
 // Enter the continuation block and emit a phi if required.
 CGF.EmitBlock(continueBB);
 if (msgRet.isScalar()) {
-  llvm::Value *v = msgRet.getScalarVal();
-  llvm::PHINode *phi = Builder.CreatePHI(v->getType(), 2);
-  phi->addIncoming(v, nonNilPathBB);
-  phi->addIncoming(CGM.EmitNullConstant(ResultType), nilPathBB);
-  msgRet = RValue::get(phi);
+  // If the return type is void, do nothing
+  if (llvm::Value *v = msgRet.getScalarVal()) {
+llvm::PHINode *phi = Builder.CreatePHI(v->getType(), 2);
+phi->addIncoming(v, nonNilPathBB);
+phi->addIncoming(CGM.EmitNullConstant(ResultType), nilPathBB);
+msgRet = RValue::get(phi);
+  }
 } else if (msgRet.isAggregate()) {
   // Aggregate zeroing is handled in nilCleanupBB when it's required.
 } else /* isComplex() */ {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D128190: [WinEH] Apply funclet operand bundles to nounwind intrinsics that lower to function calls

[David Chisnall via Phabricator via cfe-commits]

theraven accepted this revision.
theraven added a comment.
This revision is now accepted and ready to land.

LGTM, a couple of extra comments would help.




Comment at: llvm/include/llvm/IR/IntrinsicInst.h:110
 
+  // Check if this intrinsic might lower into a regular function call
+  static bool mayLowerToFunctionCall(Intrinsic::ID IID);

Minor nit, should be /// for a doc comment.



Comment at: llvm/lib/IR/IntrinsicInst.cpp:61
+  case Intrinsic::objc_sync_enter:
+  case Intrinsic::objc_sync_exit:
+return true;

I'm curious why memcpy and friends don't need to be on this list.  Do they get 
expanded at a different point?  Or is it that the front end inserts a memcpy 
call and the optimiser that turns them into intrinsics and back preserves 
operand bundles?  It would be a good idea to have a comment explaining why 
these specific ones are on the list and not other libc (or math library) 
functions that may appear in cleanup blocks.  From the code in 
`InlineFunction.cpp`, it looks as if this is a problem only for intrinsics that 
may throw, which excludes most of the C standard ones?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128190

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

[PATCH] D72167: Add support for __declspec(guard(nocf))

[David Chisnall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbdd88b7ed395: Add support for __declspec(guard(nocf)) 
(authored by ajpaverd, committed by theraven).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72167

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/guard_nocf.c
  clang/test/CodeGenCXX/guard_nocf.cpp
  clang/test/Sema/attr-guard_nocf.c
  llvm/lib/Transforms/CFGuard/CFGuard.cpp
  llvm/test/CodeGen/AArch64/cfguard-checks.ll
  llvm/test/CodeGen/ARM/cfguard-checks.ll
  llvm/test/CodeGen/X86/cfguard-checks.ll

Index: llvm/test/CodeGen/X86/cfguard-checks.ll
===
--- llvm/test/CodeGen/X86/cfguard-checks.ll
+++ llvm/test/CodeGen/X86/cfguard-checks.ll
@@ -8,26 +8,26 @@
 declare i32 @target_func()
 
 
-; Test that Control Flow Guard checks are not added to functions with nocf_checks attribute.
-define i32 @func_nocf_checks() #0 {
+; Test that Control Flow Guard checks are not added on calls with the "guard_nocf" attribute.
+define i32 @func_guard_nocf() {
 entry:
   %func_ptr = alloca i32 ()*, align 8
   store i32 ()* @target_func, i32 ()** %func_ptr, align 8
   %0 = load i32 ()*, i32 ()** %func_ptr, align 8
-  %1 = call i32 %0()
+  %1 = call i32 %0() #0
   ret i32 %1
 
-  ; X32-LABEL: func_nocf_checks
+  ; X32-LABEL: func_guard_nocf
   ; X32: 	 movl  $_target_func, %eax
   ; X32-NOT: __guard_check_icall_fptr
 	; X32: 	 calll *%eax
 
-  ; X64-LABEL: func_nocf_checks
+  ; X64-LABEL: func_guard_nocf
   ; X64:   leaq	target_func(%rip), %rax
   ; X64-NOT: __guard_dispatch_icall_fptr
   ; X64:   callq	*%rax
 }
-attributes #0 = { nocf_check }
+attributes #0 = { "guard_nocf" }
 
 
 ; Test that Control Flow Guard checks are added even at -O0.
Index: llvm/test/CodeGen/ARM/cfguard-checks.ll
===
--- llvm/test/CodeGen/ARM/cfguard-checks.ll
+++ llvm/test/CodeGen/ARM/cfguard-checks.ll
@@ -7,26 +7,27 @@
 declare i32 @target_func()
 
 
-; Test that Control Flow Guard checks are not added to functions with nocf_checks attribute.
-define i32 @func_nocf_checks() #0 {
+; Test that Control Flow Guard checks are not added on calls with the "guard_nocf" attribute.
+define i32 @func_guard_nocf() #0 {
 entry:
   %func_ptr = alloca i32 ()*, align 8
   store i32 ()* @target_func, i32 ()** %func_ptr, align 8
   %0 = load i32 ()*, i32 ()** %func_ptr, align 8
-  %1 = call arm_aapcs_vfpcc i32 %0()
+  %1 = call arm_aapcs_vfpcc i32 %0() #1
   ret i32 %1
 
-  ; CHECK-LABEL: func_nocf_checks
+  ; CHECK-LABEL: func_guard_nocf
   ; CHECK:   movw r0, :lower16:target_func
 	; CHECK:   movt r0, :upper16:target_func
   ; CHECK-NOT:   __guard_check_icall_fptr
 	; CHECK:   blx r0
 }
-attributes #0 = { nocf_check "target-cpu"="cortex-a9" "target-features"="+armv7-a,+dsp,+fp16,+neon,+strict-align,+thumb-mode,+vfp3"}
+attributes #0 = { "target-cpu"="cortex-a9" "target-features"="+armv7-a,+dsp,+fp16,+neon,+strict-align,+thumb-mode,+vfp3"}
+attributes #1 = { "guard_nocf" }
 
 
 ; Test that Control Flow Guard checks are added even at -O0.
-define i32 @func_optnone_cf() #1 {
+define i32 @func_optnone_cf() #2 {
 entry:
   %func_ptr = alloca i32 ()*, align 8
   store i32 ()* @target_func, i32 ()** %func_ptr, align 8
@@ -47,11 +48,11 @@
 	; CHECK:   blx r1
 	; CHECK-NEXT:  blx r4
 }
-attributes #1 = { noinline optnone "target-cpu"="cortex-a9" "target-features"="+armv7-a,+dsp,+fp16,+neon,+strict-align,+thumb-mode,+vfp3"}
+attributes #2 = { noinline optnone "target-cpu"="cortex-a9" "target-features"="+armv7-a,+dsp,+fp16,+neon,+strict-align,+thumb-mode,+vfp3"}
 
 
 ; Test that Control Flow Guard checks are correctly added in optimized code (common case).
-define i32 @func_cf() #2 {
+define i32 @func_cf() #0 {
 entry:
   %func_ptr = alloca i32 ()*, align 8
   store i32 ()* @target_func, i32 ()** %func_ptr, align 8
@@ -70,11 +71,10 @@
 	; CHECK:   blx r1
 	; CHECK-NEXT:  blx r4
 }
-attributes #2 = { "target-cpu"="cortex-a9" "target-features"="+armv7-a,+dsp,+fp16,+neon,+strict-align,+thumb-mode,+vfp3"}
 
 
 ; Test that Control Flow Guard checks are correctly added on invoke instructions.
-define i32 @func_cf_invoke() #2 personality i8* bitcast (void ()* @h to i8*) {
+define i32 @func_cf_invoke() #0 personality i8* bitcast (void ()* @h to i8*) {
 entry:
   %0 = alloca i32, align 4
   %func_ptr = alloca i32 ()*, align 8
@@ -112,7 +112,7 @@
 %struct._SETJMP_FLOAT128 = type { [2 x i64] }
 @buf1 = internal global [16 x %struct._SETJMP_FLOAT128] zeroinitializer, align 16
 
-define i32 @func_cf_setjmp() #2 {
+define i32 @func_cf_setjmp() #0 {
   %1 = alloca i32, align 4
   %2 = alloca i32, align 4
   store i32 0, i32* %1, align 4
Index: llvm/test/CodeGen/AArch64/cfguard-ch

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[David Chisnall via Phabricator via cfe-commits]

theraven accepted this revision.
theraven added a comment.
This revision is now accepted and ready to land.

Looks good to me, thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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

[PATCH] D91874: [GNU ObjC] Fix a regression listing methods twice.

[David Chisnall via Phabricator via cfe-commits]

theraven created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
theraven requested review of this revision.

Methods synthesized from declared properties were being added to the
method lists twice.  This came from the change to list them in the
class's method list, which missed removing the place in CGObjCGNU that
added them again.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91874

Files:
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/test/CodeGenObjC/gnu-method-only-once.m


Index: clang/test/CodeGenObjC/gnu-method-only-once.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/gnu-method-only-once.m
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -S -emit-llvm 
-fobjc-runtime=gnustep-2.0 -o - %s | FileCheck %s -check-prefix=CHECK-NEW
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -S -emit-llvm 
-fobjc-runtime=gnustep-1.8 -o - %s | FileCheck %s -check-prefix=CHECK-OLD
+
+// Clang 9 or 10 changed the handling of method lists so that methods provided
+// from synthesised properties showed up in the method list, where previously
+// CGObjCGNU had to collect them and merge them.  One of the places where this
+// merging happened was missed in the move and so we ended up emitting two
+// copies of method metadata for everything that appeared in the 
+
+// This class has only instance properties and only one pair of synthesized
+// methods from the property and so we should synthesize only one method list,
+// with precisely two methods on it.
+@interface X
+@property (retain) id iProp;
+@end
+
+@implementation X
+@synthesize iProp;
+@end
+
+// Check that the method list has precisely 2 methods.
+// CHECK-NEW: @.objc_method_list = internal global { i8*, i32, i64, [2 x
+// CHECK-OLD: @.objc_method_list = internal global { i8*, i32, [2 x
Index: clang/lib/CodeGen/CGObjCGNU.cpp
===
--- clang/lib/CodeGen/CGObjCGNU.cpp
+++ clang/lib/CodeGen/CGObjCGNU.cpp
@@ -3512,19 +3512,6 @@
   ClassMethods.insert(ClassMethods.begin(), OID->classmeth_begin(),
   OID->classmeth_end());
 
-  // Collect the same information about synthesized properties, which don't
-  // show up in the instance method lists.
-  for (auto *propertyImpl : OID->property_impls())
-if (propertyImpl->getPropertyImplementation() ==
-ObjCPropertyImplDecl::Synthesize) {
-  auto addPropertyMethod = [&](const ObjCMethodDecl *accessor) {
-if (accessor)
-  InstanceMethods.push_back(accessor);
-  };
-  addPropertyMethod(propertyImpl->getGetterMethodDecl());
-  addPropertyMethod(propertyImpl->getSetterMethodDecl());
-}
-
   llvm::Constant *Properties = GeneratePropertyList(OID, ClassDecl);
 
   // Collect the names of referenced protocols


Index: clang/test/CodeGenObjC/gnu-method-only-once.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/gnu-method-only-once.m
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -S -emit-llvm -fobjc-runtime=gnustep-2.0 -o - %s | FileCheck %s -check-prefix=CHECK-NEW
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -S -emit-llvm -fobjc-runtime=gnustep-1.8 -o - %s | FileCheck %s -check-prefix=CHECK-OLD
+
+// Clang 9 or 10 changed the handling of method lists so that methods provided
+// from synthesised properties showed up in the method list, where previously
+// CGObjCGNU had to collect them and merge them.  One of the places where this
+// merging happened was missed in the move and so we ended up emitting two
+// copies of method metadata for everything that appeared in the 
+
+// This class has only instance properties and only one pair of synthesized
+// methods from the property and so we should synthesize only one method list,
+// with precisely two methods on it.
+@interface X
+@property (retain) id iProp;
+@end
+
+@implementation X
+@synthesize iProp;
+@end
+
+// Check that the method list has precisely 2 methods.
+// CHECK-NEW: @.objc_method_list = internal global { i8*, i32, i64, [2 x
+// CHECK-OLD: @.objc_method_list = internal global { i8*, i32, [2 x
Index: clang/lib/CodeGen/CGObjCGNU.cpp
===
--- clang/lib/CodeGen/CGObjCGNU.cpp
+++ clang/lib/CodeGen/CGObjCGNU.cpp
@@ -3512,19 +3512,6 @@
   ClassMethods.insert(ClassMethods.begin(), OID->classmeth_begin(),
   OID->classmeth_end());
 
-  // Collect the same information about synthesized properties, which don't
-  // show up in the instance method lists.
-  for (auto *propertyImpl : OID->property_impls())
-if (propertyImpl->getPropertyImplementation() ==
-ObjCPropertyImplDecl::Synthesize) {
-  auto addPropertyMethod = [&](const ObjCMethodDecl *accessor) {
-if (accessor)
-  InstanceMethods.push_back(accessor);
-  };
-  addPropertyMethod(propert

[PATCH] D91874: [GNU ObjC] Fix a regression listing methods twice.

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

This was caught with the GNUstep runtime's test suite, which apparently had not 
been run with anything newer than clang 8 until recently.  With this patch, all 
of the runtime's tests now pass again (a few others failed in 10 but appear to 
have been fixed in 11 or 12).  We've now configured our CI to test with the 
nightly builds on Linux, so should catch these things sooner in the future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91874

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

[PATCH] D91874: [GNU ObjC] Fix a regression listing methods twice.

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

I'd like to get this into the 11 point release, so it would be good to have a 
review...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91874

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

[PATCH] D91874: [GNU ObjC] Fix a regression listing methods twice.

[David Chisnall via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd1ed67037de6: [GNU ObjC] Fix a regression listing methods 
twice. (authored by theraven).

Changed prior to commit:
  https://reviews.llvm.org/D91874?vs=306704&id=308583#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91874

Files:
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/test/CodeGenObjC/gnu-method-only-once.m


Index: clang/test/CodeGenObjC/gnu-method-only-once.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/gnu-method-only-once.m
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -S -emit-llvm 
-fobjc-runtime=gnustep-2.0 -o - %s | FileCheck %s -check-prefix=CHECK-NEW
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -S -emit-llvm 
-fobjc-runtime=gnustep-1.8 -o - %s | FileCheck %s -check-prefix=CHECK-OLD
+
+// Clang 9 or 10 changed the handling of method lists so that methods provided
+// from synthesised properties showed up in the method list, where previously
+// CGObjCGNU had to collect them and merge them.  One of the places where this
+// merging happened was missed in the move and so we ended up emitting two
+// copies of method metadata for declared properties.
+
+// This class has only instance properties and only one pair of synthesized
+// methods from the property and so we should synthesize only one method list,
+// with precisely two methods on it.
+@interface X
+@property (retain) id iProp;
+@end
+
+@implementation X
+@synthesize iProp;
+@end
+
+// Check that the method list has precisely 2 methods.
+// CHECK-NEW: @.objc_method_list = internal global { i8*, i32, i64, [2 x
+// CHECK-OLD: @.objc_method_list = internal global { i8*, i32, [2 x
Index: clang/lib/CodeGen/CGObjCGNU.cpp
===
--- clang/lib/CodeGen/CGObjCGNU.cpp
+++ clang/lib/CodeGen/CGObjCGNU.cpp
@@ -3512,19 +3512,6 @@
   ClassMethods.insert(ClassMethods.begin(), OID->classmeth_begin(),
   OID->classmeth_end());
 
-  // Collect the same information about synthesized properties, which don't
-  // show up in the instance method lists.
-  for (auto *propertyImpl : OID->property_impls())
-if (propertyImpl->getPropertyImplementation() ==
-ObjCPropertyImplDecl::Synthesize) {
-  auto addPropertyMethod = [&](const ObjCMethodDecl *accessor) {
-if (accessor)
-  InstanceMethods.push_back(accessor);
-  };
-  addPropertyMethod(propertyImpl->getGetterMethodDecl());
-  addPropertyMethod(propertyImpl->getSetterMethodDecl());
-}
-
   llvm::Constant *Properties = GeneratePropertyList(OID, ClassDecl);
 
   // Collect the names of referenced protocols


Index: clang/test/CodeGenObjC/gnu-method-only-once.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/gnu-method-only-once.m
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -S -emit-llvm -fobjc-runtime=gnustep-2.0 -o - %s | FileCheck %s -check-prefix=CHECK-NEW
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd -S -emit-llvm -fobjc-runtime=gnustep-1.8 -o - %s | FileCheck %s -check-prefix=CHECK-OLD
+
+// Clang 9 or 10 changed the handling of method lists so that methods provided
+// from synthesised properties showed up in the method list, where previously
+// CGObjCGNU had to collect them and merge them.  One of the places where this
+// merging happened was missed in the move and so we ended up emitting two
+// copies of method metadata for declared properties.
+
+// This class has only instance properties and only one pair of synthesized
+// methods from the property and so we should synthesize only one method list,
+// with precisely two methods on it.
+@interface X
+@property (retain) id iProp;
+@end
+
+@implementation X
+@synthesize iProp;
+@end
+
+// Check that the method list has precisely 2 methods.
+// CHECK-NEW: @.objc_method_list = internal global { i8*, i32, i64, [2 x
+// CHECK-OLD: @.objc_method_list = internal global { i8*, i32, [2 x
Index: clang/lib/CodeGen/CGObjCGNU.cpp
===
--- clang/lib/CodeGen/CGObjCGNU.cpp
+++ clang/lib/CodeGen/CGObjCGNU.cpp
@@ -3512,19 +3512,6 @@
   ClassMethods.insert(ClassMethods.begin(), OID->classmeth_begin(),
   OID->classmeth_end());
 
-  // Collect the same information about synthesized properties, which don't
-  // show up in the instance method lists.
-  for (auto *propertyImpl : OID->property_impls())
-if (propertyImpl->getPropertyImplementation() ==
-ObjCPropertyImplDecl::Synthesize) {
-  auto addPropertyMethod = [&](const ObjCMethodDecl *accessor) {
-if (accessor)
-  InstanceMethods.push_back(accessor);
-  };
-  addPropertyMethod(propertyImpl->ge

[PATCH] D96816: [ObjC] Encode pointers to C++ classes as "^v" if the encoded string would otherwise include template specialization types

[David Chisnall via Phabricator via cfe-commits]

theraven requested changes to this revision.
theraven added a comment.
This revision now requires changes to proceed.

I'm broadly in favour of this change, but with the GNUstep ABIs this is an 
ABI-breaking change and so should not be on by default.  The type encoding 
leaks into the ABI in two ways:

- Dispatch is dependent on type, which avoids some stack corruption bugs on 
NeXT-style runtimes.  This means that the types of the selector in the caller 
and the selector in the method definition must match.  I don't see a problem 
with turning this change off for this specific case, because it's incredibly 
rare to pass complex C++ template types as arguments to Objective-C methods.
- The variable that contains the offset of ivars includes the type encoding, so 
that changing the type of a public ivar breaks linkage, rather than just subtly 
corrupting data later.  We could leave this change on for any `@private` or 
`@package` ivars, but changing the type encoding for public ivars will break 
code linking with existing frameworks.

I'm quite happy with this to be opt-in for non-Apple runtimes and opt-out for 
Apple runtimes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96816

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

[PATCH] D96816: [ObjC] Encode pointers to C++ classes as "^v" if the encoded string would otherwise include template specialization types

[David Chisnall via Phabricator via cfe-commits]

theraven added inline comments.



Comment at: clang/include/clang/Driver/Options.td:2113
+def fno_objc_encode_cxx_class_template_spec :
+  Flag<["-"], "fno-objc-encode-cxx-class-template-spec">, Group;
 defm objc_convert_messages_to_runtime_calls : 
BoolFOption<"objc-convert-messages-to-runtime-calls",

I think there's some magic in Options.td for defining the thing and its no- 
variant together.  I don't think you want the f prefix in the name either if 
it's an f-option.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5784
+   options::OPT_fno_objc_encode_cxx_class_template_spec,
+   !Triple.isOSDarwin()))
+CmdArgs.push_back("-fobjc-encode-cxx-class-template-spec");

I don't think this should be guarded on Darwin, it should be guarded on whether 
the [ObjCRuntime is a NeXT-family 
runtime](https://github.com/llvm/llvm-project/blob/62ec4ac90738a5f2d209ed28c83e58aaaeb7/clang/include/clang/Basic/ObjCRuntime.h#L134).
  It might be fine to enable this for all non-GNUstep runtimes (the GCC ABI has 
typed selectors, but doesn't use the type information for dispatch and doesn't 
have non-fragile ivars.  I believe the ObjFW runtime is the same).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96816

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

[PATCH] D96816: [ObjC] Encode pointers to C++ classes as "^v" if the encoded string would otherwise include template specialization types

[David Chisnall via Phabricator via cfe-commits]

theraven accepted this revision.
theraven added a comment.
This revision is now accepted and ready to land.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96816

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

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

The error message here is very confusing:

  /home/theraven/snmalloc2/src/mem/../ds/../aal/../ds/defines.h:122:27: error: 
cannot perform a tail call to function 'error' because its signature is 
incompatible with the calling function
[[clang::musttail]] return snmalloc::error(str);
^
  /home/theraven/snmalloc2/src/mem/../ds/../aal/../ds/defines.h:63:16: note: 
target function has different number of parameters (expected 2 but has 1)
[[noreturn]] SNMALLOC_COLD void error(const char* const str);
 ^
  /home/theraven/snmalloc2/src/mem/../ds/../aal/../ds/defines.h:21:25: note: 
expanded from macro 'SNMALLOC_COLD'
  #  define SNMALLOC_COLD __attribute__((cold))
  ^
  /home/theraven/snmalloc2/src/mem/../ds/../aal/../ds/defines.h:122:9: note: 
tail call required by 'musttail' attribute here
[[clang::musttail]] return snmalloc::error(str);
  ^

The caller and callee both have one argument, the error is because the 
enclosing function has two parameters.  The error appears wrong anyway for two 
reasons in this particular context:

- The callee is `[[noreturn]]`, so the stack layout doesn't make any 
difference, anything can be tail called if it's no-return.
- The enclosing function is `always_inline`, so checking its argument-frame 
layout does not give useful information because it's the caller's 
argument-frame layout that matters.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

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

[PATCH] D105142: RFC: Implementing new mechanism for hard register operands to inline asm as a constraint.

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

The code looks fine but it would be good to see some docs along with it.  We're 
currently missing docs on inline assembly entirely and the GCC ones are 
somewhat... opaque when it comes to describing how constraints work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105142

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

[PATCH] D99517: Implemented [[clang::musttail]] attribute for guaranteed tail calls.

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

Here's a minimal test:

  void tail(int, float);
  
  __attribute__((always_inline))
  void caller(float x)
  {
[[clang::musttail]]
return tail(42, x);
  }
  
  void outer(int x, float y)
  {
  return caller(y);
  }

This raises this error:

  tail.cc:7:3: error: cannot perform a tail call to function 'tail' because its 
signature is incompatible with the calling function
return tail(42, x);
^
  tail.cc:1:1: note: target function has different number of parameters 
(expected 1 but has 2)
  void tail(int, float);
  ^
  tail.cc:6:5: note: tail call required by 'musttail' attribute here
[[clang::musttail]]
  ^

There's also an interesting counterexample:

  void tail(int, float);
  
  __attribute__((always_inline))
  void caller(int a, float x)
  {
[[clang::musttail]]
return tail(a, x);
  }
  
  void outer(float y)
  {
  return caller(42, y);
  }

This *is* accepted by clang, but then generates this IR at -O0:

  define dso_local void @_Z5outerf(float %0) #2 {
%2 = alloca i32, align 4
%3 = alloca float, align 4
%4 = alloca float, align 4
store float %0, float* %4, align 4
%5 = load float, float* %4, align 4
store i32 42, i32* %2, align 4
store float %5, float* %3, align 4
%6 = load i32, i32* %2, align 4
%7 = load float, float* %3, align 4
call void @_Z4tailif(i32 %6, float %7)
ret void
  }

And this IR at -O1:

  ; Function Attrs: uwtable mustprogress
  define dso_local void @_Z5outerf(float %0) local_unnamed_addr #2 {
call void @_Z4tailif(i32 42, float %0)
ret void
  }

Note that in both cases, the alway-inline attribute is respected (even at -O0, 
the always-inline inliner runs) but the musttail annotation is lost.  The 
inlining has inserted the call into a function with a different set of 
parameters and so it cannot have a `musttail` IR annotation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99517

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

[PATCH] D105142: RFC: Implementing new mechanism for hard register operands to inline asm as a constraint.

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

In D105142#2850247 , @anirudhp wrote:

> In D105142#2849835 , @theraven 
> wrote:
>
>> The code looks fine but it would be good to see some docs along with it.  
>> We're currently missing docs on inline assembly entirely and the GCC ones 
>> are somewhat... opaque when it comes to describing how constraints work.
>
> Thank you for your feedback! By docs do you mind updating/adding some 
> information to the existing LLVM docs(like the langref 
> https://llvm.org/docs/LangRef.html for example), or more comments to the code?

I meant user-facing clang docs.  This is not an IR change, so it does not 
belong in LangRef, but the only reference to inline assembly in clang's 
documentation  is a 
reference to the GCC docs (which are almost incomprehensible in general because 
they were very x86-specific and were then tweaked a bit to be portable, and 
specifically don't mention this feature).  If we are adding a new user-facing 
feature, we need to provide user-facing documentation for it.  Ideally this 
would provide complete documentation of inline assembly supported by clang, but 
at least we should document this feature as an extension.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105142

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

[PATCH] D112400: [clang][compiler-rt][atomics] Add `__c11_atomic_fetch_nand` builtin and support `__atomic_fetch_nand` libcall

[David Chisnall via Phabricator via cfe-commits]

theraven accepted this revision.
theraven added a comment.
This revision is now accepted and ready to land.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112400

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

[PATCH] D157962: [GNU ObjC] Unconditionally emit section markers.

[David Chisnall via Phabricator via cfe-commits]

theraven created this revision.
Herald added a project: All.
theraven requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

In the GNUstep v2 ABI (on ELF), we rely on the linker-inserted section
start and stop markers.  These only exist when binary has something in
the relevant sections.  To ensure that they exist, compilation units
that omit one of the components emit an empty object into the section
that it's not putting anything in.  These empty objects are COMDATs and
are merged in the final version.

Unfortunately, there is a corner case where the entry exists in the code
emitted by the front end but is then elided during optimisation.  We
already unconditionally emitted an empty selector to avoid this case for
message sends but it was still a problem for constant strings.

This change unconditionally emits these placeholders for everything.
This results in very slightly larger .o files but ensures that any
future optimisation that elides more things will not prevent compilation
and so reduces fragility.

Fixes #47536


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157962

Files:
  clang/lib/CodeGen/CGObjCGNU.cpp


Index: clang/lib/CodeGen/CGObjCGNU.cpp
===
--- clang/lib/CodeGen/CGObjCGNU.cpp
+++ clang/lib/CodeGen/CGObjCGNU.cpp
@@ -1614,32 +1614,24 @@
 if (!CGM.getTriple().isOSBinFormatCOFF()) {
   createNullGlobal(".objc_null_selector", {NULLPtr, NULLPtr},
   sectionName());
-  if (Categories.empty())
-createNullGlobal(".objc_null_category", {NULLPtr, NULLPtr,
-  NULLPtr, NULLPtr, NULLPtr, NULLPtr, NULLPtr},
-sectionName());
-  if (!EmittedClass) {
-createNullGlobal(".objc_null_cls_init_ref", NULLPtr,
-sectionName());
-createNullGlobal(".objc_null_class_ref", { NULLPtr, NULLPtr },
-sectionName());
-  }
-  if (!EmittedProtocol)
-createNullGlobal(".objc_null_protocol", {NULLPtr, NULLPtr, NULLPtr,
-NULLPtr, NULLPtr, NULLPtr, NULLPtr, NULLPtr, NULLPtr, NULLPtr,
-NULLPtr}, sectionName());
-  if (!EmittedProtocolRef)
-createNullGlobal(".objc_null_protocol_ref", {NULLPtr},
-sectionName());
-  if (ClassAliases.empty())
-createNullGlobal(".objc_null_class_alias", { NULLPtr, NULLPtr },
-sectionName());
-  if (ConstantStrings.empty()) {
-auto i32Zero = llvm::ConstantInt::get(Int32Ty, 0);
-createNullGlobal(".objc_null_constant_string", { NULLPtr, i32Zero,
-i32Zero, i32Zero, i32Zero, NULLPtr },
-sectionName());
-  }
+  createNullGlobal(".objc_null_category", {NULLPtr, NULLPtr,
+NULLPtr, NULLPtr, NULLPtr, NULLPtr, NULLPtr},
+  sectionName());
+  createNullGlobal(".objc_null_cls_init_ref", NULLPtr,
+  sectionName());
+  createNullGlobal(".objc_null_class_ref", { NULLPtr, NULLPtr },
+  sectionName());
+  createNullGlobal(".objc_null_protocol", {NULLPtr, NULLPtr, NULLPtr,
+  NULLPtr, NULLPtr, NULLPtr, NULLPtr, NULLPtr, NULLPtr, NULLPtr,
+  NULLPtr}, sectionName());
+  createNullGlobal(".objc_null_protocol_ref", {NULLPtr},
+  sectionName());
+  createNullGlobal(".objc_null_class_alias", { NULLPtr, NULLPtr },
+  sectionName());
+  auto i32Zero = llvm::ConstantInt::get(Int32Ty, 0);
+  createNullGlobal(".objc_null_constant_string", { NULLPtr, i32Zero,
+  i32Zero, i32Zero, i32Zero, NULLPtr },
+  sectionName());
 }
 ConstantStrings.clear();
 Categories.clear();


Index: clang/lib/CodeGen/CGObjCGNU.cpp
===
--- clang/lib/CodeGen/CGObjCGNU.cpp
+++ clang/lib/CodeGen/CGObjCGNU.cpp
@@ -1614,32 +1614,24 @@
 if (!CGM.getTriple().isOSBinFormatCOFF()) {
   createNullGlobal(".objc_null_selector", {NULLPtr, NULLPtr},
   sectionName());
-  if (Categories.empty())
-createNullGlobal(".objc_null_category", {NULLPtr, NULLPtr,
-  NULLPtr, NULLPtr, NULLPtr, NULLPtr, NULLPtr},
-sectionName());
-  if (!EmittedClass) {
-createNullGlobal(".objc_null_cls_init_ref", NULLPtr,
-sectionName());
-createNullGlobal(".objc_null_class_ref", { NULLPtr, NULLPtr },
-sectionName());
-  }
-  if (!EmittedProtocol)
-createNullGlobal(".objc_null_protocol", {NULLPtr, NULLPtr, NULLPtr,
-NULLPtr, NULLPtr, NULLPtr, NULLPtr, NULLPtr, NULLPtr, NULLPtr,
-NULLPtr}, sectionName());
-  if (!EmittedProtocolRef)
-createNullGlobal(".objc_null_protocol_ref", {NULLPtr},
-sectionName());
-  if (ClassAliases.empty())
-createNullGlobal(".objc_null_class_alias", { NULLPtr, NULLPtr },
-sectionName());
-  if (ConstantStrings.e

[PATCH] D157962: [GNU ObjC] Unconditionally emit section markers.

[David Chisnall via Phabricator via cfe-commits]

theraven updated this revision to Diff 550295.
theraven added a comment.

Update test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157962

Files:
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/test/CodeGenObjC/gnu-init.m


Index: clang/test/CodeGenObjC/gnu-init.m
===
--- clang/test/CodeGenObjC/gnu-init.m
+++ clang/test/CodeGenObjC/gnu-init.m
@@ -52,7 +52,8 @@
 // CHECK-NEW: @.objc_null_class_alias = linkonce_odr hidden global { ptr, ptr 
} zeroinitializer, section "__objc_class_aliases", comdat, align 8
 // CHECK-NEW: @.objc_null_constant_string = linkonce_odr hidden global { ptr, 
i32, i32, i32, i32, ptr } zeroinitializer, section "__objc_constant_string", 
comdat, align 8
 // Make sure that the null symbols are not going to be removed, even by 
linking.
-// CHECK-NEW: @llvm.used = appending global [8 x ptr] [ptr 
@._OBJC_INIT_CLASS_X, ptr @.objc_ctor, ptr @.objc_null_selector, ptr 
@.objc_null_category, ptr @.objc_null_protocol, ptr @.objc_null_protocol_ref, 
ptr @.objc_null_class_alias, ptr @.objc_null_constant_string], section 
"llvm.metadata"
+// CHECK-NEW: @llvm.used = appending global [10 x ptr] [ptr 
@._OBJC_INIT_CLASS_X, ptr @.objc_ctor, ptr @.objc_null_selector, ptr 
@.objc_null_category, ptr @.objc_null_cls_init_ref, ptr @.objc_null_class_ref, 
ptr @.objc_null_protocol, ptr @.objc_null_protocol_ref, ptr 
@.objc_null_class_alias, ptr @.objc_null_constant_string], section 
"llvm.metadata"
+
 // Make sure that the load function and the reference to it are marked as used.
 // CHECK-NEW: @llvm.compiler.used = appending global [1 x ptr] [ptr 
@.objcv2_load_function], section "llvm.metadata"
 
Index: clang/lib/CodeGen/CGObjCGNU.cpp
===
--- clang/lib/CodeGen/CGObjCGNU.cpp
+++ clang/lib/CodeGen/CGObjCGNU.cpp
@@ -1614,32 +1614,24 @@
 if (!CGM.getTriple().isOSBinFormatCOFF()) {
   createNullGlobal(".objc_null_selector", {NULLPtr, NULLPtr},
   sectionName());
-  if (Categories.empty())
-createNullGlobal(".objc_null_category", {NULLPtr, NULLPtr,
-  NULLPtr, NULLPtr, NULLPtr, NULLPtr, NULLPtr},
-sectionName());
-  if (!EmittedClass) {
-createNullGlobal(".objc_null_cls_init_ref", NULLPtr,
-sectionName());
-createNullGlobal(".objc_null_class_ref", { NULLPtr, NULLPtr },
-sectionName());
-  }
-  if (!EmittedProtocol)
-createNullGlobal(".objc_null_protocol", {NULLPtr, NULLPtr, NULLPtr,
-NULLPtr, NULLPtr, NULLPtr, NULLPtr, NULLPtr, NULLPtr, NULLPtr,
-NULLPtr}, sectionName());
-  if (!EmittedProtocolRef)
-createNullGlobal(".objc_null_protocol_ref", {NULLPtr},
-sectionName());
-  if (ClassAliases.empty())
-createNullGlobal(".objc_null_class_alias", { NULLPtr, NULLPtr },
-sectionName());
-  if (ConstantStrings.empty()) {
-auto i32Zero = llvm::ConstantInt::get(Int32Ty, 0);
-createNullGlobal(".objc_null_constant_string", { NULLPtr, i32Zero,
-i32Zero, i32Zero, i32Zero, NULLPtr },
-sectionName());
-  }
+  createNullGlobal(".objc_null_category", {NULLPtr, NULLPtr,
+NULLPtr, NULLPtr, NULLPtr, NULLPtr, NULLPtr},
+  sectionName());
+  createNullGlobal(".objc_null_cls_init_ref", NULLPtr,
+  sectionName());
+  createNullGlobal(".objc_null_class_ref", { NULLPtr, NULLPtr },
+  sectionName());
+  createNullGlobal(".objc_null_protocol", {NULLPtr, NULLPtr, NULLPtr,
+  NULLPtr, NULLPtr, NULLPtr, NULLPtr, NULLPtr, NULLPtr, NULLPtr,
+  NULLPtr}, sectionName());
+  createNullGlobal(".objc_null_protocol_ref", {NULLPtr},
+  sectionName());
+  createNullGlobal(".objc_null_class_alias", { NULLPtr, NULLPtr },
+  sectionName());
+  auto i32Zero = llvm::ConstantInt::get(Int32Ty, 0);
+  createNullGlobal(".objc_null_constant_string", { NULLPtr, i32Zero,
+  i32Zero, i32Zero, i32Zero, NULLPtr },
+  sectionName());
 }
 ConstantStrings.clear();
 Categories.clear();


Index: clang/test/CodeGenObjC/gnu-init.m
===
--- clang/test/CodeGenObjC/gnu-init.m
+++ clang/test/CodeGenObjC/gnu-init.m
@@ -52,7 +52,8 @@
 // CHECK-NEW: @.objc_null_class_alias = linkonce_odr hidden global { ptr, ptr } zeroinitializer, section "__objc_class_aliases", comdat, align 8
 // CHECK-NEW: @.objc_null_constant_string = linkonce_odr hidden global { ptr, i32, i32, i32, i32, ptr } zeroinitializer, section "__objc_constant_string", comdat, align 8
 // Make sure that the null symbols are not going to be removed, even by linking.
-// CHECK-NEW: @llvm.used = appending global [8 x ptr] [ptr @._OBJC_INIT_CLASS_X, ptr @.objc_ctor, ptr @.objc_null_

[PATCH] D135273: [Clang][ObjC] Add optionality to property attribute strings.

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

In D135273#3936297 , @al45tair wrote:

> @theraven Any chance you could glance over this and reassure us that it isn't 
> going to break the GNU runtime if we do this? (We're adding an extra 
> attribute in the property attribute string so that we can detect `@optional` 
> properties in ObjC protocols at runtime.)

It shouldn't, we ignore any unknown property attributes.  I'd be more concerned 
about code outside the runtime.  Lots of things parse encoding strings badly, 
but the property APIs make it much easier to query known attributes and so I 
think that's a lot lower risk than changing anything in encoding strings.  It's 
a shame to use ?, since that is unknown type in type encodings and that may 
confuse some parsers..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135273

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

[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

[David Chisnall via Phabricator via cfe-commits]

theraven added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:14323
+Result = Builder.CreateIntrinsic(
+Intrinsic::ptrmask, {Args.SrcType, SrcForMask->getType(), 
Args.IntType},
+{SrcForMask, NegatedMask}, nullptr, "aligned_result");

fhahn wrote:
> lebedev.ri wrote:
> > arichardson wrote:
> > > lebedev.ri wrote:
> > > > Is sufficient amount of passes, analyses know about this intrinsic?
> > > Good question. In the simple test cases that I looked at the code 
> > > generation was equivalent. 
> > > 
> > > In our fork we still use ptrtoint+inttoptr since I implemented them 
> > > before the new intrinsic existed. But since the ptrmask instrinsic exists 
> > > now I thought I'd use it for upstreaming.
> > > I'll investigate if this results in worse codegen for more complex uses.
> > > 
> > (TLDR: before producing it in more cases in clang, i think it should be 
> > first ensured
> > that everything in middle-end is fully aware of said intrinsic. (i.e. using 
> > it vs it's
> > exploded form results in no differences in final assembly on a sufficient 
> > test coverage))
> > I'll investigate if this results in worse codegen for more complex uses.
> 
> The findings would be interesting indeed. 
> 
> One thing to watch out for is that ptrmask should give better alias analysis 
> results than ptrtoint/inttoptr. Also, IIRC some instcombine transformations 
> for ptrtoint/inttoptr are not strictly valid according to the LangRef. Not 
> sure if that changed yet.
There is currently an open review on the semantics of inttoptr / ptrtoint.  The 
current LangRef underspecifies it.  On most architectures, both are bitcasts.  
On CHERI (including ARM's Morello system), it is not safe to round trip a 
pointer via an integer at the IR or machine-code level (in C, `[u]intptr_t` is 
represented as `i8*` in the IR).  A few architectures do some arithmetic to 
cast between pointer and integer.  The mid-level optimisers are slowly getting 
better at avoiding the patterns that rely on the underspecified behaviour, but 
removing this need for them would be a benefit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71499



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

[PATCH] D45305: ObjCGNU: Fix empty v3 protocols being emitted two fields short

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

Looks correct to me.  Protocols are only referenced by pointer, so it doesn't 
matter for old versions of the runtime if these fields are present.


Repository:
  rC Clang

https://reviews.llvm.org/D45305



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

[PATCH] D45305: ObjCGNU: Fix empty v3 protocols being emitted two fields short

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

Isn't it better to test for the correct structure existing in the IR?


Repository:
  rC Clang

https://reviews.llvm.org/D45305



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

[PATCH] D45305: ObjCGNU: Fix empty v3 protocols being emitted two fields short

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

I think that we emit empty method lists so that the GCC runtime doesn't choke 
on them, though there's no reason why we couldn't with the GNUstep ABI.  It 
would therefore be nice if the test failed if we did change to emitting null so 
that we could update the test at that point and check that it's actually doing 
the right thing.

The ELF version of the v2 ABI is mostly finished now, modulo writing a load of 
clang tests, so I'll upload that for review soon.

It would also help to have a gnu- prefix on the test name so that it's easy to 
identify the relevant tests, though not essential.


Repository:
  rC Clang

https://reviews.llvm.org/D45305



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

[PATCH] D39365: [libunwind] Change unw_word_t to always have the same size as the pointer size

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

This makes things worse for us.  On CHERI, `[u]intptr_t` is a (`typedef` for a) 
built-in type that can hold a capability.  Having `unw_word_t` be `uintptr_t` 
made LLVM's libunwind considerably easier to port than the HP unwinder would 
have been, because `uintptr_t` is a type that is able to hold either any 
integer-register type or a pointer, whereas neither `uint32_t` nor `uint64_t` 
is.  This will be true for any architecture that supports any kind of 
fat-pointer representation.

What is the motivation for this change?  It appears to be changing working code 
in such a way that removes future proofing.

Replacing integer casts with `void*` casts and using `PRIxPTR` consistently 
would make life easier, though the use of `PRIxPTR` vs `PRIXPTR` seems 
inconsistent (as is the original use of `%x` vs `%X`.


https://reviews.llvm.org/D39365



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

[PATCH] D39365: [libunwind] Change unw_word_t to always have the same size as the pointer size

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

In https://reviews.llvm.org/D39365#910622, @mstorsjo wrote:

> In https://reviews.llvm.org/D39365#910590, @theraven wrote:
>
> > This makes things worse for us.  On CHERI, `[u]intptr_t` is a (`typedef` 
> > for a) built-in type that can hold a capability.  Having `unw_word_t` be 
> > `uintptr_t`
>
>
> For understanding, I guess you meant "Having `unw_word_t` be `uint64_t`" 
> here? Because othewise, that's exactly the change I'm doing - currently it's 
> `uint64_t` while I'm proposing making it `uintptr_t` - that you're saying is 
> easier to work with?


Sorry - it looks as if I read the diff back to front.  I seem to be less awake 
than I thought today...

Reading the diff the correct way around, this seems like a definite improvement.

>> though the use of `PRIxPTR` vs `PRIXPTR` seems inconsistent (as is the 
>> original use of `%x` vs `%X`.
> 
> Yes, I've kept these as inconsistent as they were originally - if peferred I 
> can make the ones I touch consistently either upper or lower case.

I'd generally prefer `PRIxPTR`, because most of the time I either don't care or 
want to copy and paste for comparison with objdump output (which uses lower 
case).


https://reviews.llvm.org/D39365



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

[PATCH] D30025: [compiler-rt] [builtins] Fix building atomic.c with GCC

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

This code is working around something that's probably a clang bug.  It would be 
better to fix the clang bug than add more complex workarounds.


Repository:
  rL LLVM

https://reviews.llvm.org/D30025



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

[PATCH] D30025: [compiler-rt] [builtins] Fix building atomic.c with GCC

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

No, it's a bug in clang.  Clang does not reject other functions that are used 
to implement builtins (if it did, compiler-rt would be a lot more difficult to 
build).


Repository:
  rL LLVM

https://reviews.llvm.org/D30025



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

[PATCH] D111792: [ObjC-GNUstep] Fix ivars for dll{im,ex}ported classes.

[David Chisnall via Phabricator via cfe-commits]

theraven created this revision.
theraven requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

If a class is dllexported or imported then the ivar offset variables
must be as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D111792

Files:
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/test/CodeGenObjC/gnustep2-dllexport-ivar.m

Index: clang/test/CodeGenObjC/gnustep2-dllexport-ivar.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/gnustep2-dllexport-ivar.m
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-windows-msvc -S -emit-llvm -fobjc-runtime=gnustep-2.0 -o - %s | FileCheck %s
+
+__attribute__((dllexport))
+@interface Cls
+{
+  int implicit_prot;
+  @public
+  int pub;
+  @protected
+  int prot;
+  @private
+  int priv;
+  @package
+  int pkg;
+}
+@end
+@implementation Cls @end
+
+// CHECK: @__objc_ivar_offset_Cls.implicit_prot.i = dllexport global i32 0
+// CHECK: @__objc_ivar_offset_Cls.pub.i = dllexport global i32 4
+// CHECK: @__objc_ivar_offset_Cls.prot.i = dllexport global i32 8
+// CHECK: @__objc_ivar_offset_Cls.priv.i = hidden global i32 12
+// CHECK: @__objc_ivar_offset_Cls.pkg.i = hidden global i32 16
+
+__attribute__((dllimport))
+@interface ClsImport
+{
+  int implicit_prot;
+  @public
+  int pub;
+  @protected
+  int prot;
+}
+@end
+
+@interface Sub : ClsImport @end
+
+@implementation Sub
+- (id)init
+{
+  implicit_prot = 0;
+  pub = 1;
+  prot = 2;
+  return self;
+}
+@end
Index: clang/lib/CodeGen/CGObjCGNU.cpp
===
--- clang/lib/CodeGen/CGObjCGNU.cpp
+++ clang/lib/CodeGen/CGObjCGNU.cpp
@@ -1657,7 +1657,7 @@
 if (global) {
   llvm::GlobalVariable *GV = lateInit.second.first;
   b.CreateAlignedStore(
-  global,
+  llvm::ConstantExpr::getBitCast(global, PtrTy),
   b.CreateStructGEP(GV->getValueType(), GV, lateInit.second.second),
   CGM.getPointerAlign().getAsAlign());
 }
@@ -1686,14 +1686,33 @@
   + '.' + Ivar->getNameAsString() + '.' + TypeEncoding;
 return Name;
   }
-  llvm::Value *EmitIvarOffset(CodeGenFunction &CGF,
+
+  llvm::GlobalVariable *EmitIvarOffsetVariable(
   const ObjCInterfaceDecl *Interface,
-  const ObjCIvarDecl *Ivar) override {
+  const ObjCIvarDecl *Ivar) {
 const std::string Name = GetIVarOffsetVariableName(Ivar->getContainingInterface(), Ivar);
 llvm::GlobalVariable *IvarOffsetPointer = TheModule.getNamedGlobal(Name);
-if (!IvarOffsetPointer)
+if (!IvarOffsetPointer) {
   IvarOffsetPointer = new llvm::GlobalVariable(TheModule, IntTy, false,
   llvm::GlobalValue::ExternalLinkage, nullptr, Name);
+  if (CGM.getTriple().isOSBinFormatCOFF()) {
+if (Interface->hasAttr() &&
+(Ivar->getCanonicalAccessControl() != ObjCIvarDecl::Private) &&
+(Ivar->getCanonicalAccessControl() != ObjCIvarDecl::Package))
+  IvarOffsetPointer->setDLLStorageClass(
+llvm::GlobalValue::DLLExportStorageClass);
+else if (Interface->hasAttr())
+  IvarOffsetPointer->setDLLStorageClass(
+llvm::GlobalValue::DLLImportStorageClass);
+  }
+}
+return IvarOffsetPointer;
+  }
+
+  llvm::Value *EmitIvarOffset(CodeGenFunction &CGF,
+  const ObjCInterfaceDecl *Interface,
+  const ObjCIvarDecl *Ivar) override {
+llvm::GlobalVariable *IvarOffsetPointer = EmitIvarOffsetVariable(Interface, Ivar);
 CharUnits Align = CGM.getIntAlign();
 llvm::Value *Offset =
 CGF.Builder.CreateAlignedLoad(IntTy, IvarOffsetPointer, Align);
@@ -1851,14 +1870,8 @@
 uint64_t BaseOffset = ComputeIvarBaseOffset(CGM, OID, IVD);
 uint64_t Offset = BaseOffset - superInstanceSize;
 llvm::Constant *OffsetValue = llvm::ConstantInt::get(IntTy, Offset);
-std::string OffsetName = GetIVarOffsetVariableName(classDecl, IVD);
-llvm::GlobalVariable *OffsetVar = TheModule.getGlobalVariable(OffsetName);
-if (OffsetVar)
-  OffsetVar->setInitializer(OffsetValue);
-else
-  OffsetVar = new llvm::GlobalVariable(TheModule, IntTy,
-false, llvm::GlobalValue::ExternalLinkage,
-OffsetValue, OffsetName);
+llvm::GlobalVariable *OffsetVar = EmitIvarOffsetVariable(classDecl, IVD);
+OffsetVar->setInitializer(OffsetValue);
 auto ivarVisibility =
 (IVD->getAccessControl() == ObjCIvarDecl::Private ||
  IVD->getAccessControl() == ObjCIvarDecl::Package ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits