[PATCH] D112113: [ObjC] type method metadata `_imp`, messenger routine at callsite with program address space

[Matt Jacobson 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 rGc8b2f3f51bd9: [ObjC] type method metadata `_imp`, messenger 
routine at callsite with program… (authored by mhjacobson).
Herald added a project: All.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112113

Files:
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/CodeGen/CGObjCRuntime.cpp
  clang/test/CodeGen/avr/objc-method.m

Index: clang/test/CodeGen/avr/objc-method.m
===
--- /dev/null
+++ clang/test/CodeGen/avr/objc-method.m
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -triple avr -emit-llvm -fobjc-runtime=macosx %s -o /dev/null
+
+__attribute__((objc_root_class))
+@interface Foo
+
+- (id)foo;
+- (id)bar;
+
+@end
+
+@implementation Foo
+
+- (id)foo {
+  return self;
+}
+
+- (id)bar {
+  return [self foo];
+}
+
+@end
Index: clang/lib/CodeGen/CGObjCRuntime.cpp
===
--- clang/lib/CodeGen/CGObjCRuntime.cpp
+++ clang/lib/CodeGen/CGObjCRuntime.cpp
@@ -360,13 +360,15 @@
 CGObjCRuntime::getMessageSendInfo(const ObjCMethodDecl *method,
   QualType resultType,
   CallArgList &callArgs) {
+  unsigned ProgramAS = CGM.getDataLayout().getProgramAddressSpace();
+
   // If there's a method, use information from that.
   if (method) {
 const CGFunctionInfo &signature =
   CGM.getTypes().arrangeObjCMessageSendSignature(method, callArgs[0].Ty);
 
 llvm::PointerType *signatureType =
-  CGM.getTypes().GetFunctionType(signature)->getPointerTo();
+  CGM.getTypes().GetFunctionType(signature)->getPointerTo(ProgramAS);
 
 const CGFunctionInfo &signatureForCall =
   CGM.getTypes().arrangeCall(signature, callArgs);
@@ -380,7 +382,7 @@
 
   // Derive the signature to call from that.
   llvm::PointerType *signatureType =
-CGM.getTypes().GetFunctionType(argsInfo)->getPointerTo();
+CGM.getTypes().GetFunctionType(argsInfo)->getPointerTo(ProgramAS);
   return MessageSendInfo(argsInfo, signatureType);
 }
 
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -174,6 +174,7 @@
 public:
   llvm::IntegerType *ShortTy, *IntTy, *LongTy;
   llvm::PointerType *Int8PtrTy, *Int8PtrPtrTy;
+  llvm::PointerType *Int8PtrProgramASTy;
   llvm::Type *IvarOffsetVarTy;
 
   /// ObjectPtrTy - LLVM type for object handles (typeof(id))
@@ -5739,11 +5740,13 @@
 {
   CodeGen::CodeGenTypes &Types = CGM.getTypes();
   ASTContext &Ctx = CGM.getContext();
+  unsigned ProgramAS = CGM.getDataLayout().getProgramAddressSpace();
 
   ShortTy = cast(Types.ConvertType(Ctx.ShortTy));
   IntTy = CGM.IntTy;
   LongTy = cast(Types.ConvertType(Ctx.LongTy));
   Int8PtrTy = CGM.Int8PtrTy;
+  Int8PtrProgramASTy = llvm::PointerType::get(CGM.Int8Ty, ProgramAS);
   Int8PtrPtrTy = CGM.Int8PtrPtrTy;
 
   // arm64 targets use "int" ivar offset variables. All others,
@@ -5812,7 +5815,7 @@
   //   char *_imp;
   // }
   MethodTy = llvm::StructType::create("struct._objc_method", SelectorPtrTy,
-  Int8PtrTy, Int8PtrTy);
+  Int8PtrTy, Int8PtrProgramASTy);
 
   // struct _objc_cache *
   CacheTy = llvm::StructType::create(VMContext, "struct._objc_cache");
@@ -6771,11 +6774,11 @@
 
   if (forProtocol) {
 // Protocol methods have no implementation. So, this entry is always NULL.
-method.addNullPointer(ObjCTypes.Int8PtrTy);
+method.addNullPointer(ObjCTypes.Int8PtrProgramASTy);
   } else {
 llvm::Function *fn = GetMethodDefinition(MD);
 assert(fn && "no definition for method?");
-method.addBitCast(fn, ObjCTypes.Int8PtrTy);
+method.addBitCast(fn, ObjCTypes.Int8PtrProgramASTy);
   }
 
   method.finishAndAddTo(builder);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112049: [ObjC] avoid crashing when emitting synthesized getter/setter and ptrdiff_t is smaller than long

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson updated this revision to Diff 451085.
Herald added a project: All.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112049

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/avr/objc-property.m


Index: clang/test/CodeGen/avr/objc-property.m
===
--- /dev/null
+++ clang/test/CodeGen/avr/objc-property.m
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple avr -emit-llvm -fobjc-runtime=macosx %s -o /dev/null
+
+__attribute__((objc_root_class))
+@interface Foo
+
+@property(strong) Foo *f;
+
+@end
+
+@implementation Foo
+
+@synthesize f = _f;
+
+@end
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -3967,6 +3967,8 @@
 
   llvm::Value *EmitIvarOffset(const ObjCInterfaceDecl *Interface,
   const ObjCIvarDecl *Ivar);
+  llvm::Value *EmitIvarOffsetAsPointerDiff(const ObjCInterfaceDecl *Interface,
+   const ObjCIvarDecl *Ivar);
   LValue EmitLValueForField(LValue Base, const FieldDecl* Field);
   LValue EmitLValueForLambdaField(const FieldDecl *Field);
 
Index: clang/lib/CodeGen/CGObjC.cpp
===
--- clang/lib/CodeGen/CGObjC.cpp
+++ clang/lib/CodeGen/CGObjC.cpp
@@ -1192,7 +1192,7 @@
   Builder.CreateLoad(GetAddrOfLocalVar(getterMethod->getCmdDecl()), "cmd");
 llvm::Value *self = Builder.CreateBitCast(LoadObjCSelf(), VoidPtrTy);
 llvm::Value *ivarOffset =
-  EmitIvarOffset(classImpl->getClassInterface(), ivar);
+EmitIvarOffsetAsPointerDiff(classImpl->getClassInterface(), ivar);
 
 CallArgList args;
 args.add(RValue::get(self), getContext().getObjCIdType());
@@ -1479,7 +1479,7 @@
 llvm::Value *self =
   Builder.CreateBitCast(LoadObjCSelf(), VoidPtrTy);
 llvm::Value *ivarOffset =
-  EmitIvarOffset(classImpl->getClassInterface(), ivar);
+EmitIvarOffsetAsPointerDiff(classImpl->getClassInterface(), ivar);
 Address argAddr = GetAddrOfLocalVar(*setterMethod->param_begin());
 llvm::Value *arg = Builder.CreateLoad(argAddr, "arg");
 arg = Builder.CreateBitCast(arg, VoidPtrTy);
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -5278,6 +5278,15 @@
   return CGM.getObjCRuntime().EmitIvarOffset(*this, Interface, Ivar);
 }
 
+llvm::Value *
+CodeGenFunction::EmitIvarOffsetAsPointerDiff(const ObjCInterfaceDecl 
*Interface,
+ const ObjCIvarDecl *Ivar) {
+  llvm::Value *OffsetValue = EmitIvarOffset(Interface, Ivar);
+  QualType PointerDiffType = getContext().getPointerDiffType();
+  return Builder.CreateZExtOrTrunc(OffsetValue,
+   getTypes().ConvertType(PointerDiffType));
+}
+
 LValue CodeGenFunction::EmitLValueForIvar(QualType ObjectTy,
   llvm::Value *BaseValue,
   const ObjCIvarDecl *Ivar,


Index: clang/test/CodeGen/avr/objc-property.m
===
--- /dev/null
+++ clang/test/CodeGen/avr/objc-property.m
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple avr -emit-llvm -fobjc-runtime=macosx %s -o /dev/null
+
+__attribute__((objc_root_class))
+@interface Foo
+
+@property(strong) Foo *f;
+
+@end
+
+@implementation Foo
+
+@synthesize f = _f;
+
+@end
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -3967,6 +3967,8 @@
 
   llvm::Value *EmitIvarOffset(const ObjCInterfaceDecl *Interface,
   const ObjCIvarDecl *Ivar);
+  llvm::Value *EmitIvarOffsetAsPointerDiff(const ObjCInterfaceDecl *Interface,
+   const ObjCIvarDecl *Ivar);
   LValue EmitLValueForField(LValue Base, const FieldDecl* Field);
   LValue EmitLValueForLambdaField(const FieldDecl *Field);
 
Index: clang/lib/CodeGen/CGObjC.cpp
===
--- clang/lib/CodeGen/CGObjC.cpp
+++ clang/lib/CodeGen/CGObjC.cpp
@@ -1192,7 +1192,7 @@
   Builder.CreateLoad(GetAddrOfLocalVar(getterMethod->getCmdDecl()), "cmd");
 llvm::Value *self = Builder.CreateBitCast(LoadObjCSelf(), VoidPtrTy);
 llvm::Value *ivarOffset =
-  EmitIvarOffset(classImpl->getClassInterface(), ivar);
+EmitIvarOffsetAsPointerDiff(classImpl->getClassInterface(), ivar);
 
 CallArgList args;
 args.add(RValue::get(self), getContext().getObjCIdType());
@

[PATCH] D106854: Pass `--start-group` and `--end-group` to the linker when using the AVR toolchain

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson created this revision.
mhjacobson added a reviewer: dylanmckay.
mhjacobson added a project: clang.
Herald added a subscriber: Jim.
mhjacobson requested review of this revision.
Herald added a subscriber: cfe-commits.

The way the GNU linker deals with static archives is that it only loads objects
from the archive that are needed by *previously seen objects*.  That is, in this
case, if libm or libc depend on symbols from libgcc, they might be out of luck.

As it happens, it's common for avr-libc to depend on symbols (specifically, the
deduplicated function prologue/epilogue) from libgcc.  The linker, as invoked by
clang currently, fails to link the program in this case because of the argument
ordering.

In general, libraries should be supplied to the linker in a dependents-then-
dependencies order.

However, to more perfectly match GCC's behavior here--and since we don't control
the libraries in question--simply use `--start-group` and `--end-group`.  These
flags tell the linker to repeatedly search all the intervening libraries until
no new undefined symbol references are created.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D106854

Files:
  clang/lib/Driver/ToolChains/AVR.cpp


Index: clang/lib/Driver/ToolChains/AVR.cpp
===
--- clang/lib/Driver/ToolChains/AVR.cpp
+++ clang/lib/Driver/ToolChains/AVR.cpp
@@ -414,6 +414,8 @@
   if (LinkStdlib) {
 assert(!CPU.empty() && "CPU name must be known in order to link stdlibs");
 
+CmdArgs.push_back("--start-group");
+
 // Add the object file for the CRT.
 std::string CrtFileName = std::string("-l:crt") + CPU + std::string(".o");
 CmdArgs.push_back(Args.MakeArgString(CrtFileName));
@@ -425,6 +427,8 @@
 // Add the link library specific to the MCU.
 CmdArgs.push_back(Args.MakeArgString(std::string("-l") + CPU));
 
+CmdArgs.push_back("--end-group");
+
 // Specify the family name as the emulation mode to use.
 // This is almost always required because otherwise avr-ld
 // will assume 'avr2' and warn about the program being larger


Index: clang/lib/Driver/ToolChains/AVR.cpp
===
--- clang/lib/Driver/ToolChains/AVR.cpp
+++ clang/lib/Driver/ToolChains/AVR.cpp
@@ -414,6 +414,8 @@
   if (LinkStdlib) {
 assert(!CPU.empty() && "CPU name must be known in order to link stdlibs");
 
+CmdArgs.push_back("--start-group");
+
 // Add the object file for the CRT.
 std::string CrtFileName = std::string("-l:crt") + CPU + std::string(".o");
 CmdArgs.push_back(Args.MakeArgString(CrtFileName));
@@ -425,6 +427,8 @@
 // Add the link library specific to the MCU.
 CmdArgs.push_back(Args.MakeArgString(std::string("-l") + CPU));
 
+CmdArgs.push_back("--end-group");
+
 // Specify the family name as the emulation mode to use.
 // This is almost always required because otherwise avr-ld
 // will assume 'avr2' and warn about the program being larger
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106854: Pass `--start-group` and `--end-group` to the linker when using the AVR toolchain

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson updated this revision to Diff 362185.
mhjacobson added a comment.

Modified regression test `Driver/avr-ld.c` to account for the new arguments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106854

Files:
  clang/lib/Driver/ToolChains/AVR.cpp
  clang/test/Driver/avr-ld.c


Index: clang/test/Driver/avr-ld.c
===
--- clang/test/Driver/avr-ld.c
+++ clang/test/Driver/avr-ld.c
@@ -1,44 +1,44 @@
 // RUN: %clang -### --target=avr -mmcu=at90s2313 --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKA %s
-// LINKA: {{".*ld.*"}} {{.*}} {{"-L.*tiny-stack"}} {{.*}} "-Tdata=0x800060" 
{{.*}} "-lat90s2313" "-mavr2"
+// LINKA: {{".*ld.*"}} {{.*}} {{"-L.*tiny-stack"}} {{.*}} "-Tdata=0x800060" 
{{.*}} "-lat90s2313" {{.*}} "-mavr2"
 
 // RUN: %clang -### --target=avr -mmcu=at90s8515 --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKB %s
-// LINKB: {{".*ld.*"}} {{.*}} "-Tdata=0x800060" {{.*}} "-lat90s8515" "-mavr2"
+// LINKB: {{".*ld.*"}} {{.*}} "-Tdata=0x800060" {{.*}} "-lat90s8515" {{.*}} 
"-mavr2"
 
 // RUN: %clang -### --target=avr -mmcu=attiny13 --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKC %s
-// LINKC: {{".*ld.*"}} {{.*}} {{"-L.*avr25/tiny-stack"}} {{.*}} 
"-Tdata=0x800060" {{.*}} "-lattiny13" "-mavr25"
+// LINKC: {{".*ld.*"}} {{.*}} {{"-L.*avr25/tiny-stack"}} {{.*}} 
"-Tdata=0x800060" {{.*}} "-lattiny13" {{.*}} "-mavr25"
 
 // RUN: %clang -### --target=avr -mmcu=attiny44 --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKD %s
-// LINKD: {{".*ld.*"}} {{.*}} {{"-L.*avr25"}} {{.*}} "-Tdata=0x800060" {{.*}} 
"-lattiny44" "-mavr25"
+// LINKD: {{".*ld.*"}} {{.*}} {{"-L.*avr25"}} {{.*}} "-Tdata=0x800060" {{.*}} 
"-lattiny44" {{.*}} "-mavr25"
 
 // RUN: %clang -### --target=avr -mmcu=atmega103 --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKE %s
-// LINKE: {{".*ld.*"}} {{.*}} {{"-L.*avr31"}} {{.*}} "-Tdata=0x800060" {{.*}} 
"-latmega103" "-mavr31"
+// LINKE: {{".*ld.*"}} {{.*}} {{"-L.*avr31"}} {{.*}} "-Tdata=0x800060" {{.*}} 
"-latmega103" {{.*}} "-mavr31"
 
 // RUN: %clang -### --target=avr -mmcu=atmega8u2 --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKF %s
-// LINKF: {{".*ld.*"}} {{.*}} {{"-L.*avr35"}} {{.*}} "-Tdata=0x800100" {{.*}} 
"-latmega8u2" "-mavr35"
+// LINKF: {{".*ld.*"}} {{.*}} {{"-L.*avr35"}} {{.*}} "-Tdata=0x800100" {{.*}} 
"-latmega8u2" {{.*}} "-mavr35"
 
 // RUN: %clang -### --target=avr -mmcu=atmega48pa --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKG %s
-// LINKG: {{".*ld.*"}} {{.*}} {{"-L.*avr4"}} {{.*}} "-Tdata=0x800100" {{.*}} 
"-latmega48pa" "-mavr4"
+// LINKG: {{".*ld.*"}} {{.*}} {{"-L.*avr4"}} {{.*}} "-Tdata=0x800100" {{.*}} 
"-latmega48pa" {{.*}} "-mavr4"
 
 // RUN: %clang -### --target=avr -mmcu=atmega328 --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKH %s
-// LINKH: {{".*ld.*"}} {{.*}} {{"-L.*avr5"}} {{.*}} "-Tdata=0x800100" {{.*}} 
"-latmega328" "-mavr5"
+// LINKH: {{".*ld.*"}} {{.*}} {{"-L.*avr5"}} {{.*}} "-Tdata=0x800100" {{.*}} 
"-latmega328" {{.*}} "-mavr5"
 
 // RUN: %clang -### --target=avr -mmcu=atmega1281 --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKI %s
-// LINKI: {{".*ld.*"}} {{.*}} {{"-L.*avr51"}} {{.*}} "-Tdata=0x800200" {{.*}} 
"-latmega1281" "-mavr51"
+// LINKI: {{".*ld.*"}} {{.*}} {{"-L.*avr51"}} {{.*}} "-Tdata=0x800200" {{.*}} 
"-latmega1281" {{.*}} "-mavr51"
 
 // RUN: %clang -### --target=avr -mmcu=atmega2560 --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKJ %s
-// LINKJ: {{".*ld.*"}} {{.*}} {{"-L.*avr6"}} {{.*}} "-Tdata=0x800200" {{.*}} 
"-latmega2560" "-mavr6"
+// LINKJ: {{".*ld.*"}} {{.*}} {{"-L.*avr6"}} {{.*}} "-Tdata=0x800200" {{.*}} 
"-latmega2560" {{.*}} "-mavr6"
 
 // RUN: %clang -### --target=avr -mmcu=attiny10 --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKK %s
-// LINKK: {{".*ld.*"}} {{.*}} {{"-L.*avrtiny"}} {{.*}} "-Tdata=0x800040" 
{{.*}} "-lattiny10" "-mavrtiny"
+// LINKK: {{".*ld.*"}} {{.*}} {{"-L.*avrtiny"}} {{.*}} "-Tdata=0x800040" 
{{.*}} "-lattiny10" {{.*}} "-mavrtiny"
 
 // RUN: %clang -### --target=avr -mmcu=atxmega16a4 --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKL %s
-// LINKL: {{".*ld.*"}} {{.*}} {{"-L.*avrxmega2"}} {{.*}} "-Tdata=0x802000" 
{{.*}} "-latxmega16a4" "-mavrxmega2"
+// LINKL: {{".*ld.*"}} {{.*}} {{"-L.*avrxmega2"}} {{.*}} "-Tdata=0x802000" 
{{.*}} "-latxmega16a4" {{.*}} "-mavrxmega2"
 
 // RUN: %clang -### --target=avr -mmcu=atxmega64b3 --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKM %s
-// LINKM: {{".*ld.*"}} {{.*}} {{"-L.*avrxmega4"}} {{.*}} "-Tdata=0x802000" 
{{.*}} "-latxmega64b3" "-mavrxmega4"
+// LINKM: {{".*ld.*"}} {{.*}} {{"-L.*avrxmega4"

[PATCH] D106854: Pass `--start-group` and `--end-group` to the linker when using the AVR toolchain

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson updated this revision to Diff 362275.
mhjacobson added a comment.

Updated the diff to add more context.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106854

Files:
  clang/lib/Driver/ToolChains/AVR.cpp
  clang/test/Driver/avr-ld.c


Index: clang/test/Driver/avr-ld.c
===
--- clang/test/Driver/avr-ld.c
+++ clang/test/Driver/avr-ld.c
@@ -1,44 +1,44 @@
 // RUN: %clang -### --target=avr -mmcu=at90s2313 --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKA %s
-// LINKA: {{".*ld.*"}} {{.*}} {{"-L.*tiny-stack"}} {{.*}} "-Tdata=0x800060" 
{{.*}} "-lat90s2313" "-mavr2"
+// LINKA: {{".*ld.*"}} {{.*}} {{"-L.*tiny-stack"}} {{.*}} "-Tdata=0x800060" 
{{.*}} "-lat90s2313" {{.*}} "-mavr2"
 
 // RUN: %clang -### --target=avr -mmcu=at90s8515 --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKB %s
-// LINKB: {{".*ld.*"}} {{.*}} "-Tdata=0x800060" {{.*}} "-lat90s8515" "-mavr2"
+// LINKB: {{".*ld.*"}} {{.*}} "-Tdata=0x800060" {{.*}} "-lat90s8515" {{.*}} 
"-mavr2"
 
 // RUN: %clang -### --target=avr -mmcu=attiny13 --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKC %s
-// LINKC: {{".*ld.*"}} {{.*}} {{"-L.*avr25/tiny-stack"}} {{.*}} 
"-Tdata=0x800060" {{.*}} "-lattiny13" "-mavr25"
+// LINKC: {{".*ld.*"}} {{.*}} {{"-L.*avr25/tiny-stack"}} {{.*}} 
"-Tdata=0x800060" {{.*}} "-lattiny13" {{.*}} "-mavr25"
 
 // RUN: %clang -### --target=avr -mmcu=attiny44 --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKD %s
-// LINKD: {{".*ld.*"}} {{.*}} {{"-L.*avr25"}} {{.*}} "-Tdata=0x800060" {{.*}} 
"-lattiny44" "-mavr25"
+// LINKD: {{".*ld.*"}} {{.*}} {{"-L.*avr25"}} {{.*}} "-Tdata=0x800060" {{.*}} 
"-lattiny44" {{.*}} "-mavr25"
 
 // RUN: %clang -### --target=avr -mmcu=atmega103 --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKE %s
-// LINKE: {{".*ld.*"}} {{.*}} {{"-L.*avr31"}} {{.*}} "-Tdata=0x800060" {{.*}} 
"-latmega103" "-mavr31"
+// LINKE: {{".*ld.*"}} {{.*}} {{"-L.*avr31"}} {{.*}} "-Tdata=0x800060" {{.*}} 
"-latmega103" {{.*}} "-mavr31"
 
 // RUN: %clang -### --target=avr -mmcu=atmega8u2 --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKF %s
-// LINKF: {{".*ld.*"}} {{.*}} {{"-L.*avr35"}} {{.*}} "-Tdata=0x800100" {{.*}} 
"-latmega8u2" "-mavr35"
+// LINKF: {{".*ld.*"}} {{.*}} {{"-L.*avr35"}} {{.*}} "-Tdata=0x800100" {{.*}} 
"-latmega8u2" {{.*}} "-mavr35"
 
 // RUN: %clang -### --target=avr -mmcu=atmega48pa --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKG %s
-// LINKG: {{".*ld.*"}} {{.*}} {{"-L.*avr4"}} {{.*}} "-Tdata=0x800100" {{.*}} 
"-latmega48pa" "-mavr4"
+// LINKG: {{".*ld.*"}} {{.*}} {{"-L.*avr4"}} {{.*}} "-Tdata=0x800100" {{.*}} 
"-latmega48pa" {{.*}} "-mavr4"
 
 // RUN: %clang -### --target=avr -mmcu=atmega328 --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKH %s
-// LINKH: {{".*ld.*"}} {{.*}} {{"-L.*avr5"}} {{.*}} "-Tdata=0x800100" {{.*}} 
"-latmega328" "-mavr5"
+// LINKH: {{".*ld.*"}} {{.*}} {{"-L.*avr5"}} {{.*}} "-Tdata=0x800100" {{.*}} 
"-latmega328" {{.*}} "-mavr5"
 
 // RUN: %clang -### --target=avr -mmcu=atmega1281 --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKI %s
-// LINKI: {{".*ld.*"}} {{.*}} {{"-L.*avr51"}} {{.*}} "-Tdata=0x800200" {{.*}} 
"-latmega1281" "-mavr51"
+// LINKI: {{".*ld.*"}} {{.*}} {{"-L.*avr51"}} {{.*}} "-Tdata=0x800200" {{.*}} 
"-latmega1281" {{.*}} "-mavr51"
 
 // RUN: %clang -### --target=avr -mmcu=atmega2560 --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKJ %s
-// LINKJ: {{".*ld.*"}} {{.*}} {{"-L.*avr6"}} {{.*}} "-Tdata=0x800200" {{.*}} 
"-latmega2560" "-mavr6"
+// LINKJ: {{".*ld.*"}} {{.*}} {{"-L.*avr6"}} {{.*}} "-Tdata=0x800200" {{.*}} 
"-latmega2560" {{.*}} "-mavr6"
 
 // RUN: %clang -### --target=avr -mmcu=attiny10 --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKK %s
-// LINKK: {{".*ld.*"}} {{.*}} {{"-L.*avrtiny"}} {{.*}} "-Tdata=0x800040" 
{{.*}} "-lattiny10" "-mavrtiny"
+// LINKK: {{".*ld.*"}} {{.*}} {{"-L.*avrtiny"}} {{.*}} "-Tdata=0x800040" 
{{.*}} "-lattiny10" {{.*}} "-mavrtiny"
 
 // RUN: %clang -### --target=avr -mmcu=atxmega16a4 --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKL %s
-// LINKL: {{".*ld.*"}} {{.*}} {{"-L.*avrxmega2"}} {{.*}} "-Tdata=0x802000" 
{{.*}} "-latxmega16a4" "-mavrxmega2"
+// LINKL: {{".*ld.*"}} {{.*}} {{"-L.*avrxmega2"}} {{.*}} "-Tdata=0x802000" 
{{.*}} "-latxmega16a4" {{.*}} "-mavrxmega2"
 
 // RUN: %clang -### --target=avr -mmcu=atxmega64b3 --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKM %s
-// LINKM: {{".*ld.*"}} {{.*}} {{"-L.*avrxmega4"}} {{.*}} "-Tdata=0x802000" 
{{.*}} "-latxmega64b3" "-mavrxmega4"
+// LINKM: {{".*ld.*"}} {{.*}} {{"-L.*avrxmega4"}} {{.*}} "-Tdata=0x802000" 
{{.*}} "-l

[PATCH] D106854: Pass `--start-group` and `--end-group` to the linker when using the AVR toolchain

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson marked an inline comment as done.
mhjacobson added inline comments.



Comment at: clang/lib/Driver/ToolChains/AVR.cpp:430
 
+CmdArgs.push_back("--end-group");
+

benshi001 wrote:
> You are appreciated to use 'git show -U99' or 'git diff -U99' for 
> better context display.
Sorry!  Fixed, I think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106854

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

[PATCH] D106854: Pass `--start-group` and `--end-group` to the linker when using the AVR toolchain

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson updated this revision to Diff 362277.
mhjacobson marked an inline comment as done.
mhjacobson added a comment.

Explicitly added `--start-group` and `--end-group` to the expectation in the 
test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106854

Files:
  clang/lib/Driver/ToolChains/AVR.cpp
  clang/test/Driver/avr-ld.c


Index: clang/test/Driver/avr-ld.c
===
--- clang/test/Driver/avr-ld.c
+++ clang/test/Driver/avr-ld.c
@@ -1,44 +1,44 @@
 // RUN: %clang -### --target=avr -mmcu=at90s2313 --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKA %s
-// LINKA: {{".*ld.*"}} {{.*}} {{"-L.*tiny-stack"}} {{.*}} "-Tdata=0x800060" 
{{.*}} "-lat90s2313" "-mavr2"
+// LINKA: {{".*ld.*"}} {{.*}} {{"-L.*tiny-stack"}} {{.*}} "-Tdata=0x800060" 
"--start-group" {{.*}} "-lat90s2313" "--end-group" "-mavr2"
 
 // RUN: %clang -### --target=avr -mmcu=at90s8515 --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKB %s
-// LINKB: {{".*ld.*"}} {{.*}} "-Tdata=0x800060" {{.*}} "-lat90s8515" "-mavr2"
+// LINKB: {{".*ld.*"}} {{.*}} "-Tdata=0x800060" "--start-group" {{.*}} 
"-lat90s8515" "--end-group" "-mavr2"
 
 // RUN: %clang -### --target=avr -mmcu=attiny13 --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKC %s
-// LINKC: {{".*ld.*"}} {{.*}} {{"-L.*avr25/tiny-stack"}} {{.*}} 
"-Tdata=0x800060" {{.*}} "-lattiny13" "-mavr25"
+// LINKC: {{".*ld.*"}} {{.*}} {{"-L.*avr25/tiny-stack"}} {{.*}} 
"-Tdata=0x800060" "--start-group" {{.*}} "-lattiny13" "--end-group" "-mavr25"
 
 // RUN: %clang -### --target=avr -mmcu=attiny44 --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKD %s
-// LINKD: {{".*ld.*"}} {{.*}} {{"-L.*avr25"}} {{.*}} "-Tdata=0x800060" {{.*}} 
"-lattiny44" "-mavr25"
+// LINKD: {{".*ld.*"}} {{.*}} {{"-L.*avr25"}} {{.*}} "-Tdata=0x800060" 
"--start-group" {{.*}} "-lattiny44" "--end-group" "-mavr25"
 
 // RUN: %clang -### --target=avr -mmcu=atmega103 --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKE %s
-// LINKE: {{".*ld.*"}} {{.*}} {{"-L.*avr31"}} {{.*}} "-Tdata=0x800060" {{.*}} 
"-latmega103" "-mavr31"
+// LINKE: {{".*ld.*"}} {{.*}} {{"-L.*avr31"}} {{.*}} "-Tdata=0x800060" 
"--start-group" {{.*}} "-latmega103" "--end-group" "-mavr31"
 
 // RUN: %clang -### --target=avr -mmcu=atmega8u2 --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKF %s
-// LINKF: {{".*ld.*"}} {{.*}} {{"-L.*avr35"}} {{.*}} "-Tdata=0x800100" {{.*}} 
"-latmega8u2" "-mavr35"
+// LINKF: {{".*ld.*"}} {{.*}} {{"-L.*avr35"}} {{.*}} "-Tdata=0x800100" 
"--start-group" {{.*}} "-latmega8u2" "--end-group" "-mavr35"
 
 // RUN: %clang -### --target=avr -mmcu=atmega48pa --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKG %s
-// LINKG: {{".*ld.*"}} {{.*}} {{"-L.*avr4"}} {{.*}} "-Tdata=0x800100" {{.*}} 
"-latmega48pa" "-mavr4"
+// LINKG: {{".*ld.*"}} {{.*}} {{"-L.*avr4"}} {{.*}} "-Tdata=0x800100" 
"--start-group" {{.*}} "-latmega48pa" "--end-group" "-mavr4"
 
 // RUN: %clang -### --target=avr -mmcu=atmega328 --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKH %s
-// LINKH: {{".*ld.*"}} {{.*}} {{"-L.*avr5"}} {{.*}} "-Tdata=0x800100" {{.*}} 
"-latmega328" "-mavr5"
+// LINKH: {{".*ld.*"}} {{.*}} {{"-L.*avr5"}} {{.*}} "-Tdata=0x800100" 
"--start-group" {{.*}} "-latmega328" "--end-group" "-mavr5"
 
 // RUN: %clang -### --target=avr -mmcu=atmega1281 --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKI %s
-// LINKI: {{".*ld.*"}} {{.*}} {{"-L.*avr51"}} {{.*}} "-Tdata=0x800200" {{.*}} 
"-latmega1281" "-mavr51"
+// LINKI: {{".*ld.*"}} {{.*}} {{"-L.*avr51"}} {{.*}} "-Tdata=0x800200" 
"--start-group" {{.*}} "-latmega1281" "--end-group" "-mavr51"
 
 // RUN: %clang -### --target=avr -mmcu=atmega2560 --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKJ %s
-// LINKJ: {{".*ld.*"}} {{.*}} {{"-L.*avr6"}} {{.*}} "-Tdata=0x800200" {{.*}} 
"-latmega2560" "-mavr6"
+// LINKJ: {{".*ld.*"}} {{.*}} {{"-L.*avr6"}} {{.*}} "-Tdata=0x800200" 
"--start-group" {{.*}} "-latmega2560" "--end-group" "-mavr6"
 
 // RUN: %clang -### --target=avr -mmcu=attiny10 --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKK %s
-// LINKK: {{".*ld.*"}} {{.*}} {{"-L.*avrtiny"}} {{.*}} "-Tdata=0x800040" 
{{.*}} "-lattiny10" "-mavrtiny"
+// LINKK: {{".*ld.*"}} {{.*}} {{"-L.*avrtiny"}} {{.*}} "-Tdata=0x800040" 
"--start-group" {{.*}} "-lattiny10" "--end-group" "-mavrtiny"
 
 // RUN: %clang -### --target=avr -mmcu=atxmega16a4 --sysroot 
%S/Inputs/basic_avr_tree %s 2>&1 | FileCheck -check-prefix LINKL %s
-// LINKL: {{".*ld.*"}} {{.*}} {{"-L.*avrxmega2"}} {{.*}} "-Tdata=0x802000" 
{{.*}} "-latxmega16a4" "-mavrxmega2"
+// LINKL: {{".*ld.*"}} {{.*}} {{"-L.*avrxmega2"}} {{.*}} "-Tdata=0x802000" 
"--start-group" {{.*}} "-latxmega1

[PATCH] D106854: Pass `--start-group` and `--end-group` to the linker when using the AVR toolchain

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson added a comment.

Thanks, Ben.  Yes, would you mind to land the patch for me?  Name and e-mail are

Matt Jacobson 

And yes, please do change the title as appropriate.  I wasn't sure what the 
proper naming convention was.

Thanks again!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106854

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

[PATCH] D107610: [AVR][clang] Pass '-fno-use-init-array' to cc1

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson created this revision.
mhjacobson added reviewers: benshi001, MaskRay, dylanmckay.
Herald added subscribers: Jim, krytarowski.
mhjacobson requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

On AVR, `.ctors` is used, not `.init_array`.  Make this the default unless 
specifically overridden by driver argument.

This matches gcc, and it matches the behavior in (e.g.) the NetBSD driver (for 
certain OS variants).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107610

Files:
  clang/lib/Driver/ToolChains/AVR.cpp
  clang/lib/Driver/ToolChains/AVR.h
  clang/test/Driver/avr-toolchain.c


Index: clang/test/Driver/avr-toolchain.c
===
--- clang/test/Driver/avr-toolchain.c
+++ clang/test/Driver/avr-toolchain.c
@@ -1,7 +1,7 @@
 // A basic clang -cc1 command-line.
 
 // RUN: %clang %s -### -no-canonical-prefixes -target avr 2>&1 | FileCheck 
-check-prefix=CC1 %s
-// CC1: clang{{.*}} "-cc1" "-triple" "avr"
+// CC1: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-fno-use-init-array"
 
 // RUN: %clang %s -### -no-canonical-prefixes -target avr --sysroot 
%S/Inputs/basic_avr_tree 2>&1 | FileCheck -check-prefix CC1A %s
 // CC1A: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-internal-isystem" 
{{".*avr/include"}}
Index: clang/lib/Driver/ToolChains/AVR.h
===
--- clang/lib/Driver/ToolChains/AVR.h
+++ clang/lib/Driver/ToolChains/AVR.h
@@ -11,8 +11,8 @@
 
 #include "Gnu.h"
 #include "clang/Driver/InputInfo.h"
-#include "clang/Driver/ToolChain.h"
 #include "clang/Driver/Tool.h"
+#include "clang/Driver/ToolChain.h"
 
 namespace clang {
 namespace driver {
@@ -26,6 +26,11 @@
   AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
 llvm::opt::ArgStringList &CC1Args) const override;
 
+  void
+  addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
+llvm::opt::ArgStringList &CC1Args,
+Action::OffloadKind DeviceOffloadKind) const override;
+
 protected:
   Tool *buildLinker() const override;
 
Index: clang/lib/Driver/ToolChains/AVR.cpp
===
--- clang/lib/Driver/ToolChains/AVR.cpp
+++ clang/lib/Driver/ToolChains/AVR.cpp
@@ -370,6 +370,16 @@
 addSystemInclude(DriverArgs, CC1Args, AVRInc);
 }
 
+void AVRToolChain::addClangTargetOptions(
+const llvm::opt::ArgList &DriverArgs, llvm::opt::ArgStringList &CC1Args,
+Action::OffloadKind DeviceOffloadKind) const {
+  // By default, use `.ctors` (not `.init_array`), as required by libgcc, which
+  // runs constructors/destructors on AVR.
+  if (!DriverArgs.hasFlag(options::OPT_fuse_init_array,
+  options::OPT_fno_use_init_array, false))
+CC1Args.push_back("-fno-use-init-array");
+}
+
 Tool *AVRToolChain::buildLinker() const {
   return new tools::AVR::Linker(getTriple(), *this, LinkStdlib);
 }


Index: clang/test/Driver/avr-toolchain.c
===
--- clang/test/Driver/avr-toolchain.c
+++ clang/test/Driver/avr-toolchain.c
@@ -1,7 +1,7 @@
 // A basic clang -cc1 command-line.
 
 // RUN: %clang %s -### -no-canonical-prefixes -target avr 2>&1 | FileCheck -check-prefix=CC1 %s
-// CC1: clang{{.*}} "-cc1" "-triple" "avr"
+// CC1: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-fno-use-init-array"
 
 // RUN: %clang %s -### -no-canonical-prefixes -target avr --sysroot %S/Inputs/basic_avr_tree 2>&1 | FileCheck -check-prefix CC1A %s
 // CC1A: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-internal-isystem" {{".*avr/include"}}
Index: clang/lib/Driver/ToolChains/AVR.h
===
--- clang/lib/Driver/ToolChains/AVR.h
+++ clang/lib/Driver/ToolChains/AVR.h
@@ -11,8 +11,8 @@
 
 #include "Gnu.h"
 #include "clang/Driver/InputInfo.h"
-#include "clang/Driver/ToolChain.h"
 #include "clang/Driver/Tool.h"
+#include "clang/Driver/ToolChain.h"
 
 namespace clang {
 namespace driver {
@@ -26,6 +26,11 @@
   AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs,
 llvm::opt::ArgStringList &CC1Args) const override;
 
+  void
+  addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
+llvm::opt::ArgStringList &CC1Args,
+Action::OffloadKind DeviceOffloadKind) const override;
+
 protected:
   Tool *buildLinker() const override;
 
Index: clang/lib/Driver/ToolChains/AVR.cpp
===
--- clang/lib/Driver/ToolChains/AVR.cpp
+++ clang/lib/Driver/ToolChains/AVR.cpp
@@ -370,6 +370,16 @@
 addSystemInclude(DriverArgs, CC1Args, AVRInc);
 }
 
+void AVRToolChain::addClangTargetOptions(
+const llvm::opt::ArgList &DriverArgs, llvm::opt::ArgStringList &CC1Args,
+Action::OffloadKind Device

[PATCH] D107610: [AVR][clang] Pass '-fno-use-init-array' to cc1

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107610

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

[PATCH] D107672: [AVR][clang] Search for avr-libc in $SYSROOT/avr

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson created this revision.
mhjacobson added reviewers: benshi001, MaskRay, dylanmckay.
Herald added a subscriber: Jim.
mhjacobson requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The paths in PossibleAVRLibcLocations are appended with `/lib/` and `/include/` 
to find the binaries and headers, respectively, for avr-libc.

It's common for avr-libc to live at `$SYSROOT/avr/{lib,include}/`, e.g. when 
`$SYSROOT` is, for example, `/opt/local/`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107672

Files:
  clang/lib/Driver/ToolChains/AVR.cpp


Index: clang/lib/Driver/ToolChains/AVR.cpp
===
--- clang/lib/Driver/ToolChains/AVR.cpp
+++ clang/lib/Driver/ToolChains/AVR.cpp
@@ -298,6 +298,7 @@
 }
 
 const StringRef PossibleAVRLibcLocations[] = {
+"/avr",
 "/usr/avr",
 "/usr/lib/avr",
 };


Index: clang/lib/Driver/ToolChains/AVR.cpp
===
--- clang/lib/Driver/ToolChains/AVR.cpp
+++ clang/lib/Driver/ToolChains/AVR.cpp
@@ -298,6 +298,7 @@
 }
 
 const StringRef PossibleAVRLibcLocations[] = {
+"/avr",
 "/usr/avr",
 "/usr/lib/avr",
 };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D107672: [AVR][clang] Search for avr-libc in $SYSROOT/avr

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson added a comment.

