[PATCH] D73208: [objc_direct] fix codegen for mismatched Decl/Impl return types

[Pierre Habouzit via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6eb969b7c5b5: [objc_direct] fix codegen for mismatched 
Decl/Impl return types (authored by MadCoder).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73208

Files:
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/test/CodeGenObjC/direct-method-ret-mismatch.m


Index: clang/test/CodeGenObjC/direct-method-ret-mismatch.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/direct-method-ret-mismatch.m
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -o - 
| FileCheck %s
+
+__attribute__((objc_root_class))
+@interface Root
+- (Root *)method __attribute__((objc_direct));
+@end
+
+@implementation Root
+// CHECK-LABEL: define internal i8* @"\01-[Root something]"(
+- (id)something {
+  // CHECK: %{{[^ ]*}} = call {{.*}} @"\01-[Root method]"
+  return [self method];
+}
+
+// CHECK-LABEL: define hidden i8* @"\01-[Root method]"(
+- (id)method {
+  return self;
+}
+@end
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -4032,22 +4032,49 @@
 llvm::Function *
 CGObjCCommonMac::GenerateDirectMethod(const ObjCMethodDecl *OMD,
   const ObjCContainerDecl *CD) {
-  auto I = DirectMethodDefinitions.find(OMD->getCanonicalDecl());
-  if (I != DirectMethodDefinitions.end())
-return I->second;
+  auto *COMD = OMD->getCanonicalDecl();
+  auto I = DirectMethodDefinitions.find(COMD);
+  llvm::Function *OldFn = nullptr, *Fn = nullptr;
 
-  SmallString<256> Name;
-  GetNameForMethod(OMD, CD, Name, /*ignoreCategoryNamespace*/true);
+  if (I != DirectMethodDefinitions.end()) {
+// Objective-C allows for the declaration and implementation types
+// to differ slightly.
+//
+// If we're being asked for the Function associated for a method
+// implementation, a previous value might have been cached
+// based on the type of the canonical declaration.
+//
+// If these do not match, then we'll replace this function with
+// a new one that has the proper type below.
+if (!OMD->getBody() || COMD->getReturnType() == OMD->getReturnType())
+  return I->second;
+OldFn = I->second;
+  }
 
   CodeGenTypes  = CGM.getTypes();
   llvm::FunctionType *MethodTy =
 Types.GetFunctionType(Types.arrangeObjCMethodDeclaration(OMD));
-  llvm::Function *Method =
-  llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
- Name.str(), ());
-  DirectMethodDefinitions.insert(std::make_pair(OMD->getCanonicalDecl(), 
Method));
 
-  return Method;
+  if (OldFn) {
+Fn = llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
+"", ());
+Fn->takeName(OldFn);
+OldFn->replaceAllUsesWith(
+llvm::ConstantExpr::getBitCast(Fn, OldFn->getType()));
+OldFn->eraseFromParent();
+
+// Replace the cached function in the map.
+I->second = Fn;
+  } else {
+SmallString<256> Name;
+GetNameForMethod(OMD, CD, Name, /*ignoreCategoryNamespace*/ true);
+
+Fn = llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
+Name.str(), ());
+DirectMethodDefinitions.insert(std::make_pair(COMD, Fn));
+  }
+
+  return Fn;
 }
 
 void CGObjCCommonMac::GenerateDirectMethodPrologue(


Index: clang/test/CodeGenObjC/direct-method-ret-mismatch.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/direct-method-ret-mismatch.m
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -o - | FileCheck %s
+
+__attribute__((objc_root_class))
+@interface Root
+- (Root *)method __attribute__((objc_direct));
+@end
+
+@implementation Root
+// CHECK-LABEL: define internal i8* @"\01-[Root something]"(
+- (id)something {
+  // CHECK: %{{[^ ]*}} = call {{.*}} @"\01-[Root method]"
+  return [self method];
+}
+
+// CHECK-LABEL: define hidden i8* @"\01-[Root method]"(
+- (id)method {
+  return self;
+}
+@end
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -4032,22 +4032,49 @@
 llvm::Function *
 CGObjCCommonMac::GenerateDirectMethod(const ObjCMethodDecl *OMD,
   const ObjCContainerDecl *CD) {
-  auto I = DirectMethodDefinitions.find(OMD->getCanonicalDecl());
-  if (I != DirectMethodDefinitions.end())
-return I->second;
+  auto *COMD = OMD->getCanonicalDecl();
+  auto I = DirectMethodDefinitions.find(COMD);
+  llvm::Function *OldFn = nullptr, *Fn = nullptr;
 
-  SmallString<256> Name;
-  

[PATCH] D73208: [objc_direct] fix codegen for mismatched Decl/Impl return types

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder updated this revision to Diff 241613.
MadCoder marked an inline comment as done.

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

https://reviews.llvm.org/D73208

Files:
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/test/CodeGenObjC/direct-method-ret-mismatch.m


Index: clang/test/CodeGenObjC/direct-method-ret-mismatch.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/direct-method-ret-mismatch.m
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -o - 
| FileCheck %s
+
+__attribute__((objc_root_class))
+@interface Root
+- (Root *)method __attribute__((objc_direct));
+@end
+
+@implementation Root
+// CHECK-LABEL: define internal i8* @"\01-[Root something]"(
+- (id)something {
+  // CHECK: %{{[^ ]*}} = call {{.*}} @"\01-[Root method]"
+  return [self method];
+}
+
+// CHECK-LABEL: define hidden i8* @"\01-[Root method]"(
+- (id)method {
+  return self;
+}
+@end
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -4032,22 +4032,49 @@
 llvm::Function *
 CGObjCCommonMac::GenerateDirectMethod(const ObjCMethodDecl *OMD,
   const ObjCContainerDecl *CD) {
-  auto I = DirectMethodDefinitions.find(OMD->getCanonicalDecl());
-  if (I != DirectMethodDefinitions.end())
-return I->second;
+  auto *COMD = OMD->getCanonicalDecl();
+  auto I = DirectMethodDefinitions.find(COMD);
+  llvm::Function *OldFn = nullptr, *Fn = nullptr;
 
-  SmallString<256> Name;
-  GetNameForMethod(OMD, CD, Name, /*ignoreCategoryNamespace*/true);
+  if (I != DirectMethodDefinitions.end()) {
+// Objective-C allows for the declaration and implementation types
+// to differ slightly.
+//
+// If we're being asked for the Function associated for a method
+// implementation, a previous value might have been cached
+// based on the type of the canonical declaration.
+//
+// If these do not match, then we'll replace this function with
+// a new one that has the proper type below.
+if (!OMD->getBody() || COMD->getReturnType() == OMD->getReturnType())
+  return I->second;
+OldFn = I->second;
+  }
 
   CodeGenTypes  = CGM.getTypes();
   llvm::FunctionType *MethodTy =
 Types.GetFunctionType(Types.arrangeObjCMethodDeclaration(OMD));
-  llvm::Function *Method =
-  llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
- Name.str(), ());
-  DirectMethodDefinitions.insert(std::make_pair(OMD->getCanonicalDecl(), 
Method));
 
-  return Method;
+  if (OldFn) {
+Fn = llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
+"", ());
+Fn->takeName(OldFn);
+OldFn->replaceAllUsesWith(
+llvm::ConstantExpr::getBitCast(Fn, OldFn->getType()));
+OldFn->eraseFromParent();
+
+// Replace the cached function in the map.
+I->second = Fn;
+  } else {
+SmallString<256> Name;
+GetNameForMethod(OMD, CD, Name, /*ignoreCategoryNamespace*/ true);
+
+Fn = llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
+Name.str(), ());
+DirectMethodDefinitions.insert(std::make_pair(COMD, Fn));
+  }
+
+  return Fn;
 }
 
 void CGObjCCommonMac::GenerateDirectMethodPrologue(


Index: clang/test/CodeGenObjC/direct-method-ret-mismatch.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/direct-method-ret-mismatch.m
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -o - | FileCheck %s
+
+__attribute__((objc_root_class))
+@interface Root
+- (Root *)method __attribute__((objc_direct));
+@end
+
+@implementation Root
+// CHECK-LABEL: define internal i8* @"\01-[Root something]"(
+- (id)something {
+  // CHECK: %{{[^ ]*}} = call {{.*}} @"\01-[Root method]"
+  return [self method];
+}
+
+// CHECK-LABEL: define hidden i8* @"\01-[Root method]"(
+- (id)method {
+  return self;
+}
+@end
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -4032,22 +4032,49 @@
 llvm::Function *
 CGObjCCommonMac::GenerateDirectMethod(const ObjCMethodDecl *OMD,
   const ObjCContainerDecl *CD) {
-  auto I = DirectMethodDefinitions.find(OMD->getCanonicalDecl());
-  if (I != DirectMethodDefinitions.end())
-return I->second;
+  auto *COMD = OMD->getCanonicalDecl();
+  auto I = DirectMethodDefinitions.find(COMD);
+  llvm::Function *OldFn = nullptr, *Fn = nullptr;
 
-  SmallString<256> Name;
-  GetNameForMethod(OMD, CD, Name, /*ignoreCategoryNamespace*/true);
+  if (I != DirectMethodDefinitions.end()) {
+// Objective-C allows for the declaration and 

[PATCH] D73208: [objc_direct] fix codegen for mismatched Decl/Impl return types

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

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

LGTM, with one style nitpick.




Comment at: clang/lib/CodeGen/CGObjCMac.cpp:4049-4051
+if (!OMD->getBody() || COMD->getReturnType() == OMD->getReturnType()) {
+  return I->second;
+}

I think the LLVM style is to leave out these braces.


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

https://reviews.llvm.org/D73208



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

[PATCH] D73208: [objc_direct] fix codegen for mismatched Decl/Impl return types

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder updated this revision to Diff 241599.
MadCoder added a comment.

@dexonsmith here, I still hook the same method but do it in a more 
LLVM-approved way ;)

It works with minimap perf impect because the only case we call 
GenerateDirectMethod with an Implementation is if we're about to codegen the 
body, so the case when you build against a header without seing the 
@implementation will not be affected perf-wise.


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

https://reviews.llvm.org/D73208

Files:
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/test/CodeGenObjC/direct-method-ret-mismatch.m


Index: clang/test/CodeGenObjC/direct-method-ret-mismatch.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/direct-method-ret-mismatch.m
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -o - 
| FileCheck %s
+
+__attribute__((objc_root_class))
+@interface Root
+- (Root *)method __attribute__((objc_direct));
+@end
+
+@implementation Root
+// CHECK-LABEL: define internal i8* @"\01-[Root something]"(
+- (id)something {
+  // CHECK: %{{[^ ]*}} = call {{.*}} @"\01-[Root method]"
+  return [self method];
+}
+
+// CHECK-LABEL: define hidden i8* @"\01-[Root method]"(
+- (id)method {
+  return self;
+}
+@end
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -4032,22 +4032,50 @@
 llvm::Function *
 CGObjCCommonMac::GenerateDirectMethod(const ObjCMethodDecl *OMD,
   const ObjCContainerDecl *CD) {
-  auto I = DirectMethodDefinitions.find(OMD->getCanonicalDecl());
-  if (I != DirectMethodDefinitions.end())
-return I->second;
+  auto *COMD = OMD->getCanonicalDecl();
+  auto I = DirectMethodDefinitions.find(COMD);
+  llvm::Function *OldFn = nullptr, *Fn = nullptr;
 
-  SmallString<256> Name;
-  GetNameForMethod(OMD, CD, Name, /*ignoreCategoryNamespace*/true);
+  if (I != DirectMethodDefinitions.end()) {
+// Objective-C allows for the declaration and implementation types
+// to differ slightly.
+   //
+   // If we're being asked for the Function associated for a method
+   // implementation, a previous value might have been cached
+   // based on the type of the canonical declaration.
+   //
+   // If these do not match, then we'll replace this function with
+   // a new one that has the proper type below.
+if (!OMD->getBody() || COMD->getReturnType() == OMD->getReturnType()) {
+  return I->second;
+}
+OldFn = I->second;
+  }
 
   CodeGenTypes  = CGM.getTypes();
   llvm::FunctionType *MethodTy =
 Types.GetFunctionType(Types.arrangeObjCMethodDeclaration(OMD));
-  llvm::Function *Method =
-  llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
- Name.str(), ());
-  DirectMethodDefinitions.insert(std::make_pair(OMD->getCanonicalDecl(), 
Method));
 
-  return Method;
+  if (OldFn) {
+Fn = llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
+"", ());
+Fn->takeName(OldFn);
+OldFn->replaceAllUsesWith(
+llvm::ConstantExpr::getBitCast(Fn, OldFn->getType()));
+OldFn->eraseFromParent();
+
+   // Replace the cached function in the map.
+I->second = Fn;
+  } else {
+SmallString<256> Name;
+GetNameForMethod(OMD, CD, Name, /*ignoreCategoryNamespace*/ true);
+
+Fn = llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
+Name.str(), ());
+DirectMethodDefinitions.insert(std::make_pair(COMD, Fn));
+  }
+
+  return Fn;
 }
 
 void CGObjCCommonMac::GenerateDirectMethodPrologue(


Index: clang/test/CodeGenObjC/direct-method-ret-mismatch.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/direct-method-ret-mismatch.m
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -o - | FileCheck %s
+
+__attribute__((objc_root_class))
+@interface Root
+- (Root *)method __attribute__((objc_direct));
+@end
+
+@implementation Root
+// CHECK-LABEL: define internal i8* @"\01-[Root something]"(
+- (id)something {
+  // CHECK: %{{[^ ]*}} = call {{.*}} @"\01-[Root method]"
+  return [self method];
+}
+
+// CHECK-LABEL: define hidden i8* @"\01-[Root method]"(
+- (id)method {
+  return self;
+}
+@end
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -4032,22 +4032,50 @@
 llvm::Function *
 CGObjCCommonMac::GenerateDirectMethod(const ObjCMethodDecl *OMD,
   const ObjCContainerDecl *CD) {
-  auto I = DirectMethodDefinitions.find(OMD->getCanonicalDecl());
-  if (I != 

[PATCH] D73208: [objc_direct] fix codegen for mismatched Decl/Impl return types

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder updated this revision to Diff 241601.
MadCoder added a comment.

damn you tabs!


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

https://reviews.llvm.org/D73208

Files:
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/test/CodeGenObjC/direct-method-ret-mismatch.m


Index: clang/test/CodeGenObjC/direct-method-ret-mismatch.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/direct-method-ret-mismatch.m
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -o - 
| FileCheck %s
+
+__attribute__((objc_root_class))
+@interface Root
+- (Root *)method __attribute__((objc_direct));
+@end
+
+@implementation Root
+// CHECK-LABEL: define internal i8* @"\01-[Root something]"(
+- (id)something {
+  // CHECK: %{{[^ ]*}} = call {{.*}} @"\01-[Root method]"
+  return [self method];
+}
+
+// CHECK-LABEL: define hidden i8* @"\01-[Root method]"(
+- (id)method {
+  return self;
+}
+@end
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -4032,22 +4032,50 @@
 llvm::Function *
 CGObjCCommonMac::GenerateDirectMethod(const ObjCMethodDecl *OMD,
   const ObjCContainerDecl *CD) {
-  auto I = DirectMethodDefinitions.find(OMD->getCanonicalDecl());
-  if (I != DirectMethodDefinitions.end())
-return I->second;
+  auto *COMD = OMD->getCanonicalDecl();
+  auto I = DirectMethodDefinitions.find(COMD);
+  llvm::Function *OldFn = nullptr, *Fn = nullptr;
 
-  SmallString<256> Name;
-  GetNameForMethod(OMD, CD, Name, /*ignoreCategoryNamespace*/true);
+  if (I != DirectMethodDefinitions.end()) {
+// Objective-C allows for the declaration and implementation types
+// to differ slightly.
+//
+// If we're being asked for the Function associated for a method
+// implementation, a previous value might have been cached
+// based on the type of the canonical declaration.
+//
+// If these do not match, then we'll replace this function with
+// a new one that has the proper type below.
+if (!OMD->getBody() || COMD->getReturnType() == OMD->getReturnType()) {
+  return I->second;
+}
+OldFn = I->second;
+  }
 
   CodeGenTypes  = CGM.getTypes();
   llvm::FunctionType *MethodTy =
 Types.GetFunctionType(Types.arrangeObjCMethodDeclaration(OMD));
-  llvm::Function *Method =
-  llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
- Name.str(), ());
-  DirectMethodDefinitions.insert(std::make_pair(OMD->getCanonicalDecl(), 
Method));
 
-  return Method;
+  if (OldFn) {
+Fn = llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
+"", ());
+Fn->takeName(OldFn);
+OldFn->replaceAllUsesWith(
+llvm::ConstantExpr::getBitCast(Fn, OldFn->getType()));
+OldFn->eraseFromParent();
+
+// Replace the cached function in the map.
+I->second = Fn;
+  } else {
+SmallString<256> Name;
+GetNameForMethod(OMD, CD, Name, /*ignoreCategoryNamespace*/ true);
+
+Fn = llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
+Name.str(), ());
+DirectMethodDefinitions.insert(std::make_pair(COMD, Fn));
+  }
+
+  return Fn;
 }
 
 void CGObjCCommonMac::GenerateDirectMethodPrologue(


Index: clang/test/CodeGenObjC/direct-method-ret-mismatch.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/direct-method-ret-mismatch.m
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -o - | FileCheck %s
+
+__attribute__((objc_root_class))
+@interface Root
+- (Root *)method __attribute__((objc_direct));
+@end
+
+@implementation Root
+// CHECK-LABEL: define internal i8* @"\01-[Root something]"(
+- (id)something {
+  // CHECK: %{{[^ ]*}} = call {{.*}} @"\01-[Root method]"
+  return [self method];
+}
+
+// CHECK-LABEL: define hidden i8* @"\01-[Root method]"(
+- (id)method {
+  return self;
+}
+@end
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -4032,22 +4032,50 @@
 llvm::Function *
 CGObjCCommonMac::GenerateDirectMethod(const ObjCMethodDecl *OMD,
   const ObjCContainerDecl *CD) {
-  auto I = DirectMethodDefinitions.find(OMD->getCanonicalDecl());
-  if (I != DirectMethodDefinitions.end())
-return I->second;
+  auto *COMD = OMD->getCanonicalDecl();
+  auto I = DirectMethodDefinitions.find(COMD);
+  llvm::Function *OldFn = nullptr, *Fn = nullptr;
 
-  SmallString<256> Name;
-  GetNameForMethod(OMD, CD, Name, /*ignoreCategoryNamespace*/true);
+  if (I != DirectMethodDefinitions.end()) {
+// Objective-C allows for the 

[PATCH] D73208: [objc_direct] fix codegen for mismatched Decl/Impl return types

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

In D73208#1837909 , @MadCoder wrote:

> In D73208#1836789 , @dexonsmith 
> wrote:
>
> > I think we're talking across each other.  The idea is check the type when 
> > generating the implementation; if it's not correct, you fix it (need to 
> > update existing uses to bitcast).  So the type used for the implementation 
> > would match dynamic methods.
>
>
> ah I see, I don't know how to do that, I couldn't figure out how to fix the 
> function declaration, if you want to give it a stab I'd love it.


I missed that question until now.  This uses `llvm::Value::replaceAllUsesWith`, 
if you `git grep` through clang you'll find examples.

Here's one from `CodeGenFunction::AddInitializerToStaticVarDecl`:

  // The initializer may differ in type from the global. Rewrite
  // the global to match the initializer.  (We have to do this
  // because some types, like unions, can't be completely represented
  // in the LLVM type system.)
  if (GV->getType()->getElementType() != Init->getType()) {
llvm::GlobalVariable *OldGV = GV;
  
GV = new llvm::GlobalVariable(CGM.getModule(), Init->getType(),
  OldGV->isConstant(),
  OldGV->getLinkage(), Init, "",
  /*InsertBefore*/ OldGV,
  OldGV->getThreadLocalMode(),
   CGM.getContext().getTargetAddressSpace(D.getType()));
GV->setVisibility(OldGV->getVisibility());
GV->setDSOLocal(OldGV->isDSOLocal());
GV->setComdat(OldGV->getComdat());
  
// Steal the name of the old global
GV->takeName(OldGV);
  
// Replace all uses of the old global with the new global
llvm::Constant *NewPtrForOldDecl =
llvm::ConstantExpr::getBitCast(GV, OldGV->getType());
OldGV->replaceAllUsesWith(NewPtrForOldDecl);
  
// Erase the old global, since it is no longer used.
OldGV->eraseFromParent();
  }

This example is analogous to your case.  The pattern is:

1. Create the new thing (with no name, or with a temporary/bad name).
2. Steal the name of the old thing.
3. Bitcast from the new thing to the old type.
4. RAUW to replace all uses of the old thing with the bitcast-of-new-thing from 
(3).
5. Delete the old thing.

The RAUW isn't going to fix up the `DenseMap` you have on the side though, so 
you'll need to handle that explicitly; likely during step (4) makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73208



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

[PATCH] D73208: [objc_direct] fix codegen for mismatched Decl/Impl return types

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added a comment.

In D73208#1836789 , @dexonsmith wrote:

> In D73208#1836722 , @MadCoder wrote:
>
> > In D73208#1836704 , @dexonsmith 
> > wrote:
> >
> > > In D73208#1835264 , @MadCoder 
> > > wrote:
> > >
> > > > In D73208#1835051 , 
> > > > @dexonsmith wrote:
> > > >
> > > > > Why isn't a similar dance needed for non-direct methods?
> > > >
> > > >
> > > > because non direct methods do not need an `llvm::Function` to be 
> > > > synthesized at the call-site. direct methods do, and they form one with 
> > > > the type of the declaration they see. Then that same `llvm::Function` 
> > > > is used when you CodeGen the Implementation, so if there's a mismatch, 
> > > > sadness ensues because the LLVM IR verifier will notice the discrepancy 
> > > > between the declared return type of the function and the actual types 
> > > > coming out of the `ret` codepaths.
> > > >
> > > > Regular obj-C methods use the _implementation_ types for the codegen 
> > > > (the declaration(s) aren't even consulted) and I want to stick at what 
> > > > obj-c does as much as I can.
> > > >
> > > > (as a data point: If you use obj-C types with C functions, the type of 
> > > > the first declaration seen is used instead).
> > >
> > >
> > > Okay, that makes sense to me.
> > >
> > > Another solution would be to change IRGen for the implementation: if the 
> > > declaration already exists (`getFunction`), do a bitcast + RAUW dance to 
> > > fix it up (and update the `DirectMethodDefinitions` table).  WDYT?
> >
> >
> > I didn't want to do that because that would mean that the type used for the 
> > implementation would depart from dynamic Objective-C methods, and it feels 
> > that it shouldn't. hence I took this option.
>
>
> I think we're talking across each other.  The idea is check the type when 
> generating the implementation; if it's not correct, you fix it (need to 
> update existing uses to bitcast).  So the type used for the implementation 
> would match dynamic methods.


ah I see, I don't know how to do that, I couldn't figure out how to fix the 
function declaration, if you want to give it a stab I'd love it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73208



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

[PATCH] D73208: [objc_direct] fix codegen for mismatched Decl/Impl return types

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

In D73208#1836722 , @MadCoder wrote:

> In D73208#1836704 , @dexonsmith 
> wrote:
>
> > In D73208#1835264 , @MadCoder 
> > wrote:
> >
> > > In D73208#1835051 , @dexonsmith 
> > > wrote:
> > >
> > > > Why isn't a similar dance needed for non-direct methods?
> > >
> > >
> > > because non direct methods do not need an `llvm::Function` to be 
> > > synthesized at the call-site. direct methods do, and they form one with 
> > > the type of the declaration they see. Then that same `llvm::Function` is 
> > > used when you CodeGen the Implementation, so if there's a mismatch, 
> > > sadness ensues because the LLVM IR verifier will notice the discrepancy 
> > > between the declared return type of the function and the actual types 
> > > coming out of the `ret` codepaths.
> > >
> > > Regular obj-C methods use the _implementation_ types for the codegen (the 
> > > declaration(s) aren't even consulted) and I want to stick at what obj-c 
> > > does as much as I can.
> > >
> > > (as a data point: If you use obj-C types with C functions, the type of 
> > > the first declaration seen is used instead).
> >
> >
> > Okay, that makes sense to me.
> >
> > Another solution would be to change IRGen for the implementation: if the 
> > declaration already exists (`getFunction`), do a bitcast + RAUW dance to 
> > fix it up (and update the `DirectMethodDefinitions` table).  WDYT?
>
>
> I didn't want to do that because that would mean that the type used for the 
> implementation would depart from dynamic Objective-C methods, and it feels 
> that it shouldn't. hence I took this option.


I think we're talking across each other.  The idea is check the type when 
generating the implementation; if it's not correct, you fix it (need to update 
existing uses to bitcast).  So the type used for the implementation would match 
dynamic methods.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73208



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

[PATCH] D73208: [objc_direct] fix codegen for mismatched Decl/Impl return types

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added a comment.

In D73208#1836704 , @dexonsmith wrote:

> In D73208#1835264 , @MadCoder wrote:
>
> > In D73208#1835051 , @dexonsmith 
> > wrote:
> >
> > > Why isn't a similar dance needed for non-direct methods?
> >
> >
> > because non direct methods do not need an `llvm::Function` to be 
> > synthesized at the call-site. direct methods do, and they form one with the 
> > type of the declaration they see. Then that same `llvm::Function` is used 
> > when you CodeGen the Implementation, so if there's a mismatch, sadness 
> > ensues because the LLVM IR verifier will notice the discrepancy between the 
> > declared return type of the function and the actual types coming out of the 
> > `ret` codepaths.
> >
> > Regular obj-C methods use the _implementation_ types for the codegen (the 
> > declaration(s) aren't even consulted) and I want to stick at what obj-c 
> > does as much as I can.
> >
> > (as a data point: If you use obj-C types with C functions, the type of the 
> > first declaration seen is used instead).
>
>
> Okay, that makes sense to me.
>
> Another solution would be to change IRGen for the implementation: if the 
> declaration already exists (`getFunction`), do a bitcast + RAUW dance to fix 
> it up (and update the `DirectMethodDefinitions` table).  WDYT?


I didn't want to do that because that would mean that the type used for the 
implementation would depart from dynamic Objective-C methods, and it feels that 
it shouldn't. hence I took this option.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73208



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

[PATCH] D73208: [objc_direct] fix codegen for mismatched Decl/Impl return types

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

In D73208#1835264 , @MadCoder wrote:

> In D73208#1835051 , @dexonsmith 
> wrote:
>
> > Why isn't a similar dance needed for non-direct methods?
>
>
> because non direct methods do not need an `llvm::Function` to be synthesized 
> at the call-site. direct methods do, and they form one with the type of the 
> declaration they see. Then that same `llvm::Function` is used when you 
> CodeGen the Implementation, so if there's a mismatch, sadness ensues because 
> the LLVM IR verifier will notice the discrepancy between the declared return 
> type of the function and the actual types coming out of the `ret` codepaths.
>
> Regular obj-C methods use the _implementation_ types for the codegen (the 
> declaration(s) aren't even consulted) and I want to stick at what obj-c does 
> as much as I can.
>
> (as a data point: If you use obj-C types with C functions, the type of the 
> first declaration seen is used instead).


Okay, that makes sense to me.

Another solution would be to change IRGen for the implementation: if the 
declaration already exists (`getFunction`), do a bitcast + RAUW dance to fix it 
up (and update the `DirectMethodDefinitions` table).  WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73208



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

[PATCH] D73208: [objc_direct] fix codegen for mismatched Decl/Impl return types

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added a comment.

In D73208#1835051 , @dexonsmith wrote:

> Why isn't a similar dance needed for non-direct methods?


because non direct methods do not need an `llvm::Function` to be synthesized at 
the call-site. direct methods do, and they form one with the type of the 
declaration they see. Then that same `llvm::Function` is used when you CodeGen 
the Implementation, so if there's a mismatch, sadness ensues because the LLVM 
IR verifier will notice the discrepancy between the declared return type of the 
function and the actual types coming out of the `ret` codepaths.

Regular obj-C methods use the _implementation_ types for the codegen (the 
declaration(s) aren't even consulted) and I want to stick at what obj-c does as 
much as I can.

(as a data point: If you use obj-C types with C functions, the type of the 
first declaration seen is used instead).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73208



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

[PATCH] D73208: [objc_direct] fix codegen for mismatched Decl/Impl return types

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

Why isn't a similar dance needed for non-direct methods?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73208



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

[PATCH] D73208: [objc_direct] fix codegen for mismatched Decl/Impl return types

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder created this revision.
MadCoder added reviewers: dexonsmith, ahatanak, erik.pilkington, arphaman.
MadCoder added a project: clang.
Herald added a subscriber: cfe-commits.

For non direct methods, the codegen uses the type of the Implementation.
Because Objective-C rules allow some differences between the Declaration
and Implementation return types, when the Implementation is in this
translation unit, the type of the Implementation should be preferred to
emit the Function over the Declaration.

Radar-Id: rdar://problem/58797748


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D73208

Files:
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/test/CodeGenObjC/direct-method-ret-mismatch.m


Index: clang/test/CodeGenObjC/direct-method-ret-mismatch.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/direct-method-ret-mismatch.m
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -o - 
| FileCheck %s
+
+__attribute__((objc_root_class))
+@interface Root
+- (Root *)method __attribute__((objc_direct));
+@end
+
+@implementation Root
+// CHECK-LABEL: define internal i8* @"\01-[Root something]"(
+- (id)something {
+  // CHECK: %{{[^ ]*}} = call {{.*}} @"\01-[Root method]"
+  return [self method];
+}
+
+// CHECK-LABEL: define hidden i8* @"\01-[Root method]"(
+- (id)method {
+  return self;
+}
+@end
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -4029,10 +4029,28 @@
 llvm::Function *
 CGObjCCommonMac::GenerateDirectMethod(const ObjCMethodDecl *OMD,
   const ObjCContainerDecl *CD) {
-  auto I = DirectMethodDefinitions.find(OMD->getCanonicalDecl());
+  auto *COMD = OMD->getCanonicalDecl();
+  auto I = DirectMethodDefinitions.find(COMD);
   if (I != DirectMethodDefinitions.end())
 return I->second;
 
+  // If this translation unit sees the implementation, we need to use
+  // it to generate the llvm::Function below. It is important to do so
+  // because Objective-C allows for return types to be somewhat mismatched
+  // and the Body of the function will expect to be generated with
+  // the type of its implementation.
+  if (!OMD->getBody()) {
+if (auto *OI = dyn_cast(CD)) {
+  if (auto *Impl = OI->getImplementation())
+if (auto *M = Impl->getMethod(OMD->getSelector(), 
OMD->isInstanceMethod()))
+  OMD = M;
+} else if (auto *CI = dyn_cast(CD)) {
+  if (auto *Impl = OI->getImplementation())
+if (auto *M = Impl->getMethod(OMD->getSelector(), 
OMD->isInstanceMethod()))
+  OMD = M;
+}
+  }
+
   SmallString<256> Name;
   GetNameForMethod(OMD, CD, Name, /*ignoreCategoryNamespace*/true);
 
@@ -4042,7 +4060,7 @@
   llvm::Function *Method =
   llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
  Name.str(), ());
-  DirectMethodDefinitions.insert(std::make_pair(OMD->getCanonicalDecl(), 
Method));
+  DirectMethodDefinitions.insert(std::make_pair(COMD, Method));
 
   return Method;
 }


Index: clang/test/CodeGenObjC/direct-method-ret-mismatch.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/direct-method-ret-mismatch.m
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -o - | FileCheck %s
+
+__attribute__((objc_root_class))
+@interface Root
+- (Root *)method __attribute__((objc_direct));
+@end
+
+@implementation Root
+// CHECK-LABEL: define internal i8* @"\01-[Root something]"(
+- (id)something {
+  // CHECK: %{{[^ ]*}} = call {{.*}} @"\01-[Root method]"
+  return [self method];
+}
+
+// CHECK-LABEL: define hidden i8* @"\01-[Root method]"(
+- (id)method {
+  return self;
+}
+@end
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -4029,10 +4029,28 @@
 llvm::Function *
 CGObjCCommonMac::GenerateDirectMethod(const ObjCMethodDecl *OMD,
   const ObjCContainerDecl *CD) {
-  auto I = DirectMethodDefinitions.find(OMD->getCanonicalDecl());
+  auto *COMD = OMD->getCanonicalDecl();
+  auto I = DirectMethodDefinitions.find(COMD);
   if (I != DirectMethodDefinitions.end())
 return I->second;
 
+  // If this translation unit sees the implementation, we need to use
+  // it to generate the llvm::Function below. It is important to do so
+  // because Objective-C allows for return types to be somewhat mismatched
+  // and the Body of the function will expect to be generated with
+  // the type of its implementation.
+  if (!OMD->getBody()) {
+if (auto *OI = dyn_cast(CD)) {
+  if (auto *Impl = OI->getImplementation())
+if (auto *M = Impl->getMethod(OMD->getSelector(),