I'm also tempted to remove `/usr/lib/avr`, because these paths make no sense to 
me: `$SYSROOT/usr/lib/avr/lib` and `$SYSROOT/usr/lib/avr/include`.

But I haven't done that here, in case there's some reason those paths do make 
sense.  (Also, the files in `clang/test/Driver/Inputs/basic_avr_tree` make use 
of the `/usr/lib/avr/{lib,include}` structure.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107672

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

[PATCH] D107672: [AVR][clang] Search for avr-libc in $SYSROOT/avr

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson added a comment.

In D107672#2932190 , @mhjacobson 
wrote:

> I'm also tempted to remove `/usr/lib/avr`, because these paths make no sense 
> to me: `$SYSROOT/usr/lib/avr/lib` and `$SYSROOT/usr/lib/avr/include`.

Ah.  According to https://reviews.llvm.org/D54334, that was added because 
that's where Ubuntu installs avr-libc.  OK then!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107672

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

[PATCH] D107672: [AVR][clang] Search for avr-libc in $SYSROOT/avr

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson added a comment.

In D107672#2932406 , @MaskRay wrote:

> I know nearly nothing about AVR, but from `avr-gcc a.c '-###'` I doubt this 
> is the GCC behavior. GCC's library path is relative to the GCC installation. 
> `/usr/lib/gcc/avr/5.4.0/../../../avr/lib`
>
> There is a difference between sysroot inferred path and GCC installation 
> inferred path. 
> https://maskray.me/blog/2021-03-28-compiler-driver-and-cross-compilation

Wow, nice write-up!  I'll have to spend some more time to digest it fully.

At first glance, I think you're right: avr-gcc does expect avr-libc to be 
relative to itself.  That seems slightly weird to me (since avr-libc is 
technically separate from GCC), but OK.

clang's driver certainly searches for libgcc and avr-libc separately.  I guess 
you're saying it shouldn't?

Obviously the separate searching isn't something introduced here, so (unless 
you object) I'll file a bug report about it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107672

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

[PATCH] D107672: [AVR][clang] Search for avr-libc in $SYSROOT/avr

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson added a comment.

In D107672#2932413 , @mhjacobson 
wrote:

> At first glance, I think you're right: avr-gcc does expect avr-libc to be 
> relative to itself.  That seems slightly weird to me (since avr-libc is 
> technically separate from GCC), but OK.

OK, after looking at the GCC source, I see there's nothing specific to 
avr-libc.  It's just that GCC basically tries to look in 
`$PREFIX/[machine/]lib`, where `$PREFIX` is actually the path to GCC, 
dot-dotted up to where it thinks the prefix lives, to allow for the possibility 
that the whole prefix was relocated.

So, @MaskRay, correct me if I'm wrong: it seems like you're suggesting that we 
use something derived from the `GCCInstallationDetector` to find libc?  Is 
there another platform's driver I can look at as an example?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107672

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

[PATCH] D107672: [AVR][clang] Search for avr-libc in $SYSROOT/avr

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson added a comment.

In D107672#2932423 , @MaskRay wrote:

> @benshi001 This was prematurely committed. It has no test and has apparent 
> unaddressed issue. (I revamped many parts in the Linux/Gnu  search paths so 
> am quite confident about my observation.)

I'm working on a test now, and I'll submit a follow-up patch adding it, if 
that's OK.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107672

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

[PATCH] D107684: [NFC][AVR][clang] Add test for D107672

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson created this revision.
mhjacobson added reviewers: MaskRay, benshi001.
Herald added subscribers: Jim, dylanmckay.
mhjacobson requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Add test that ensures that an avr-libc in `$SYSROOT/avr` is found, modeled 
after the similar test checking for `$SYSROOT/usr/lib/avr`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107684

Files:
  clang/test/Driver/Inputs/basic_avr_tree_opt_local/opt/local/avr/include/.keep
  clang/test/Driver/Inputs/basic_avr_tree_opt_local/opt/local/avr/lib/libavr.a
  clang/test/Driver/Inputs/basic_avr_tree_opt_local/opt/local/bin/avr-ld
  
clang/test/Driver/Inputs/basic_avr_tree_opt_local/opt/local/lib/gcc/avr/5.4.0/libgcc.a
  clang/test/Driver/avr-toolchain.c


Index: clang/test/Driver/avr-toolchain.c
===
--- clang/test/Driver/avr-toolchain.c
+++ clang/test/Driver/avr-toolchain.c
@@ -6,8 +6,11 @@
 // RUN: %clang %s -### -no-canonical-prefixes -target avr --sysroot 
%S/Inputs/basic_avr_tree 2>&1 | FileCheck -check-prefix CC1A %s
 // CC1A: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-internal-isystem" 
{{".*avr/include"}}
 
-// RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree 2>&1 
-nostdinc | FileCheck -check-prefix CC1B %s
-// CC1B-NOT: "-internal-isystem" {{".*avr/include"}}
+// RUN: %clang %s -### -no-canonical-prefixes -target avr --sysroot 
%S/Inputs/basic_avr_tree_opt_local/opt/local 2>&1 | FileCheck -check-prefix 
CC1B %s
+// CC1B: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-internal-isystem" 
{{".*/opt/local/avr/include"}}
 
-// RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree 2>&1 
-nostdlibinc | FileCheck -check-prefix CC1C %s
+// RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree 2>&1 
-nostdinc | FileCheck -check-prefix CC1C %s
 // CC1C-NOT: "-internal-isystem" {{".*avr/include"}}
+
+// RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree 2>&1 
-nostdlibinc | FileCheck -check-prefix CC1D %s
+// CC1D-NOT: "-internal-isystem" {{".*avr/include"}}


Index: clang/test/Driver/avr-toolchain.c
===
--- clang/test/Driver/avr-toolchain.c
+++ clang/test/Driver/avr-toolchain.c
@@ -6,8 +6,11 @@
 // RUN: %clang %s -### -no-canonical-prefixes -target avr --sysroot %S/Inputs/basic_avr_tree 2>&1 | FileCheck -check-prefix CC1A %s
 // CC1A: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-internal-isystem" {{".*avr/include"}}
 
-// RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree 2>&1 -nostdinc | FileCheck -check-prefix CC1B %s
-// CC1B-NOT: "-internal-isystem" {{".*avr/include"}}
+// RUN: %clang %s -### -no-canonical-prefixes -target avr --sysroot %S/Inputs/basic_avr_tree_opt_local/opt/local 2>&1 | FileCheck -check-prefix CC1B %s
+// CC1B: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-internal-isystem" {{".*/opt/local/avr/include"}}
 
-// RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree 2>&1 -nostdlibinc | FileCheck -check-prefix CC1C %s
+// RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree 2>&1 -nostdinc | FileCheck -check-prefix CC1C %s
 // CC1C-NOT: "-internal-isystem" {{".*avr/include"}}
+
+// RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree 2>&1 -nostdlibinc | FileCheck -check-prefix CC1D %s
+// CC1D-NOT: "-internal-isystem" {{".*avr/include"}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D107682: [AVR][clang] Improve search for avr-libc installation path

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson added inline comments.



Comment at: clang/lib/Driver/ToolChains/AVR.cpp:457
+  std::string GCCRoot(GCCInstallation.getInstallPath());
+  std::string Path(GCCRoot + "/../../../avr");
+  if (llvm::sys::fs::is_directory(Path))

Would it be worth encapsulating this logic in an accessor method on 
GCCInstallation?


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

https://reviews.llvm.org/D107682

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

[PATCH] D107682: [AVR][clang] Improve search for avr-libc installation path

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson added a comment.

In D107682#2932599 , @benshi001 wrote:

> @mhjacobson Could you please check if my modification also works as expected 
> on your platform ?

Hm... it doesn't, because there aren't enough `..`s.

Here's what my directory tree looks like.  My gcc install consists of:

  /opt/local/bin/avr-gcc-10.3.0
  /opt/local/lib/gcc/avr/10.3.0/libgcc.a
  /opt/local/libexec/gcc/avr/10.3.0/cc1

And avr-libc is at:

  /opt/local/avr/lib/libc.a
  /opt/local/avr/include/stdio.h

It would work if you appended four `..`s (which is effectively what GCC does, 
since it backs up to the "prefix", which in my case is `/opt/local/`).


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

https://reviews.llvm.org/D107682

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

[PATCH] D107684: [NFC][AVR][clang] Add test for D107672

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson added a comment.

In D107684#2932603 , @benshi001 wrote:

> We need not add another `basic_avr_tree_opt_local`, since you added `/avr` to 
> possible avr-libc pathes, you can test your change by specifying `--sysroot  
> %S/Inputs/basic_avr_tree/usr/lib/` .

Oh, good idea.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107684

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

[PATCH] D107682: [AVR][clang] Improve search for avr-libc installation path

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson added a comment.

For other examples of using four `..`s, look in `Gnu.cpp`, where there are 
several places where `getParentLibPath()` is appended with `/../`.  
`AddMultilibPaths()`, `PushPPaths()`.

`getParentLibPath()`, in turn, is `GCCInstallPath/../../../`


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

https://reviews.llvm.org/D107682

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

[PATCH] D107684: [NFC][AVR][clang] Add test for D107672

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson updated this revision to Diff 364942.
mhjacobson added a comment.

Simplify.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107684

Files:
  clang/test/Driver/avr-toolchain.c


Index: clang/test/Driver/avr-toolchain.c
===
--- clang/test/Driver/avr-toolchain.c
+++ clang/test/Driver/avr-toolchain.c
@@ -11,3 +11,7 @@
 
 // RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree 2>&1 
-nostdlibinc | FileCheck -check-prefix CC1C %s
 // CC1C-NOT: "-internal-isystem" {{".*avr/include"}}
+
+// Test that the driver finds an avr-libc installation at `$SYSROOT/avr/`.
+// RUN: %clang %s -### -no-canonical-prefixes -target avr --sysroot 
%S/Inputs/basic_avr_tree/usr/lib 2>&1 | FileCheck -check-prefix CC1D %s
+// CC1D: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-internal-isystem" 
{{".*avr/include"}}


Index: clang/test/Driver/avr-toolchain.c
===
--- clang/test/Driver/avr-toolchain.c
+++ clang/test/Driver/avr-toolchain.c
@@ -11,3 +11,7 @@
 
 // RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree 2>&1 -nostdlibinc | FileCheck -check-prefix CC1C %s
 // CC1C-NOT: "-internal-isystem" {{".*avr/include"}}
+
+// Test that the driver finds an avr-libc installation at `$SYSROOT/avr/`.
+// RUN: %clang %s -### -no-canonical-prefixes -target avr --sysroot %S/Inputs/basic_avr_tree/usr/lib 2>&1 | FileCheck -check-prefix CC1D %s
+// CC1D: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-internal-isystem" {{".*avr/include"}}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D107684: [NFC][AVR][clang] Add test for D107672

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson added a comment.

In D107684#2932609 , @benshi001 wrote:

> In D107684#2932605 , @mhjacobson 
> wrote:
>
>> In D107684#2932603 , @benshi001 
>> wrote:
>>
>>> We need not add another `basic_avr_tree_opt_local`, since you added `/avr` 
>>> to possible avr-libc pathes, you can test your change by specifying 
>>> `--sysroot  %S/Inputs/basic_avr_tree/usr/lib/` .
>>
>> Oh, good idea.
>
> Sorry, only spefifying `--sysroot  %S/Inputs/basic_avr_tree/usr/lib/` does 
> not work, since the search for avr-ld also relies on `--sysroot`.

For the purpose of the test, that doesn't matter, since we're just looking for 
the existence of the `-internal-isystem` argument.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107684

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

[PATCH] D107684: [NFC][AVR][clang] Add test for D107672

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson added inline comments.



Comment at: clang/test/Driver/avr-toolchain.c:17
+// RUN: %clang %s -### -no-canonical-prefixes -target avr --sysroot 
%S/Inputs/basic_avr_tree/usr/lib 2>&1 | FileCheck -check-prefix CC1D %s
+// CC1D: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-internal-isystem" 
{{".*avr/include"}}

MaskRay wrote:
> Please use the pattern in linux-cross.cpp
To be clear, which pattern?  Placing each checked argument in its own check 
line?  I'm happy to do that, although the other tests here don't do so...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107684

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

[PATCH] D107682: [AVR][clang] Improve search for avr-libc installation path

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson added a comment.

In D107682#2932685 , @benshi001 wrote:

> In D107682#2932607 , @mhjacobson 
> wrote:
>
>> For other examples of using four `..`s, look in `Gnu.cpp`, where there are 
>> several places where `getParentLibPath()` is appended with `/../`.  
>> `AddMultilibPaths()`, `PushPPaths()`.
>>
>> `getParentLibPath()`, in turn, is `GCCInstallPath/../../../`
>
> Thanks for your help. I have updated my patch. Now it should work for both 
> your platform and mine. It search `GCCRoot.getParentLibPath()/avr` first, 
> otherwise `GCCRoot.getParentLibPath()/../avr`.
>
> Please help me check.

Latest diff works for me, thanks!


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

https://reviews.llvm.org/D107682

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

[PATCH] D107684: [NFC][AVR][clang] Add test for D107672

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson updated this revision to Diff 365343.
mhjacobson added a comment.

Rebase atop recent changes; use separate check lines and string substitution 
for `SYSROOT`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107684

Files:
  clang/test/Driver/avr-toolchain.c


Index: clang/test/Driver/avr-toolchain.c
===
--- clang/test/Driver/avr-toolchain.c
+++ clang/test/Driver/avr-toolchain.c
@@ -12,6 +12,13 @@
 // CHECK1-SAME: "-o" "a.out"
 // CHECK1-SAME: {{^}} "--gc-sections"
 
+/// Check the case where avr-libc lives at $SYSROOT/avr.
+// RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree/usr/lib 
-resource-dir=%S/Inputs/resource_dir 2>&1 | FileCheck --check-prefix=CHECK2 %s
+// CHECK2: clang{{.*}} "-cc1" "-triple" "avr"
+// CHECK2-SAME: "-isysroot" "[[SYSROOT:[^"]+/basic_avr_tree/usr/lib]]"
+// CHECK2-SAME: "-internal-isystem"
+// CHECK2-SAME: {{^}} "[[SYSROOT]]/avr/include"
+
 // RUN: %clang %s -### -target avr 2>&1 | FileCheck -check-prefix=CC1 %s
 // CC1: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-fno-use-init-array"
 


Index: clang/test/Driver/avr-toolchain.c
===
--- clang/test/Driver/avr-toolchain.c
+++ clang/test/Driver/avr-toolchain.c
@@ -12,6 +12,13 @@
 // CHECK1-SAME: "-o" "a.out"
 // CHECK1-SAME: {{^}} "--gc-sections"
 
+/// Check the case where avr-libc lives at $SYSROOT/avr.
+// RUN: %clang %s -### -target avr --sysroot %S/Inputs/basic_avr_tree/usr/lib -resource-dir=%S/Inputs/resource_dir 2>&1 | FileCheck --check-prefix=CHECK2 %s
+// CHECK2: clang{{.*}} "-cc1" "-triple" "avr"
+// CHECK2-SAME: "-isysroot" "[[SYSROOT:[^"]+/basic_avr_tree/usr/lib]]"
+// CHECK2-SAME: "-internal-isystem"
+// CHECK2-SAME: {{^}} "[[SYSROOT]]/avr/include"
+
 // RUN: %clang %s -### -target avr 2>&1 | FileCheck -check-prefix=CC1 %s
 // CC1: clang{{.*}} "-cc1" "-triple" "avr" {{.*}} "-fno-use-init-array"
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D107684: [NFC][AVR][clang] Add test for D107672

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson added a comment.

In D107684#2936033 , @MaskRay wrote:

> `--sysroot %S/Inputs/basic_avr_tree/usr/lib` seems incorrect.
>
> sysroot is usually a directory which contains `usr/lib` and `lib/`. 
> `somewhere/usr/lib` is not usually used as a sysroot.

Yeah, I agree it's weird.  This is basically a shortcut to avoid creating a 
duplicate sysroot in `Inputs`.  Would you prefer I create the duplicate tree?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107684

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

[PATCH] D112113: [ObjC] type method metadata `_imp`, messenger routine at callsite with program address space

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson added a comment.

Ping.  Is there anyone more suited to reviewing this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112113

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

[PATCH] D112113: [ObjC] type method metadata `_imp`, messenger routine at callsite with program address space

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson added a comment.

Well, I'm hoping it doesn't prove to be //too// much work.  🙂  With this fix 
and D112049 , most things are working quite 
well, though I'm sure I'll uncover more interesting things as I go.

As for the runtime, I've built my own: https://github.com/mhjacobson/avr-objc.  
It conforms to the Mac OS X non-fragile-ivars ABI.

By the way, I don't have write privs to LLVM, so assuming this is good to go, 
could someone commit this for me?  Thanks for all your help!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112113

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

[PATCH] D136575: [Headers] remove define of _GNU_SOURCE when including Darwin's unwind.h

[Matt Jacobson via Phabricator via cfe-commits]

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

Darwin's unwind.h has never surrounded declarations with _GNU_SOURCE guards.
The _GNU_SOURCE stuff removed here was introduced by a09e62a042, which expanded
the #include_next branch to non-Darwin platforms, including those that use
https://github.com/libunwind/libunwind (which *does* use _GNU_SOURCE guards).
That change was partially reverted by 032d422d2e, which left the _GNU_SOURCE
stuff as an "open question".

Therefore: close the question; remove the _GNU_SOURCE define.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136575

Files:
  clang/lib/Headers/unwind.h


Index: clang/lib/Headers/unwind.h
===
--- clang/lib/Headers/unwind.h
+++ clang/lib/Headers/unwind.h
@@ -14,12 +14,8 @@
 
 #if defined(__APPLE__) && __has_include_next()
 /* Darwin (from 11.x on) provide an unwind.h. If that's available,
- * use it. libunwind wraps some of its definitions in #ifdef _GNU_SOURCE,
- * so define that around the include.*/
-# ifndef _GNU_SOURCE
-#  define _SHOULD_UNDEFINE_GNU_SOURCE
-#  define _GNU_SOURCE
-# endif
+ * use it. */
+
 // libunwind's unwind.h reflects the current visibility.  However, Mozilla
 // builds with -fvisibility=hidden and relies on gcc's unwind.h to reset the
 // visibility to default and export its contents.  gcc also allows users to
@@ -33,10 +29,6 @@
 #  include_next 
 #  pragma GCC visibility pop
 # endif
-# ifdef _SHOULD_UNDEFINE_GNU_SOURCE
-#  undef _GNU_SOURCE
-#  undef _SHOULD_UNDEFINE_GNU_SOURCE
-# endif
 #else
 
 #include 


Index: clang/lib/Headers/unwind.h
===
--- clang/lib/Headers/unwind.h
+++ clang/lib/Headers/unwind.h
@@ -14,12 +14,8 @@
 
 #if defined(__APPLE__) && __has_include_next()
 /* Darwin (from 11.x on) provide an unwind.h. If that's available,
- * use it. libunwind wraps some of its definitions in #ifdef _GNU_SOURCE,
- * so define that around the include.*/
-# ifndef _GNU_SOURCE
-#  define _SHOULD_UNDEFINE_GNU_SOURCE
-#  define _GNU_SOURCE
-# endif
+ * use it. */
+
 // libunwind's unwind.h reflects the current visibility.  However, Mozilla
 // builds with -fvisibility=hidden and relies on gcc's unwind.h to reset the
 // visibility to default and export its contents.  gcc also allows users to
@@ -33,10 +29,6 @@
 #  include_next 
 #  pragma GCC visibility pop
 # endif
-# ifdef _SHOULD_UNDEFINE_GNU_SOURCE
-#  undef _GNU_SOURCE
-#  undef _SHOULD_UNDEFINE_GNU_SOURCE
-# endif
 #else
 
 #include 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112049: [ObjC] avoid crashing when emitting synthesized getter/setter and ptrdiff_t is smaller than long

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson added a comment.

Hello, and sorry for the delay.

The blocking bug was fixed by D112113 .  This 
is ready for review.  Please let me know if you have any feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112049

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

[PATCH] D112049: [ObjC] avoid crashing when emitting synthesized getter/setter and ptrdiff_t is smaller than long

[Matt Jacobson 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 rGdd9f7963e434: [ObjC] avoid crashing when emitting 
synthesized getter/setter and ptrdiff_t is… (authored by mhjacobson).

Changed prior to commit:
  https://reviews.llvm.org/D112049?vs=451085&id=474449#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112049

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/avr/objc-property.m


Index: clang/test/CodeGen/avr/objc-property.m
===
--- /dev/null
+++ clang/test/CodeGen/avr/objc-property.m
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple avr -emit-llvm -fobjc-runtime=macosx %s -o /dev/null
+
+__attribute__((objc_root_class))
+@interface Foo
+
+@property(strong) Foo *f;
+
+@end
+
+@implementation Foo
+
+@synthesize f = _f;
+
+@end
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -3974,6 +3974,8 @@
 
   llvm::Value *EmitIvarOffset(const ObjCInterfaceDecl *Interface,
   const ObjCIvarDecl *Ivar);
+  llvm::Value *EmitIvarOffsetAsPointerDiff(const ObjCInterfaceDecl *Interface,
+   const ObjCIvarDecl *Ivar);
   LValue EmitLValueForField(LValue Base, const FieldDecl* Field);
   LValue EmitLValueForLambdaField(const FieldDecl *Field);
 
Index: clang/lib/CodeGen/CGObjC.cpp
===
--- clang/lib/CodeGen/CGObjC.cpp
+++ clang/lib/CodeGen/CGObjC.cpp
@@ -1228,7 +1228,7 @@
 llvm::Value *cmd = emitCmdValueForGetterSetterBody(*this, getterMethod);
 llvm::Value *self = Builder.CreateBitCast(LoadObjCSelf(), VoidPtrTy);
 llvm::Value *ivarOffset =
-  EmitIvarOffset(classImpl->getClassInterface(), ivar);
+EmitIvarOffsetAsPointerDiff(classImpl->getClassInterface(), ivar);
 
 CallArgList args;
 args.add(RValue::get(self), getContext().getObjCIdType());
@@ -1532,7 +1532,7 @@
 llvm::Value *self =
   Builder.CreateBitCast(LoadObjCSelf(), VoidPtrTy);
 llvm::Value *ivarOffset =
-  EmitIvarOffset(classImpl->getClassInterface(), ivar);
+EmitIvarOffsetAsPointerDiff(classImpl->getClassInterface(), ivar);
 Address argAddr = GetAddrOfLocalVar(*setterMethod->param_begin());
 llvm::Value *arg = Builder.CreateLoad(argAddr, "arg");
 arg = Builder.CreateBitCast(arg, VoidPtrTy);
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -5242,6 +5242,15 @@
   return CGM.getObjCRuntime().EmitIvarOffset(*this, Interface, Ivar);
 }
 
+llvm::Value *
+CodeGenFunction::EmitIvarOffsetAsPointerDiff(const ObjCInterfaceDecl 
*Interface,
+ const ObjCIvarDecl *Ivar) {
+  llvm::Value *OffsetValue = EmitIvarOffset(Interface, Ivar);
+  QualType PointerDiffType = getContext().getPointerDiffType();
+  return Builder.CreateZExtOrTrunc(OffsetValue,
+   getTypes().ConvertType(PointerDiffType));
+}
+
 LValue CodeGenFunction::EmitLValueForIvar(QualType ObjectTy,
   llvm::Value *BaseValue,
   const ObjCIvarDecl *Ivar,


Index: clang/test/CodeGen/avr/objc-property.m
===
--- /dev/null
+++ clang/test/CodeGen/avr/objc-property.m
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple avr -emit-llvm -fobjc-runtime=macosx %s -o /dev/null
+
+__attribute__((objc_root_class))
+@interface Foo
+
+@property(strong) Foo *f;
+
+@end
+
+@implementation Foo
+
+@synthesize f = _f;
+
+@end
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -3974,6 +3974,8 @@
 
   llvm::Value *EmitIvarOffset(const ObjCInterfaceDecl *Interface,
   const ObjCIvarDecl *Ivar);
+  llvm::Value *EmitIvarOffsetAsPointerDiff(const ObjCInterfaceDecl *Interface,
+   const ObjCIvarDecl *Ivar);
   LValue EmitLValueForField(LValue Base, const FieldDecl* Field);
   LValue EmitLValueForLambdaField(const FieldDecl *Field);
 
Index: clang/lib/CodeGen/CGObjC.cpp
===
--- clang/lib/CodeGen/CGObjC.cpp
+++ clang/lib/CodeGen/CGObjC.cpp
@@ -1228,7 +1228,7 @@
 llvm::Value *cmd = emitCmdValueForGetterSetterBody(*this, getterMethod);
 llvm::Value *self = Builder.Cr

[PATCH] D138183: [Driver] move FreeBSD header search path management to the driver

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson created this revision.
mhjacobson added a project: clang.
Herald added subscribers: fedor.sergeev, krytarowski, emaste.
Herald added a project: All.
mhjacobson requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.

This matches OpenBSD, and it supports Swift's use of clang for its C interop 
functionality.  Recent changes to Swift use `AddClangSystemIncludeArgs()` to 
inspect the cc1 args; this doesn't work for platforms where cc1 adds standard 
include paths implicitly.

Also, clean up InitHeaderSearch, making it clearer which targets manage header 
search paths in the driver.

Add test.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138183

Files:
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/FreeBSD.h
  clang/lib/Lex/InitHeaderSearch.cpp

Index: clang/lib/Lex/InitHeaderSearch.cpp
===
--- clang/lib/Lex/InitHeaderSearch.cpp
+++ clang/lib/Lex/InitHeaderSearch.cpp
@@ -100,6 +100,11 @@
const llvm::Triple &triple,
const HeaderSearchOptions &HSOpts);
 
+  /// ShouldAddDefaultIncludePaths - Returns true iff AddDefaultIncludePaths
+  ///  should actually do anything.  If this returns false, include management
+  ///  should instead be handled in the driver.
+  bool ShouldAddDefaultIncludePaths(const llvm::Triple &triple);
+
   /// AddDefaultSystemIncludePaths - Adds the default system include paths so
   ///  that e.g. stdio.h is found.
   void AddDefaultIncludePaths(const LangOptions &Lang,
@@ -225,18 +230,16 @@
 
 void InitHeaderSearch::AddDefaultCIncludePaths(const llvm::Triple &triple,
 const HeaderSearchOptions &HSOpts) {
-  llvm::Triple::OSType os = triple.getOS();
-
-  if (triple.isOSDarwin()) {
+  if (!ShouldAddDefaultIncludePaths(triple)) {
 llvm_unreachable("Include management is handled in the driver.");
   }
 
+  llvm::Triple::OSType os = triple.getOS();
+
   if (HSOpts.UseStandardSystemIncludes) {
 switch (os) {
 case llvm::Triple::CloudABI:
-case llvm::Triple::FreeBSD:
 case llvm::Triple::NetBSD:
-case llvm::Triple::OpenBSD:
 case llvm::Triple::NaCl:
 case llvm::Triple::PS4:
 case llvm::Triple::PS5:
@@ -280,12 +283,6 @@
   }
 
   switch (os) {
-  case llvm::Triple::Linux:
-  case llvm::Triple::Hurd:
-  case llvm::Triple::Solaris:
-  case llvm::Triple::OpenBSD:
-llvm_unreachable("Include management is handled in the driver.");
-
   case llvm::Triple::CloudABI: {
 // //include
 SmallString<128> P = StringRef(HSOpts.ResourceDir);
@@ -386,20 +383,14 @@
 void InitHeaderSearch::AddDefaultCPlusPlusIncludePaths(
 const LangOptions &LangOpts, const llvm::Triple &triple,
 const HeaderSearchOptions &HSOpts) {
-  llvm::Triple::OSType os = triple.getOS();
-  // FIXME: temporary hack: hard-coded paths.
-
-  if (triple.isOSDarwin()) {
+  if (!ShouldAddDefaultIncludePaths(triple)) {
 llvm_unreachable("Include management is handled in the driver.");
   }
 
+  llvm::Triple::OSType os = triple.getOS();
+
+  // FIXME: temporary hack: hard-coded paths.
   switch (os) {
-  case llvm::Triple::Linux:
-  case llvm::Triple::Hurd:
-  case llvm::Triple::Solaris:
-  case llvm::Triple::AIX:
-llvm_unreachable("Include management is handled in the driver.");
-break;
   case llvm::Triple::Win32:
 switch (triple.getEnvironment()) {
 default: llvm_unreachable("Include management is handled in the driver.");
@@ -425,46 +416,55 @@
   }
 }
 
-void InitHeaderSearch::AddDefaultIncludePaths(const LangOptions &Lang,
-  const llvm::Triple &triple,
-const HeaderSearchOptions &HSOpts) {
-  // NB: This code path is going away. All of the logic is moving into the
-  // driver which has the information necessary to do target-specific
-  // selections of default include paths. Each target which moves there will be
-  // exempted from this logic here until we can delete the entire pile of code.
-  switch (triple.getOS()) {
-  default:
-break; // Everything else continues to use this routine's logic.
+bool InitHeaderSearch::ShouldAddDefaultIncludePaths(
+const llvm::Triple &triple) {
+  llvm::Triple::OSType os = triple.getOS();
 
+  switch (triple.getOS()) {
+  case llvm::Triple::AIX:
   case llvm::Triple::Emscripten:
-  case llvm::Triple::Linux:
+  case llvm::Triple::FreeBSD:
   case llvm::Triple::Hurd:
+  case llvm::Triple::Linux:
   case llvm::Triple::OpenBSD:
   case llvm::Triple::Solaris:
   case llvm::Triple::WASI:
-  case llvm::Triple::AIX:
-return;
+return false;
 
   case llvm::Triple::Win32:
 if (triple.getEnvironment() != llvm::Triple::Cygnus ||
 triple.isOSBinFormatMachO())
-  return;
+  return false;
 break;
 
   case llvm::Triple::UnknownOS:
 if (triple.isWasm()

[PATCH] D138183: [Driver] move FreeBSD header search path management to the driver

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson updated this revision to Diff 476009.
mhjacobson added a comment.
Herald added a subscriber: ormris.

Added test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138183

Files:
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/FreeBSD.h
  clang/lib/Lex/InitHeaderSearch.cpp
  clang/test/Driver/freebsd.c

Index: clang/test/Driver/freebsd.c
===
--- clang/test/Driver/freebsd.c
+++ clang/test/Driver/freebsd.c
@@ -213,3 +213,8 @@
 // RELOCATABLE-NOT: "-dynamic-linker"
 // RELOCATABLE-NOT: "-l
 // RELOCATABLE-NOT: crt{{[^./]+}}.o
+
+// Check that the driver passes include paths to cc1 on FreeBSD.
+// RUN: %clang -### %s --target=x86_64-unknown-freebsd13.1 -r 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=DRIVER-PASS-INCLUDES
+// DRIVER-PASS-INCLUDES: "-cc1" {{.*}}"-internal-externc-isystem" "/usr/include"
Index: clang/lib/Lex/InitHeaderSearch.cpp
===
--- clang/lib/Lex/InitHeaderSearch.cpp
+++ clang/lib/Lex/InitHeaderSearch.cpp
@@ -100,6 +100,11 @@
const llvm::Triple &triple,
const HeaderSearchOptions &HSOpts);
 
+  /// ShouldAddDefaultIncludePaths - Returns true iff AddDefaultIncludePaths
+  ///  should actually do anything.  If this returns false, include management
+  ///  should instead be handled in the driver.
+  bool ShouldAddDefaultIncludePaths(const llvm::Triple &triple);
+
   /// AddDefaultSystemIncludePaths - Adds the default system include paths so
   ///  that e.g. stdio.h is found.
   void AddDefaultIncludePaths(const LangOptions &Lang,
@@ -225,18 +230,16 @@
 
 void InitHeaderSearch::AddDefaultCIncludePaths(const llvm::Triple &triple,
 const HeaderSearchOptions &HSOpts) {
-  llvm::Triple::OSType os = triple.getOS();
-
-  if (triple.isOSDarwin()) {
+  if (!ShouldAddDefaultIncludePaths(triple)) {
 llvm_unreachable("Include management is handled in the driver.");
   }
 
+  llvm::Triple::OSType os = triple.getOS();
+
   if (HSOpts.UseStandardSystemIncludes) {
 switch (os) {
 case llvm::Triple::CloudABI:
-case llvm::Triple::FreeBSD:
 case llvm::Triple::NetBSD:
-case llvm::Triple::OpenBSD:
 case llvm::Triple::NaCl:
 case llvm::Triple::PS4:
 case llvm::Triple::PS5:
@@ -280,12 +283,6 @@
   }
 
   switch (os) {
-  case llvm::Triple::Linux:
-  case llvm::Triple::Hurd:
-  case llvm::Triple::Solaris:
-  case llvm::Triple::OpenBSD:
-llvm_unreachable("Include management is handled in the driver.");
-
   case llvm::Triple::CloudABI: {
 // //include
 SmallString<128> P = StringRef(HSOpts.ResourceDir);
@@ -386,20 +383,14 @@
 void InitHeaderSearch::AddDefaultCPlusPlusIncludePaths(
 const LangOptions &LangOpts, const llvm::Triple &triple,
 const HeaderSearchOptions &HSOpts) {
-  llvm::Triple::OSType os = triple.getOS();
-  // FIXME: temporary hack: hard-coded paths.
-
-  if (triple.isOSDarwin()) {
+  if (!ShouldAddDefaultIncludePaths(triple)) {
 llvm_unreachable("Include management is handled in the driver.");
   }
 
+  llvm::Triple::OSType os = triple.getOS();
+
+  // FIXME: temporary hack: hard-coded paths.
   switch (os) {
-  case llvm::Triple::Linux:
-  case llvm::Triple::Hurd:
-  case llvm::Triple::Solaris:
-  case llvm::Triple::AIX:
-llvm_unreachable("Include management is handled in the driver.");
-break;
   case llvm::Triple::Win32:
 switch (triple.getEnvironment()) {
 default: llvm_unreachable("Include management is handled in the driver.");
@@ -425,46 +416,55 @@
   }
 }
 
-void InitHeaderSearch::AddDefaultIncludePaths(const LangOptions &Lang,
-  const llvm::Triple &triple,
-const HeaderSearchOptions &HSOpts) {
-  // NB: This code path is going away. All of the logic is moving into the
-  // driver which has the information necessary to do target-specific
-  // selections of default include paths. Each target which moves there will be
-  // exempted from this logic here until we can delete the entire pile of code.
-  switch (triple.getOS()) {
-  default:
-break; // Everything else continues to use this routine's logic.
+bool InitHeaderSearch::ShouldAddDefaultIncludePaths(
+const llvm::Triple &triple) {
+  llvm::Triple::OSType os = triple.getOS();
 
+  switch (triple.getOS()) {
+  case llvm::Triple::AIX:
   case llvm::Triple::Emscripten:
-  case llvm::Triple::Linux:
+  case llvm::Triple::FreeBSD:
   case llvm::Triple::Hurd:
+  case llvm::Triple::Linux:
   case llvm::Triple::OpenBSD:
   case llvm::Triple::Solaris:
   case llvm::Triple::WASI:
-  case llvm::Triple::AIX:
-return;
+return false;
 
   case llvm::Triple::Win32:
 if (triple.getEnvironment() != llvm::Triple:

[PATCH] D138183: [Driver] move FreeBSD header search path management to the driver

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson updated this revision to Diff 476017.
mhjacobson added a comment.

Run clang-format.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138183

Files:
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/FreeBSD.h
  clang/lib/Lex/InitHeaderSearch.cpp
  clang/test/Driver/freebsd.c

Index: clang/test/Driver/freebsd.c
===
--- clang/test/Driver/freebsd.c
+++ clang/test/Driver/freebsd.c
@@ -213,3 +213,8 @@
 // RELOCATABLE-NOT: "-dynamic-linker"
 // RELOCATABLE-NOT: "-l
 // RELOCATABLE-NOT: crt{{[^./]+}}.o
+
+// Check that the driver passes include paths to cc1 on FreeBSD.
+// RUN: %clang -### %s --target=x86_64-unknown-freebsd13.1 -r 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=DRIVER-PASS-INCLUDES
+// DRIVER-PASS-INCLUDES: "-cc1" {{.*}}"-internal-externc-isystem" "/usr/include"
Index: clang/lib/Lex/InitHeaderSearch.cpp
===
--- clang/lib/Lex/InitHeaderSearch.cpp
+++ clang/lib/Lex/InitHeaderSearch.cpp
@@ -100,6 +100,11 @@
const llvm::Triple &triple,
const HeaderSearchOptions &HSOpts);
 
+  /// ShouldAddDefaultIncludePaths - Returns true iff AddDefaultIncludePaths
+  ///  should actually do anything.  If this returns false, include management
+  ///  should instead be handled in the driver.
+  bool ShouldAddDefaultIncludePaths(const llvm::Triple &triple);
+
   /// AddDefaultSystemIncludePaths - Adds the default system include paths so
   ///  that e.g. stdio.h is found.
   void AddDefaultIncludePaths(const LangOptions &Lang,
@@ -225,18 +230,16 @@
 
 void InitHeaderSearch::AddDefaultCIncludePaths(const llvm::Triple &triple,
 const HeaderSearchOptions &HSOpts) {
-  llvm::Triple::OSType os = triple.getOS();
-
-  if (triple.isOSDarwin()) {
+  if (!ShouldAddDefaultIncludePaths(triple)) {
 llvm_unreachable("Include management is handled in the driver.");
   }
 
+  llvm::Triple::OSType os = triple.getOS();
+
   if (HSOpts.UseStandardSystemIncludes) {
 switch (os) {
 case llvm::Triple::CloudABI:
-case llvm::Triple::FreeBSD:
 case llvm::Triple::NetBSD:
-case llvm::Triple::OpenBSD:
 case llvm::Triple::NaCl:
 case llvm::Triple::PS4:
 case llvm::Triple::PS5:
@@ -280,12 +283,6 @@
   }
 
   switch (os) {
-  case llvm::Triple::Linux:
-  case llvm::Triple::Hurd:
-  case llvm::Triple::Solaris:
-  case llvm::Triple::OpenBSD:
-llvm_unreachable("Include management is handled in the driver.");
-
   case llvm::Triple::CloudABI: {
 // //include
 SmallString<128> P = StringRef(HSOpts.ResourceDir);
@@ -386,20 +383,14 @@
 void InitHeaderSearch::AddDefaultCPlusPlusIncludePaths(
 const LangOptions &LangOpts, const llvm::Triple &triple,
 const HeaderSearchOptions &HSOpts) {
-  llvm::Triple::OSType os = triple.getOS();
-  // FIXME: temporary hack: hard-coded paths.
-
-  if (triple.isOSDarwin()) {
+  if (!ShouldAddDefaultIncludePaths(triple)) {
 llvm_unreachable("Include management is handled in the driver.");
   }
 
+  llvm::Triple::OSType os = triple.getOS();
+
+  // FIXME: temporary hack: hard-coded paths.
   switch (os) {
-  case llvm::Triple::Linux:
-  case llvm::Triple::Hurd:
-  case llvm::Triple::Solaris:
-  case llvm::Triple::AIX:
-llvm_unreachable("Include management is handled in the driver.");
-break;
   case llvm::Triple::Win32:
 switch (triple.getEnvironment()) {
 default: llvm_unreachable("Include management is handled in the driver.");
@@ -425,46 +416,55 @@
   }
 }
 
-void InitHeaderSearch::AddDefaultIncludePaths(const LangOptions &Lang,
-  const llvm::Triple &triple,
-const HeaderSearchOptions &HSOpts) {
-  // NB: This code path is going away. All of the logic is moving into the
-  // driver which has the information necessary to do target-specific
-  // selections of default include paths. Each target which moves there will be
-  // exempted from this logic here until we can delete the entire pile of code.
-  switch (triple.getOS()) {
-  default:
-break; // Everything else continues to use this routine's logic.
+bool InitHeaderSearch::ShouldAddDefaultIncludePaths(
+const llvm::Triple &triple) {
+  llvm::Triple::OSType os = triple.getOS();
 
+  switch (triple.getOS()) {
+  case llvm::Triple::AIX:
   case llvm::Triple::Emscripten:
-  case llvm::Triple::Linux:
+  case llvm::Triple::FreeBSD:
   case llvm::Triple::Hurd:
+  case llvm::Triple::Linux:
   case llvm::Triple::OpenBSD:
   case llvm::Triple::Solaris:
   case llvm::Triple::WASI:
-  case llvm::Triple::AIX:
-return;
+return false;
 
   case llvm::Triple::Win32:
 if (triple.getEnvironment() != llvm::Triple::Cygnus ||
 triple.is

[PATCH] D138183: [Driver] move FreeBSD header search path management to the driver

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson updated this revision to Diff 476018.
mhjacobson added a comment.

Clarify comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138183

Files:
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/FreeBSD.h
  clang/lib/Lex/InitHeaderSearch.cpp
  clang/test/Driver/freebsd.c

Index: clang/test/Driver/freebsd.c
===
--- clang/test/Driver/freebsd.c
+++ clang/test/Driver/freebsd.c
@@ -213,3 +213,8 @@
 // RELOCATABLE-NOT: "-dynamic-linker"
 // RELOCATABLE-NOT: "-l
 // RELOCATABLE-NOT: crt{{[^./]+}}.o
+
+// Check that the driver passes include paths to cc1 on FreeBSD.
+// RUN: %clang -### %s --target=x86_64-unknown-freebsd13.1 -r 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=DRIVER-PASS-INCLUDES
+// DRIVER-PASS-INCLUDES: "-cc1" {{.*}}"-internal-externc-isystem" "/usr/include"
Index: clang/lib/Lex/InitHeaderSearch.cpp
===
--- clang/lib/Lex/InitHeaderSearch.cpp
+++ clang/lib/Lex/InitHeaderSearch.cpp
@@ -100,6 +100,11 @@
const llvm::Triple &triple,
const HeaderSearchOptions &HSOpts);
 
+  /// ShouldAddDefaultIncludePaths - Returns true iff AddDefaultIncludePaths
+  ///  should actually do anything.  If this returns false, include management
+  ///  should instead be handled in the driver.
+  bool ShouldAddDefaultIncludePaths(const llvm::Triple &triple);
+
   /// AddDefaultSystemIncludePaths - Adds the default system include paths so
   ///  that e.g. stdio.h is found.
   void AddDefaultIncludePaths(const LangOptions &Lang,
@@ -225,18 +230,16 @@
 
 void InitHeaderSearch::AddDefaultCIncludePaths(const llvm::Triple &triple,
 const HeaderSearchOptions &HSOpts) {
-  llvm::Triple::OSType os = triple.getOS();
-
-  if (triple.isOSDarwin()) {
+  if (!ShouldAddDefaultIncludePaths(triple)) {
 llvm_unreachable("Include management is handled in the driver.");
   }
 
+  llvm::Triple::OSType os = triple.getOS();
+
   if (HSOpts.UseStandardSystemIncludes) {
 switch (os) {
 case llvm::Triple::CloudABI:
-case llvm::Triple::FreeBSD:
 case llvm::Triple::NetBSD:
-case llvm::Triple::OpenBSD:
 case llvm::Triple::NaCl:
 case llvm::Triple::PS4:
 case llvm::Triple::PS5:
@@ -280,12 +283,6 @@
   }
 
   switch (os) {
-  case llvm::Triple::Linux:
-  case llvm::Triple::Hurd:
-  case llvm::Triple::Solaris:
-  case llvm::Triple::OpenBSD:
-llvm_unreachable("Include management is handled in the driver.");
-
   case llvm::Triple::CloudABI: {
 // //include
 SmallString<128> P = StringRef(HSOpts.ResourceDir);
@@ -386,20 +383,14 @@
 void InitHeaderSearch::AddDefaultCPlusPlusIncludePaths(
 const LangOptions &LangOpts, const llvm::Triple &triple,
 const HeaderSearchOptions &HSOpts) {
-  llvm::Triple::OSType os = triple.getOS();
-  // FIXME: temporary hack: hard-coded paths.
-
-  if (triple.isOSDarwin()) {
+  if (!ShouldAddDefaultIncludePaths(triple)) {
 llvm_unreachable("Include management is handled in the driver.");
   }
 
+  llvm::Triple::OSType os = triple.getOS();
+
+  // FIXME: temporary hack: hard-coded paths.
   switch (os) {
-  case llvm::Triple::Linux:
-  case llvm::Triple::Hurd:
-  case llvm::Triple::Solaris:
-  case llvm::Triple::AIX:
-llvm_unreachable("Include management is handled in the driver.");
-break;
   case llvm::Triple::Win32:
 switch (triple.getEnvironment()) {
 default: llvm_unreachable("Include management is handled in the driver.");
@@ -425,46 +416,55 @@
   }
 }
 
-void InitHeaderSearch::AddDefaultIncludePaths(const LangOptions &Lang,
-  const llvm::Triple &triple,
-const HeaderSearchOptions &HSOpts) {
-  // NB: This code path is going away. All of the logic is moving into the
-  // driver which has the information necessary to do target-specific
-  // selections of default include paths. Each target which moves there will be
-  // exempted from this logic here until we can delete the entire pile of code.
-  switch (triple.getOS()) {
-  default:
-break; // Everything else continues to use this routine's logic.
+bool InitHeaderSearch::ShouldAddDefaultIncludePaths(
+const llvm::Triple &triple) {
+  llvm::Triple::OSType os = triple.getOS();
 
+  switch (triple.getOS()) {
+  case llvm::Triple::AIX:
   case llvm::Triple::Emscripten:
-  case llvm::Triple::Linux:
+  case llvm::Triple::FreeBSD:
   case llvm::Triple::Hurd:
+  case llvm::Triple::Linux:
   case llvm::Triple::OpenBSD:
   case llvm::Triple::Solaris:
   case llvm::Triple::WASI:
-  case llvm::Triple::AIX:
-return;
+return false;
 
   case llvm::Triple::Win32:
 if (triple.getEnvironment() != llvm::Triple::Cygnus ||
 triple.isO

[PATCH] D138183: [Driver] move FreeBSD header search path management to the driver

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson added inline comments.



Comment at: clang/lib/Driver/ToolChains/FreeBSD.cpp:442
+
+  addExternCSystemInclude(DriverArgs, CC1Args,
+  concat(D.SysRoot, "/usr/include"));

MaskRay wrote:
> I think Fuchsia way of checking `if (!D.SysRoot.empty()) {` before adding 
> `/usr/include` is probably better. Is it /include or /usr/include ?
It's `/usr/include` on FreeBSD.

I'm confused by the Fuchsia code—does it not add `/include` if the sysroot is 
empty?  Appears not...

```
$ clang-15 -### -c -target arm64-fuchsia -xc /dev/null 
clang version 15.0.3
Target: arm64-unknown-fuchsia
Thread model: posix
InstalledDir: /opt/local/libexec/llvm-15/bin
 (in-process)
 "/opt/local/libexec/llvm-15/bin/clang" "-cc1" "-triple" 
"arm64-unknown-fuchsia" "-emit-obj" "-mrelax-all" "--mrelax-relocations" 
"-disable-free" "-clear-ast-before-backend" "-disable-llvm-verifier" 
"-discard-value-names" "-main-file-name" "null" "-mrelocation-model" "pic" 
"-pic-level" "2" "-pic-is-pie" "-mframe-pointer=non-leaf" "-ffp-contract=on" 
"-fno-rounding-math" "-mconstructor-aliases" "-funwind-tables=2" "-target-cpu" 
"generic" "-target-feature" "+neon" "-target-feature" "+v8a" "-target-abi" 
"aapcs" "-fallow-half-arguments-and-returns" "-mllvm" 
"-treat-scalable-fixed-error-as-warning" "-debugger-tuning=gdb" 
"-target-linker-version" "556.6" "-fcoverage-compilation-dir=/Users/matt" 
"-resource-dir" "/opt/local/libexec/llvm-15/lib/clang/15.0.3" 
"-I/usr/local/include" "-internal-isystem" 
"/opt/local/libexec/llvm-15/lib/clang/15.0.3/include" 
"-fdebug-compilation-dir=/Users/matt" "-ferror-limit" "19" 
"-fsanitize=shadow-call-stack" "-stack-protector" "2" "-fno-signed-char" 
"-fgnuc-version=4.2.1" "-fcolor-diagnostics" "-target-feature" 
"+outline-atomics" "-faddrsig" "-D__GCC_HAVE_DWARF2_CFI_ASM=1" "-o" "null.o" 
"-x" "c" "/dev/null"

$ clang-15 -### -c -target arm64-fuchsia --sysroot /foo -xc /dev/null
clang version 15.0.3
Target: arm64-unknown-fuchsia
Thread model: posix
InstalledDir: /opt/local/libexec/llvm-15/bin
 (in-process)
 "/opt/local/libexec/llvm-15/bin/clang" "-cc1" "-triple" 
"arm64-unknown-fuchsia" "-emit-obj" "-mrelax-all" "--mrelax-relocations" 
"-disable-free" "-clear-ast-before-backend" "-disable-llvm-verifier" 
"-discard-value-names" "-main-file-name" "null" "-mrelocation-model" "pic" 
"-pic-level" "2" "-pic-is-pie" "-mframe-pointer=non-leaf" "-ffp-contract=on" 
"-fno-rounding-math" "-mconstructor-aliases" "-funwind-tables=2" "-target-cpu" 
"generic" "-target-feature" "+neon" "-target-feature" "+v8a" "-target-abi" 
"aapcs" "-fallow-half-arguments-and-returns" "-mllvm" 
"-treat-scalable-fixed-error-as-warning" "-debugger-tuning=gdb" 
"-target-linker-version" "556.6" "-fcoverage-compilation-dir=/Users/matt" 
"-resource-dir" "/opt/local/libexec/llvm-15/lib/clang/15.0.3" "-isysroot" 
"/foo" "-internal-isystem" 
"/opt/local/libexec/llvm-15/lib/clang/15.0.3/include" 
"-internal-externc-isystem" "/foo/include" 
"-fdebug-compilation-dir=/Users/matt" "-ferror-limit" "19" 
"-fsanitize=shadow-call-stack" "-stack-protector" "2" "-fno-signed-char" 
"-fgnuc-version=4.2.1" "-fcolor-diagnostics" "-target-feature" 
"+outline-atomics" "-faddrsig" "-D__GCC_HAVE_DWARF2_CFI_ASM=1" "-o" "null.o" 
"-x" "c" "/dev/null"
```

I don't know anything about Fuchsia, but isn't that wrong?  Certainly for 
FreeBSD we'd want to add an unprefixed `/usr/include` normally...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138183

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

[PATCH] D138183: [Driver] move FreeBSD header search path management to the driver

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson added inline comments.



Comment at: clang/test/Driver/freebsd.c:217
+
+// Check that the driver passes include paths to cc1 on FreeBSD.
+// RUN: %clang -### %s --target=x86_64-unknown-freebsd13.1 -r 2>&1 \

MaskRay wrote:
> Is this  sufficient? For Linux `linux-cross.cpp` checks a lot of directories 
> to facilitate code refactoring.
On FreeBSD, the only directories included by default are

  - /usr/include/c++/v1 (in C++ mode)
  - /include
  - /usr/include

I suppose I can add checks for the first two.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138183

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

[PATCH] D138183: [Driver] move FreeBSD header search path management to the driver

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson updated this revision to Diff 476031.
mhjacobson added a comment.

Test for more paths; test C++.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138183

Files:
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/FreeBSD.h
  clang/lib/Lex/InitHeaderSearch.cpp
  clang/test/Driver/freebsd.c
  clang/test/Driver/freebsd.cpp

Index: clang/test/Driver/freebsd.cpp
===
--- clang/test/Driver/freebsd.cpp
+++ clang/test/Driver/freebsd.cpp
@@ -40,3 +40,11 @@
 // CHECK-LIBCXX-SYSROOT-SLASH: "-cc1"
 // CHECK-LIBCXX-SYSROOT-SLASH-SAME: "-isysroot" "[[SYSROOT:[^"]+/]]"
 // CHECK-LIBCXX-SYSROOT-SLASH-SAME: "-internal-isystem" "[[SYSROOT]]usr/include/c++/v1"
+
+// Check that the driver passes include paths to cc1 on FreeBSD.
+// RUN: %clang -### %s --target=x86_64-unknown-freebsd13.1 -r 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=DRIVER-PASS-INCLUDES
+// DRIVER-PASS-INCLUDES: "-cc1" {{.*}}"-resource-dir" "[[RESOURCE:[^"]+]]"
+// DRIVER-PASS-INCLUDES: "-internal-isystem" "/usr/include/c++/v1"
+// DRIVER-PASS-INCLUDES: "-internal-isystem" "[[RESOURCE]]/include"
+// DRIVER-PASS-INCLUDES: "-internal-externc-isystem" "/usr/include"
Index: clang/test/Driver/freebsd.c
===
--- clang/test/Driver/freebsd.c
+++ clang/test/Driver/freebsd.c
@@ -213,3 +213,10 @@
 // RELOCATABLE-NOT: "-dynamic-linker"
 // RELOCATABLE-NOT: "-l
 // RELOCATABLE-NOT: crt{{[^./]+}}.o
+
+// Check that the driver passes include paths to cc1 on FreeBSD.
+// RUN: %clang -### %s --target=x86_64-unknown-freebsd13.1 -r 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=DRIVER-PASS-INCLUDES
+// DRIVER-PASS-INCLUDES: "-cc1" {{.*}}"-resource-dir" "[[RESOURCE:[^"]+]]"
+// DRIVER-PASS-INCLUDES: "-internal-isystem" "[[RESOURCE]]/include"
+// DRIVER-PASS-INCLUDES: "-internal-externc-isystem" "/usr/include"
Index: clang/lib/Lex/InitHeaderSearch.cpp
===
--- clang/lib/Lex/InitHeaderSearch.cpp
+++ clang/lib/Lex/InitHeaderSearch.cpp
@@ -100,6 +100,11 @@
const llvm::Triple &triple,
const HeaderSearchOptions &HSOpts);
 
+  /// ShouldAddDefaultIncludePaths - Returns true iff AddDefaultIncludePaths
+  ///  should actually do anything.  If this returns false, include management
+  ///  should instead be handled in the driver.
+  bool ShouldAddDefaultIncludePaths(const llvm::Triple &triple);
+
   /// AddDefaultSystemIncludePaths - Adds the default system include paths so
   ///  that e.g. stdio.h is found.
   void AddDefaultIncludePaths(const LangOptions &Lang,
@@ -225,18 +230,16 @@
 
 void InitHeaderSearch::AddDefaultCIncludePaths(const llvm::Triple &triple,
 const HeaderSearchOptions &HSOpts) {
-  llvm::Triple::OSType os = triple.getOS();
-
-  if (triple.isOSDarwin()) {
+  if (!ShouldAddDefaultIncludePaths(triple)) {
 llvm_unreachable("Include management is handled in the driver.");
   }
 
+  llvm::Triple::OSType os = triple.getOS();
+
   if (HSOpts.UseStandardSystemIncludes) {
 switch (os) {
 case llvm::Triple::CloudABI:
-case llvm::Triple::FreeBSD:
 case llvm::Triple::NetBSD:
-case llvm::Triple::OpenBSD:
 case llvm::Triple::NaCl:
 case llvm::Triple::PS4:
 case llvm::Triple::PS5:
@@ -280,12 +283,6 @@
   }
 
   switch (os) {
-  case llvm::Triple::Linux:
-  case llvm::Triple::Hurd:
-  case llvm::Triple::Solaris:
-  case llvm::Triple::OpenBSD:
-llvm_unreachable("Include management is handled in the driver.");
-
   case llvm::Triple::CloudABI: {
 // //include
 SmallString<128> P = StringRef(HSOpts.ResourceDir);
@@ -386,20 +383,14 @@
 void InitHeaderSearch::AddDefaultCPlusPlusIncludePaths(
 const LangOptions &LangOpts, const llvm::Triple &triple,
 const HeaderSearchOptions &HSOpts) {
-  llvm::Triple::OSType os = triple.getOS();
-  // FIXME: temporary hack: hard-coded paths.
-
-  if (triple.isOSDarwin()) {
+  if (!ShouldAddDefaultIncludePaths(triple)) {
 llvm_unreachable("Include management is handled in the driver.");
   }
 
+  llvm::Triple::OSType os = triple.getOS();
+
+  // FIXME: temporary hack: hard-coded paths.
   switch (os) {
-  case llvm::Triple::Linux:
-  case llvm::Triple::Hurd:
-  case llvm::Triple::Solaris:
-  case llvm::Triple::AIX:
-llvm_unreachable("Include management is handled in the driver.");
-break;
   case llvm::Triple::Win32:
 switch (triple.getEnvironment()) {
 default: llvm_unreachable("Include management is handled in the driver.");
@@ -425,46 +416,55 @@
   }
 }
 
-void InitHeaderSearch::AddDefaultIncludePaths(const LangOptions &Lang,
-  const llvm::Triple &triple,
-const He

[PATCH] D138183: [Driver] move FreeBSD header search path management to the driver

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson updated this revision to Diff 476042.
mhjacobson added a comment.

Fix a couple warnings.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138183

Files:
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/FreeBSD.h
  clang/lib/Lex/InitHeaderSearch.cpp
  clang/test/Driver/freebsd.c
  clang/test/Driver/freebsd.cpp

Index: clang/test/Driver/freebsd.cpp
===
--- clang/test/Driver/freebsd.cpp
+++ clang/test/Driver/freebsd.cpp
@@ -40,3 +40,11 @@
 // CHECK-LIBCXX-SYSROOT-SLASH: "-cc1"
 // CHECK-LIBCXX-SYSROOT-SLASH-SAME: "-isysroot" "[[SYSROOT:[^"]+/]]"
 // CHECK-LIBCXX-SYSROOT-SLASH-SAME: "-internal-isystem" "[[SYSROOT]]usr/include/c++/v1"
+
+// Check that the driver passes include paths to cc1 on FreeBSD.
+// RUN: %clang -### %s --target=x86_64-unknown-freebsd13.1 -r 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=DRIVER-PASS-INCLUDES
+// DRIVER-PASS-INCLUDES: "-cc1" {{.*}}"-resource-dir" "[[RESOURCE:[^"]+]]"
+// DRIVER-PASS-INCLUDES: "-internal-isystem" "/usr/include/c++/v1"
+// DRIVER-PASS-INCLUDES: "-internal-isystem" "[[RESOURCE]]/include"
+// DRIVER-PASS-INCLUDES: "-internal-externc-isystem" "/usr/include"
Index: clang/test/Driver/freebsd.c
===
--- clang/test/Driver/freebsd.c
+++ clang/test/Driver/freebsd.c
@@ -213,3 +213,10 @@
 // RELOCATABLE-NOT: "-dynamic-linker"
 // RELOCATABLE-NOT: "-l
 // RELOCATABLE-NOT: crt{{[^./]+}}.o
+
+// Check that the driver passes include paths to cc1 on FreeBSD.
+// RUN: %clang -### %s --target=x86_64-unknown-freebsd13.1 -r 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=DRIVER-PASS-INCLUDES
+// DRIVER-PASS-INCLUDES: "-cc1" {{.*}}"-resource-dir" "[[RESOURCE:[^"]+]]"
+// DRIVER-PASS-INCLUDES: "-internal-isystem" "[[RESOURCE]]/include"
+// DRIVER-PASS-INCLUDES: "-internal-externc-isystem" "/usr/include"
Index: clang/lib/Lex/InitHeaderSearch.cpp
===
--- clang/lib/Lex/InitHeaderSearch.cpp
+++ clang/lib/Lex/InitHeaderSearch.cpp
@@ -100,6 +100,11 @@
const llvm::Triple &triple,
const HeaderSearchOptions &HSOpts);
 
+  /// ShouldAddDefaultIncludePaths - Returns true iff AddDefaultIncludePaths
+  ///  should actually do anything.  If this returns false, include management
+  ///  should instead be handled in the driver.
+  bool ShouldAddDefaultIncludePaths(const llvm::Triple &triple);
+
   /// AddDefaultSystemIncludePaths - Adds the default system include paths so
   ///  that e.g. stdio.h is found.
   void AddDefaultIncludePaths(const LangOptions &Lang,
@@ -225,18 +230,16 @@
 
 void InitHeaderSearch::AddDefaultCIncludePaths(const llvm::Triple &triple,
 const HeaderSearchOptions &HSOpts) {
-  llvm::Triple::OSType os = triple.getOS();
-
-  if (triple.isOSDarwin()) {
+  if (!ShouldAddDefaultIncludePaths(triple)) {
 llvm_unreachable("Include management is handled in the driver.");
   }
 
+  llvm::Triple::OSType os = triple.getOS();
+
   if (HSOpts.UseStandardSystemIncludes) {
 switch (os) {
 case llvm::Triple::CloudABI:
-case llvm::Triple::FreeBSD:
 case llvm::Triple::NetBSD:
-case llvm::Triple::OpenBSD:
 case llvm::Triple::NaCl:
 case llvm::Triple::PS4:
 case llvm::Triple::PS5:
@@ -280,12 +283,6 @@
   }
 
   switch (os) {
-  case llvm::Triple::Linux:
-  case llvm::Triple::Hurd:
-  case llvm::Triple::Solaris:
-  case llvm::Triple::OpenBSD:
-llvm_unreachable("Include management is handled in the driver.");
-
   case llvm::Triple::CloudABI: {
 // //include
 SmallString<128> P = StringRef(HSOpts.ResourceDir);
@@ -386,20 +383,14 @@
 void InitHeaderSearch::AddDefaultCPlusPlusIncludePaths(
 const LangOptions &LangOpts, const llvm::Triple &triple,
 const HeaderSearchOptions &HSOpts) {
-  llvm::Triple::OSType os = triple.getOS();
-  // FIXME: temporary hack: hard-coded paths.
-
-  if (triple.isOSDarwin()) {
+  if (!ShouldAddDefaultIncludePaths(triple)) {
 llvm_unreachable("Include management is handled in the driver.");
   }
 
+  llvm::Triple::OSType os = triple.getOS();
+
+  // FIXME: temporary hack: hard-coded paths.
   switch (os) {
-  case llvm::Triple::Linux:
-  case llvm::Triple::Hurd:
-  case llvm::Triple::Solaris:
-  case llvm::Triple::AIX:
-llvm_unreachable("Include management is handled in the driver.");
-break;
   case llvm::Triple::Win32:
 switch (triple.getEnvironment()) {
 default: llvm_unreachable("Include management is handled in the driver.");
@@ -425,46 +416,56 @@
   }
 }
 
-void InitHeaderSearch::AddDefaultIncludePaths(const LangOptions &Lang,
-  const llvm::Triple &triple,
-const HeaderSear

[PATCH] D138183: [Driver] move FreeBSD header search path management to the driver

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson updated this revision to Diff 476320.
mhjacobson added a comment.
Herald added a subscriber: mstorsjo.

Style fixes.

Ensure include paths are sequential.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138183

Files:
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/FreeBSD.h
  clang/lib/Lex/InitHeaderSearch.cpp
  clang/test/Driver/freebsd.c
  clang/test/Driver/freebsd.cpp

Index: clang/test/Driver/freebsd.cpp
===
--- clang/test/Driver/freebsd.cpp
+++ clang/test/Driver/freebsd.cpp
@@ -40,3 +40,11 @@
 // CHECK-LIBCXX-SYSROOT-SLASH: "-cc1"
 // CHECK-LIBCXX-SYSROOT-SLASH-SAME: "-isysroot" "[[SYSROOT:[^"]+/]]"
 // CHECK-LIBCXX-SYSROOT-SLASH-SAME: "-internal-isystem" "[[SYSROOT]]usr/include/c++/v1"
+
+// Check that the driver passes include paths to cc1 on FreeBSD.
+// RUN: %clang -### %s --target=x86_64-unknown-freebsd13.1 -r 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=DRIVER-PASS-INCLUDES
+// DRIVER-PASS-INCLUDES:  "-cc1" {{.*}}"-resource-dir" "[[RESOURCE:[^"]+]]"
+// DRIVER-PASS-INCLUDES-SAME: {{^}} "-internal-isystem" "/usr/include/c++/v1"
+// DRIVER-PASS-INCLUDES-SAME: {{^}} "-internal-isystem" "[[RESOURCE]]/include"
+// DRIVER-PASS-INCLUDES-SAME: {{^}} "-internal-externc-isystem" "/usr/include"
Index: clang/test/Driver/freebsd.c
===
--- clang/test/Driver/freebsd.c
+++ clang/test/Driver/freebsd.c
@@ -213,3 +213,10 @@
 // RELOCATABLE-NOT: "-dynamic-linker"
 // RELOCATABLE-NOT: "-l
 // RELOCATABLE-NOT: crt{{[^./]+}}.o
+
+// Check that the driver passes include paths to cc1 on FreeBSD.
+// RUN: %clang -### %s --target=x86_64-unknown-freebsd13.1 -r 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=DRIVER-PASS-INCLUDES
+// DRIVER-PASS-INCLUDES:  "-cc1" {{.*}}"-resource-dir" "[[RESOURCE:[^"]+]]"
+// DRIVER-PASS-INCLUDES-SAME: {{^}} "-internal-isystem" "[[RESOURCE]]/include"
+// DRIVER-PASS-INCLUDES-SAME: {{^}} "-internal-externc-isystem" "/usr/include"
Index: clang/lib/Lex/InitHeaderSearch.cpp
===
--- clang/lib/Lex/InitHeaderSearch.cpp
+++ clang/lib/Lex/InitHeaderSearch.cpp
@@ -42,9 +42,9 @@
   : Group(Group), Lookup(Lookup), UserEntryIdx(UserEntryIdx) {}
 };
 
-/// InitHeaderSearch - This class makes it easier to set the search paths of
-///  a HeaderSearch object. InitHeaderSearch stores several search path lists
-///  internally, which can be sent to a HeaderSearch object in one swoop.
+/// This class makes it easier to set the search paths of a HeaderSearch object.
+/// InitHeaderSearch stores several search path lists internally, which can be
+/// sent to a HeaderSearch object in one swoop.
 class InitHeaderSearch {
   std::vector IncludePath;
   std::vector > SystemHeaderPrefixes;
@@ -58,56 +58,54 @@
   : Headers(HS), Verbose(verbose), IncludeSysroot(std::string(sysroot)),
 HasSysroot(!(sysroot.empty() || sysroot == "/")) {}
 
-  /// AddPath - Add the specified path to the specified group list, prefixing
-  /// the sysroot if used.
+  /// Add the specified path to the specified group list, prefixing the sysroot
+  /// if used.
   /// Returns true if the path exists, false if it was ignored.
   bool AddPath(const Twine &Path, IncludeDirGroup Group, bool isFramework,
Optional UserEntryIdx = None);
 
-  /// AddUnmappedPath - Add the specified path to the specified group list,
-  /// without performing any sysroot remapping.
+  /// Add the specified path to the specified group list, without performing any
+  /// sysroot remapping.
   /// Returns true if the path exists, false if it was ignored.
   bool AddUnmappedPath(const Twine &Path, IncludeDirGroup Group,
bool isFramework,
Optional UserEntryIdx = None);
 
-  /// AddSystemHeaderPrefix - Add the specified prefix to the system header
-  /// prefix list.
+  /// Add the specified prefix to the system header prefix list.
   void AddSystemHeaderPrefix(StringRef Prefix, bool IsSystemHeader) {
 SystemHeaderPrefixes.emplace_back(std::string(Prefix), IsSystemHeader);
   }
 
-  /// AddGnuCPlusPlusIncludePaths - Add the necessary paths to support a gnu
-  ///  libstdc++.
+  /// Add the necessary paths to support a gnu libstdc++.
   /// Returns true if the \p Base path was found, false if it does not exist.
   bool AddGnuCPlusPlusIncludePaths(StringRef Base, StringRef ArchDir,
StringRef Dir32, StringRef Dir64,
const llvm::Triple &triple);
 
-  /// AddMinGWCPlusPlusIncludePaths - Add the necessary paths to support a MinGW
-  ///  libstdc++.
+  /// Add the necessary paths to support a MinGW libstdc++.
   void AddMinGWCPlusPlusIncludePaths(StringRef Base,
  StringRef A

[PATCH] D138183: [Driver] move FreeBSD header search path management to the driver

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson updated this revision to Diff 476336.
mhjacobson added a comment.

Make the tests play nice with Windows path separators.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138183

Files:
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/FreeBSD.h
  clang/lib/Lex/InitHeaderSearch.cpp
  clang/test/Driver/freebsd.c
  clang/test/Driver/freebsd.cpp

Index: clang/test/Driver/freebsd.cpp
===
--- clang/test/Driver/freebsd.cpp
+++ clang/test/Driver/freebsd.cpp
@@ -40,3 +40,11 @@
 // CHECK-LIBCXX-SYSROOT-SLASH: "-cc1"
 // CHECK-LIBCXX-SYSROOT-SLASH-SAME: "-isysroot" "[[SYSROOT:[^"]+/]]"
 // CHECK-LIBCXX-SYSROOT-SLASH-SAME: "-internal-isystem" "[[SYSROOT]]usr/include/c++/v1"
+
+// Check that the driver passes include paths to cc1 on FreeBSD.
+// RUN: %clang -### %s --target=x86_64-unknown-freebsd13.1 -r 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=DRIVER-PASS-INCLUDES
+// DRIVER-PASS-INCLUDES:  "-cc1" {{.*}}"-resource-dir" "[[RESOURCE:[^"]+]]"
+// DRIVER-PASS-INCLUDES-SAME: {{^}} "-internal-isystem" "/usr/include/c++/v1"
+// DRIVER-PASS-INCLUDES-SAME: {{^}} "-internal-isystem" "[[RESOURCE]]{{/|}}include"
+// DRIVER-PASS-INCLUDES-SAME: {{^}} "-internal-externc-isystem" "/usr/include"
Index: clang/test/Driver/freebsd.c
===
--- clang/test/Driver/freebsd.c
+++ clang/test/Driver/freebsd.c
@@ -213,3 +213,10 @@
 // RELOCATABLE-NOT: "-dynamic-linker"
 // RELOCATABLE-NOT: "-l
 // RELOCATABLE-NOT: crt{{[^./]+}}.o
+
+// Check that the driver passes include paths to cc1 on FreeBSD.
+// RUN: %clang -### %s --target=x86_64-unknown-freebsd13.1 -r 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=DRIVER-PASS-INCLUDES
+// DRIVER-PASS-INCLUDES:  "-cc1" {{.*}}"-resource-dir" "[[RESOURCE:[^"]+]]"
+// DRIVER-PASS-INCLUDES-SAME: {{^}} "-internal-isystem" "[[RESOURCE]]{{/|}}include"
+// DRIVER-PASS-INCLUDES-SAME: {{^}} "-internal-externc-isystem" "/usr/include"
Index: clang/lib/Lex/InitHeaderSearch.cpp
===
--- clang/lib/Lex/InitHeaderSearch.cpp
+++ clang/lib/Lex/InitHeaderSearch.cpp
@@ -42,9 +42,9 @@
   : Group(Group), Lookup(Lookup), UserEntryIdx(UserEntryIdx) {}
 };
 
-/// InitHeaderSearch - This class makes it easier to set the search paths of
-///  a HeaderSearch object. InitHeaderSearch stores several search path lists
-///  internally, which can be sent to a HeaderSearch object in one swoop.
+/// This class makes it easier to set the search paths of a HeaderSearch object.
+/// InitHeaderSearch stores several search path lists internally, which can be
+/// sent to a HeaderSearch object in one swoop.
 class InitHeaderSearch {
   std::vector IncludePath;
   std::vector > SystemHeaderPrefixes;
@@ -58,56 +58,54 @@
   : Headers(HS), Verbose(verbose), IncludeSysroot(std::string(sysroot)),
 HasSysroot(!(sysroot.empty() || sysroot == "/")) {}
 
-  /// AddPath - Add the specified path to the specified group list, prefixing
-  /// the sysroot if used.
+  /// Add the specified path to the specified group list, prefixing the sysroot
+  /// if used.
   /// Returns true if the path exists, false if it was ignored.
   bool AddPath(const Twine &Path, IncludeDirGroup Group, bool isFramework,
Optional UserEntryIdx = None);
 
-  /// AddUnmappedPath - Add the specified path to the specified group list,
-  /// without performing any sysroot remapping.
+  /// Add the specified path to the specified group list, without performing any
+  /// sysroot remapping.
   /// Returns true if the path exists, false if it was ignored.
   bool AddUnmappedPath(const Twine &Path, IncludeDirGroup Group,
bool isFramework,
Optional UserEntryIdx = None);
 
-  /// AddSystemHeaderPrefix - Add the specified prefix to the system header
-  /// prefix list.
+  /// Add the specified prefix to the system header prefix list.
   void AddSystemHeaderPrefix(StringRef Prefix, bool IsSystemHeader) {
 SystemHeaderPrefixes.emplace_back(std::string(Prefix), IsSystemHeader);
   }
 
-  /// AddGnuCPlusPlusIncludePaths - Add the necessary paths to support a gnu
-  ///  libstdc++.
+  /// Add the necessary paths to support a gnu libstdc++.
   /// Returns true if the \p Base path was found, false if it does not exist.
   bool AddGnuCPlusPlusIncludePaths(StringRef Base, StringRef ArchDir,
StringRef Dir32, StringRef Dir64,
const llvm::Triple &triple);
 
-  /// AddMinGWCPlusPlusIncludePaths - Add the necessary paths to support a MinGW
-  ///  libstdc++.
+  /// Add the necessary paths to support a MinGW libstdc++.
   void AddMinGWCPlusPlusIncludePaths(StringRef Base,
  StringRef Arch,
  

[PATCH] D138183: [Driver] move FreeBSD header search path management to the driver

[Matt Jacobson via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGba7a1d9e4aec: [Driver] move FreeBSD header search path 
management to the driver (authored by mhjacobson).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138183

Files:
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/FreeBSD.h
  clang/lib/Lex/InitHeaderSearch.cpp
  clang/test/Driver/freebsd.c
  clang/test/Driver/freebsd.cpp

Index: clang/test/Driver/freebsd.cpp
===
--- clang/test/Driver/freebsd.cpp
+++ clang/test/Driver/freebsd.cpp
@@ -40,3 +40,11 @@
 // CHECK-LIBCXX-SYSROOT-SLASH: "-cc1"
 // CHECK-LIBCXX-SYSROOT-SLASH-SAME: "-isysroot" "[[SYSROOT:[^"]+/]]"
 // CHECK-LIBCXX-SYSROOT-SLASH-SAME: "-internal-isystem" "[[SYSROOT]]usr/include/c++/v1"
+
+// Check that the driver passes include paths to cc1 on FreeBSD.
+// RUN: %clang -### %s --target=x86_64-unknown-freebsd13.1 -r 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=DRIVER-PASS-INCLUDES
+// DRIVER-PASS-INCLUDES:  "-cc1" {{.*}}"-resource-dir" "[[RESOURCE:[^"]+]]"
+// DRIVER-PASS-INCLUDES-SAME: {{^}} "-internal-isystem" "/usr/include/c++/v1"
+// DRIVER-PASS-INCLUDES-SAME: {{^}} "-internal-isystem" "[[RESOURCE]]{{/|}}include"
+// DRIVER-PASS-INCLUDES-SAME: {{^}} "-internal-externc-isystem" "/usr/include"
Index: clang/test/Driver/freebsd.c
===
--- clang/test/Driver/freebsd.c
+++ clang/test/Driver/freebsd.c
@@ -213,3 +213,10 @@
 // RELOCATABLE-NOT: "-dynamic-linker"
 // RELOCATABLE-NOT: "-l
 // RELOCATABLE-NOT: crt{{[^./]+}}.o
+
+// Check that the driver passes include paths to cc1 on FreeBSD.
+// RUN: %clang -### %s --target=x86_64-unknown-freebsd13.1 -r 2>&1 \
+// RUN:   | FileCheck %s --check-prefix=DRIVER-PASS-INCLUDES
+// DRIVER-PASS-INCLUDES:  "-cc1" {{.*}}"-resource-dir" "[[RESOURCE:[^"]+]]"
+// DRIVER-PASS-INCLUDES-SAME: {{^}} "-internal-isystem" "[[RESOURCE]]{{/|}}include"
+// DRIVER-PASS-INCLUDES-SAME: {{^}} "-internal-externc-isystem" "/usr/include"
Index: clang/lib/Lex/InitHeaderSearch.cpp
===
--- clang/lib/Lex/InitHeaderSearch.cpp
+++ clang/lib/Lex/InitHeaderSearch.cpp
@@ -42,9 +42,9 @@
   : Group(Group), Lookup(Lookup), UserEntryIdx(UserEntryIdx) {}
 };
 
-/// InitHeaderSearch - This class makes it easier to set the search paths of
-///  a HeaderSearch object. InitHeaderSearch stores several search path lists
-///  internally, which can be sent to a HeaderSearch object in one swoop.
+/// This class makes it easier to set the search paths of a HeaderSearch object.
+/// InitHeaderSearch stores several search path lists internally, which can be
+/// sent to a HeaderSearch object in one swoop.
 class InitHeaderSearch {
   std::vector IncludePath;
   std::vector > SystemHeaderPrefixes;
@@ -58,56 +58,54 @@
   : Headers(HS), Verbose(verbose), IncludeSysroot(std::string(sysroot)),
 HasSysroot(!(sysroot.empty() || sysroot == "/")) {}
 
-  /// AddPath - Add the specified path to the specified group list, prefixing
-  /// the sysroot if used.
+  /// Add the specified path to the specified group list, prefixing the sysroot
+  /// if used.
   /// Returns true if the path exists, false if it was ignored.
   bool AddPath(const Twine &Path, IncludeDirGroup Group, bool isFramework,
Optional UserEntryIdx = None);
 
-  /// AddUnmappedPath - Add the specified path to the specified group list,
-  /// without performing any sysroot remapping.
+  /// Add the specified path to the specified group list, without performing any
+  /// sysroot remapping.
   /// Returns true if the path exists, false if it was ignored.
   bool AddUnmappedPath(const Twine &Path, IncludeDirGroup Group,
bool isFramework,
Optional UserEntryIdx = None);
 
-  /// AddSystemHeaderPrefix - Add the specified prefix to the system header
-  /// prefix list.
+  /// Add the specified prefix to the system header prefix list.
   void AddSystemHeaderPrefix(StringRef Prefix, bool IsSystemHeader) {
 SystemHeaderPrefixes.emplace_back(std::string(Prefix), IsSystemHeader);
   }
 
-  /// AddGnuCPlusPlusIncludePaths - Add the necessary paths to support a gnu
-  ///  libstdc++.
+  /// Add the necessary paths to support a gnu libstdc++.
   /// Returns true if the \p Base path was found, false if it does not exist.
   bool AddGnuCPlusPlusIncludePaths(StringRef Base, StringRef ArchDir,
StringRef Dir32, StringRef Dir64,
const llvm::Triple &triple);
 
-  /// AddMinGWCPlusPlusIncludePaths - Add the necessary paths to support a MinGW
-  ///  libstdc++.
+  /// Add the necessary paths to support a MinGW libstdc++.
   void AddMinGWCPlusPlusIncludePaths(StringRef Base

[PATCH] D112113: [ObjC] type method metadata `_imp`, messenger routine at callsite with program address space

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson added a comment.

Happy new year!  If this change is acceptable, could someone please merge it 
for me?  (And if not, could someone point out what needs more work?)  Thanks 
for your help!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112113

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

[PATCH] D112049: [ObjC] avoid crashing when emitting synthesized getter/setter and ptrdiff_t is smaller than long

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson created this revision.
mhjacobson added reviewers: rjmccall, ddunbar.
Herald added subscribers: Jim, dylanmckay.
mhjacobson requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

On targets where `ptrdiff_t` is smaller than `long`, clang crashes (backtrace 
below) when emitting synthesized getters/setters that call 
`objc_[gs]etProperty`.  Explicitly emit a zext/trunc of the ivar offset value 
(which is defined to `long`) to `ptrdiff_t`, which `objc_[gs]etProperty` takes.

Add a test using the AVR target, where `ptrdiff_t` is smaller than `long`.  
Test failed previously and passes now.

Backtrace:

  Assertion failed: (castIsValid(op, S, Ty) && "Invalid cast!"), function 
Create, file /Users/matt/src/llvm/llvm/lib/IR/Instructions.cpp, line 3042.
  
  ...
  
  7  libsystem_c.dylib0x7fff6ec09ac6 err + 0
  8  clang-14 0x00010acf104c 
llvm::CastInst::Create(llvm::Instruction::CastOps, llvm::Value*, llvm::Type*, 
llvm::Twine const&, llvm::Instruction*) + 124
  9  clang-14 0x00010beb140a 
llvm::IRBuilderBase::CreateCast(llvm::Instruction::CastOps, llvm::Value*, 
llvm::Type*, llvm::Twine const&) + 234
  10 clang-14 0x00010bebdec2 
llvm::IRBuilderBase::CreateZExt(llvm::Value*, llvm::Type*, llvm::Twine const&) 
+ 50
  11 clang-14 0x00010cab50aa 
clang::CodeGen::CodeGenFunction::EmitCall(clang::CodeGen::CGFunctionInfo 
const&, clang::CodeGen::CGCallee const&, clang::CodeGen::ReturnValueSlot, 
clang::CodeGen::CallArgList const&, llvm::CallBase**, bool, 
clang::SourceLocation) + 8442
  12 clang-14 0x00010ceadeb8 
clang::CodeGen::CodeGenFunction::EmitCall(clang::CodeGen::CGFunctionInfo 
const&, clang::CodeGen::CGCallee const&, clang::CodeGen::ReturnValueSlot, 
clang::CodeGen::CallArgList const&, llvm::CallBase**, bool) + 216
  13 clang-14 0x00010cc76fe3 
clang::CodeGen::CodeGenFunction::generateObjCGetterBody(clang::ObjCImplementationDecl
 const*, clang::ObjCPropertyImplDecl const*, clang::ObjCMethodDecl const*, 
llvm::Constant*) + 2595
  14 clang-14 0x00010cc75387 
clang::CodeGen::CodeGenFunction::GenerateObjCGetter(clang::ObjCImplementationDecl*,
 clang::ObjCPropertyImplDecl const*) + 343
  15 clang-14 0x00010cf1c4c0 
clang::CodeGen::CodeGenModule::EmitObjCPropertyImplementations(clang::ObjCImplementationDecl
 const*) + 336
  16 clang-14 0x00010cf17799 
clang::CodeGen::CodeGenModule::EmitTopLevelDecl(clang::Decl*) + 2009
  17 clang-14 0x00010d114bf2 (anonymous 
namespace)::CodeGeneratorImpl::HandleTopLevelDecl(clang::DeclGroupRef) + 146
  18 clang-14 0x00010cec0014 
clang::BackendConsumer::HandleTopLevelDecl(clang::DeclGroupRef) + 228
  19 clang-14 0x000110436425 clang::ParseAST(clang::Sema&, 
bool, bool) + 533


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112049

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/avr/objc-property.m


Index: clang/test/CodeGen/avr/objc-property.m
===
--- /dev/null
+++ clang/test/CodeGen/avr/objc-property.m
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple avr -emit-llvm -fobjc-runtime=macosx %s -o /dev/null
+
+__attribute__((objc_root_class))
+@interface Foo
+
+@property(strong) Foo *f;
+
+@end
+
+@implementation Foo
+
+@synthesize f = _f;
+
+@end
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -3906,6 +3906,8 @@
 
   llvm::Value *EmitIvarOffset(const ObjCInterfaceDecl *Interface,
   const ObjCIvarDecl *Ivar);
+  llvm::Value *EmitIvarOffsetAsPointerDiff(const ObjCInterfaceDecl *Interface,
+   const ObjCIvarDecl *Ivar);
   LValue EmitLValueForField(LValue Base, const FieldDecl* Field);
   LValue EmitLValueForLambdaField(const FieldDecl *Field);
 
Index: clang/lib/CodeGen/CGObjC.cpp
===
--- clang/lib/CodeGen/CGObjC.cpp
+++ clang/lib/CodeGen/CGObjC.cpp
@@ -1192,7 +1192,7 @@
   Builder.CreateLoad(GetAddrOfLocalVar(getterMethod->getCmdDecl()), "cmd");
 llvm::Value *self = Builder.CreateBitCast(LoadObjCSelf(), VoidPtrTy);
 llvm::Value *ivarOffset =
-  EmitIvarOffset(classImpl->getClassInterface(), ivar);
+EmitIvarOffsetAsPointerDiff(classImpl->getClassInterface(), ivar);
 
 CallArgList args;
 args.add(RValue::get(self), getContext().getObjCIdType());
@@ -1479,7 +1479,7 @@
 llvm::Value *self =
   Builder.CreateBitCast(LoadObjCSelf(), VoidPtrTy);
 llvm::Value *ivarOffset =
-  EmitIvarOffset(classImpl->getCl

[PATCH] D112049: [ObjC] avoid crashing when emitting synthesized getter/setter and ptrdiff_t is smaller than long

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson added a comment.

The test is failing for a different reason (that I'd forgotten I worked around 
locally).  I'll look into fixing that first.  Sorry for noise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112049

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

[PATCH] D112113: [ObjC] type method metadata `_imp`, messenger routine at callsite with program address space

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson created this revision.
mhjacobson added reviewers: rjmccall, ddunbar.
Herald added subscribers: pengfei, Jim, dylanmckay.
mhjacobson requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

On targets with non-default program address space (e.g., Harvard 
architectures), clang crashes (example backtrace below) when emitting 
Objective-C method metadata, because the address of the method IMP cannot be 
bitcast to `i8*`.  It similarly crashes at messenger callsite with a failed 
bitcast.

Define the `_imp` field instead as `i8 addrspace(1)*` (or whatever the target's 
program address space is).  And in `getMessageSendInfo()`, create 
`signatureType` by specifying the program address space.

Add a regression test using the AVR target.  Test failed previously and passes 
now.  Checked codegen of the test for x86_64-apple-darwin19.6.0 and saw no 
difference, as expected.

Backtrace (when emitting method metadata):

  Assertion failed: (CastInst::castIsValid(Instruction::BitCast, C, DstTy) && 
"Invalid constantexpr bitcast!"), function getBitCast, file 
/Users/matt/src/llvm/llvm/lib/IR/Constants.cpp, line 2224.
  
  ...
  
  7  libsystem_c.dylib0x7fff6ec09ac6 err + 0
  8  clang-14 0x000101800518 
llvm::ConstantExpr::getBitCast(llvm::Constant*, llvm::Type*, bool) + 120
  9  clang-14 0x00010394ca70 
clang::CodeGen::ConstantAggregateBuilderBase::addBitCast(llvm::Constant*, 
llvm::Type*) + 48
  10 clang-14 0x000103999ac2 (anonymous 
namespace)::CGObjCNonFragileABIMac::emitMethodConstant(clang::CodeGen::ConstantArrayBuilder&,
 clang::ObjCMethodDecl const*, bool) + 402
  11 clang-14 0x000103999102 (anonymous 
namespace)::CGObjCNonFragileABIMac::emitMethodList(llvm::Twine, (anonymous 
namespace)::(anonymous namespace)::MethodListType, 
llvm::ArrayRef) + 1074
  12 clang-14 0x00010399a4c0 (anonymous 
namespace)::CGObjCNonFragileABIMac::BuildClassRoTInitializer(unsigned int, 
unsigned int, unsigned int, clang::ObjCImplementationDecl const*) + 1376
  13 clang-14 0x000103990b9b (anonymous 
namespace)::CGObjCNonFragileABIMac::GenerateClass(clang::ObjCImplementationDecl 
const*) + 1995
  14 clang-14 0x000103bb87d1 
clang::CodeGen::CodeGenModule::EmitTopLevelDecl(clang::Decl*) + 2065
  15 clang-14 0x000103db5bf2 (anonymous 
namespace)::CodeGeneratorImpl::HandleTopLevelDecl(clang::DeclGroupRef) + 146
  16 clang-14 0x000103b61014 
clang::BackendConsumer::HandleTopLevelDecl(clang::DeclGroupRef) + 228
  17 clang-14 0x0001070d7425 clang::ParseAST(clang::Sema&, 
bool, bool) + 533


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112113

Files:
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/CodeGen/CGObjCRuntime.cpp
  clang/test/CodeGen/avr/objc-method.m

Index: clang/test/CodeGen/avr/objc-method.m
===
--- /dev/null
+++ clang/test/CodeGen/avr/objc-method.m
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -triple avr -emit-llvm -fobjc-runtime=macosx %s -o /dev/null
+
+__attribute__((objc_root_class))
+@interface Foo
+
+- (id)foo;
+- (id)bar;
+
+@end
+
+@implementation Foo
+
+- (id)foo {
+  return self;
+}
+
+- (id)bar {
+  return [self foo];
+}
+
+@end
Index: clang/lib/CodeGen/CGObjCRuntime.cpp
===
--- clang/lib/CodeGen/CGObjCRuntime.cpp
+++ clang/lib/CodeGen/CGObjCRuntime.cpp
@@ -361,13 +361,15 @@
 CGObjCRuntime::getMessageSendInfo(const ObjCMethodDecl *method,
   QualType resultType,
   CallArgList &callArgs) {
+  unsigned ProgramAS = CGM.getDataLayout().getProgramAddressSpace();
+
   // If there's a method, use information from that.
   if (method) {
 const CGFunctionInfo &signature =
   CGM.getTypes().arrangeObjCMessageSendSignature(method, callArgs[0].Ty);
 
 llvm::PointerType *signatureType =
-  CGM.getTypes().GetFunctionType(signature)->getPointerTo();
+  CGM.getTypes().GetFunctionType(signature)->getPointerTo(ProgramAS);
 
 const CGFunctionInfo &signatureForCall =
   CGM.getTypes().arrangeCall(signature, callArgs);
@@ -381,7 +383,7 @@
 
   // Derive the signature to call from that.
   llvm::PointerType *signatureType =
-CGM.getTypes().GetFunctionType(argsInfo)->getPointerTo();
+CGM.getTypes().GetFunctionType(argsInfo)->getPointerTo(ProgramAS);
   return MessageSendInfo(argsInfo, signatureType);
 }
 
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -174,6 +174,7 @@
 public:
   llvm::IntegerType *ShortTy, *IntTy, *LongTy;
   llvm::PointerType *Int8PtrTy, *Int8PtrPtrTy;
+  llvm::PointerType *Int8PtrProgr