[PATCH] D46550: Support Swift calling convention for PPC64 targets
bob.wilson closed this revision. bob.wilson added a comment. Committed in Clang r16 Repository: rC Clang https://reviews.llvm.org/D46550 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41087: [Preprocessor] Implement __is_target_{arch|vendor|os|environment} function-like builtin macros
bob.wilson added a comment. I'm concerned here about the check for the names as written vs. the canonical names. @compnerd pointed out one specific case with armv7, and I see that you've got special handling for "darwin", but I think there are more. What about "arm64" vs. the canonical "aarch64"? Look through the triple parsing code in Triple.cpp and I'm pretty sure you'll find more. I had been thinking about using the canonical names. However, that's not ideal either because the canonical names intentionally exclude suffixes that some users may want to distinguish (e.g., armv7 vs armv7k). Repository: rC Clang https://reviews.llvm.org/D41087 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41087: [Preprocessor] Implement __is_target_{arch|vendor|os|environment} function-like builtin macros
bob.wilson added a comment. Thanks! LGTM Repository: rC Clang https://reviews.llvm.org/D41087 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41076: [driver][darwin] Set the 'simulator' environment when it's specified in '-target'
bob.wilson accepted this revision. bob.wilson added a comment. LGTM Comment at: lib/Driver/ToolChains/Darwin.cpp:1603 // Recognize iOS targets with an x86 architecture as the iOS simulator. - if (Platform != MacOS && (getTriple().getArch() == llvm::Triple::x86 || -getTriple().getArch() == llvm::Triple::x86_64)) + if (Environment == NativeEnvironment && Platform != MacOS && + (getTriple().getArch() == llvm::Triple::x86 || steven_wu wrote: > Will be good to emit a warning for this. User should use -target with > simulator or simulator deployment target. Can we hold off on the warning and try to add that separately in the future? It would be good to first find out how often this code is being used. If there are a lot of uses, we may need to try to get people to stop relying on it before we add the warning. (I'm concerned about -Werror breaking builds with this.) Repository: rC Clang https://reviews.llvm.org/D41076 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40998: [driver][darwin] Take the OS version specified in "-target" as the target OS instead of inferring it from SDK / environment
bob.wilson accepted this revision. bob.wilson added a comment. LGTM Repository: rC Clang https://reviews.llvm.org/D40998 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41425: [darwin][driver] Warn about mismatching --version-min rather than superfluous --version-min compiler option
bob.wilson accepted this revision. bob.wilson added a comment. This revision is now accepted and ready to land. Eventually it would be nice to also warn about redundant -m*-version-min options, but for now I agree that it would be best to start with warning only when the options are different. Repository: rC Clang https://reviews.llvm.org/D41425 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D58216: Support attribute used in member funcs of class templates II
bob.wilson added a comment. OK, that doesn't sound like it will be a problem Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58216/new/ https://reviews.llvm.org/D58216 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46550: Support Swift calling convention for PPC64 targets
bob.wilson created this revision.
bob.wilson added a reviewer: aschwaighofer.
Herald added subscribers: kbarton, nemanjai, mcrosier.
This adds basic support for the Swift calling convention with PPC64 targets.
Patch provided by Atul Sowani in bug report #37223
Repository:
rC Clang
https://reviews.llvm.org/D46550
Files:
lib/Basic/Targets/PPC.h
lib/CodeGen/TargetInfo.cpp
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -4287,7 +4287,7 @@
namespace {
/// PPC64_SVR4_ABIInfo - The 64-bit PowerPC ELF (SVR4) ABI information.
-class PPC64_SVR4_ABIInfo : public ABIInfo {
+class PPC64_SVR4_ABIInfo : public SwiftABIInfo {
public:
enum ABIKind {
ELFv1 = 0,
@@ -4331,7 +4331,7 @@
public:
PPC64_SVR4_ABIInfo(CodeGen::CodeGenTypes &CGT, ABIKind Kind, bool HasQPX,
bool SoftFloatABI)
- : ABIInfo(CGT), Kind(Kind), HasQPX(HasQPX),
+ : SwiftABIInfo(CGT), Kind(Kind), HasQPX(HasQPX),
IsSoftFloatABI(SoftFloatABI) {}
bool isPromotableTypeForABI(QualType Ty) const;
@@ -4374,6 +4374,15 @@
Address EmitVAArg(CodeGenFunction &CGF, Address VAListAddr,
QualType Ty) const override;
+
+ bool shouldPassIndirectlyForSwift(ArrayRef scalars,
+bool asReturnValue) const override {
+return occupiesMoreThan(CGT, scalars, /*total*/ 4);
+ }
+
+ bool isSwiftErrorInRegister() const override {
+return false;
+ }
};
class PPC64_SVR4_TargetCodeGenInfo : public TargetCodeGenInfo {
Index: lib/Basic/Targets/PPC.h
===
--- lib/Basic/Targets/PPC.h
+++ lib/Basic/Targets/PPC.h
@@ -335,6 +335,15 @@
}
return false;
}
+
+ CallingConvCheckResult checkCallingConvention(CallingConv CC) const override
{
+switch (CC) {
+case CC_Swift:
+ return CCCR_OK;
+default:
+ return CCCR_Warning;
+}
+ }
};
class LLVM_LIBRARY_VISIBILITY DarwinPPC32TargetInfo
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -4287,7 +4287,7 @@
namespace {
/// PPC64_SVR4_ABIInfo - The 64-bit PowerPC ELF (SVR4) ABI information.
-class PPC64_SVR4_ABIInfo : public ABIInfo {
+class PPC64_SVR4_ABIInfo : public SwiftABIInfo {
public:
enum ABIKind {
ELFv1 = 0,
@@ -4331,7 +4331,7 @@
public:
PPC64_SVR4_ABIInfo(CodeGen::CodeGenTypes &CGT, ABIKind Kind, bool HasQPX,
bool SoftFloatABI)
- : ABIInfo(CGT), Kind(Kind), HasQPX(HasQPX),
+ : SwiftABIInfo(CGT), Kind(Kind), HasQPX(HasQPX),
IsSoftFloatABI(SoftFloatABI) {}
bool isPromotableTypeForABI(QualType Ty) const;
@@ -4374,6 +4374,15 @@
Address EmitVAArg(CodeGenFunction &CGF, Address VAListAddr,
QualType Ty) const override;
+
+ bool shouldPassIndirectlyForSwift(ArrayRef scalars,
+bool asReturnValue) const override {
+return occupiesMoreThan(CGT, scalars, /*total*/ 4);
+ }
+
+ bool isSwiftErrorInRegister() const override {
+return false;
+ }
};
class PPC64_SVR4_TargetCodeGenInfo : public TargetCodeGenInfo {
Index: lib/Basic/Targets/PPC.h
===
--- lib/Basic/Targets/PPC.h
+++ lib/Basic/Targets/PPC.h
@@ -335,6 +335,15 @@
}
return false;
}
+
+ CallingConvCheckResult checkCallingConvention(CallingConv CC) const override {
+switch (CC) {
+case CC_Swift:
+ return CCCR_OK;
+default:
+ return CCCR_Warning;
+}
+ }
};
class LLVM_LIBRARY_VISIBILITY DarwinPPC32TargetInfo
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46550: Support Swift calling convention for PPC64 targets
bob.wilson added a comment. Previous review (for the swift-llvm GitHub repo): https://github.com/apple/swift-clang/pull/167 Repository: rC Clang https://reviews.llvm.org/D46550 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40682: [driver] Set the 'simulator' environment for Darwin when -msimulator-version-min is used
bob.wilson requested changes to this revision. bob.wilson added inline comments. This revision now requires changes to proceed. Comment at: lib/Driver/ToolChains/Darwin.cpp:1457-1465 if (iOSVersion && (getTriple().getArch() == llvm::Triple::x86 || getTriple().getArch() == llvm::Triple::x86_64)) Platform = IPhoneOSSimulator; if (TvOSVersion && (getTriple().getArch() == llvm::Triple::x86 || getTriple().getArch() == llvm::Triple::x86_64)) Platform = TvOSSimulator; if (WatchOSVersion && (getTriple().getArch() == llvm::Triple::x86 || These checks should set the simulator target as well. Repository: rC Clang https://reviews.llvm.org/D40682 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40682: [driver] Set the 'simulator' environment for Darwin when -msimulator-version-min is used
bob.wilson added inline comments. Comment at: lib/Driver/ToolChains/Darwin.cpp:1457-1465 if (iOSVersion && (getTriple().getArch() == llvm::Triple::x86 || getTriple().getArch() == llvm::Triple::x86_64)) Platform = IPhoneOSSimulator; if (TvOSVersion && (getTriple().getArch() == llvm::Triple::x86 || getTriple().getArch() == llvm::Triple::x86_64)) Platform = TvOSSimulator; if (WatchOSVersion && (getTriple().getArch() == llvm::Triple::x86 || arphaman wrote: > bob.wilson wrote: > > These checks should set the simulator target as well. > This would break the `__APPLE_EMBEDDED_SIMULATOR__` macro, as that has to be > specified only when is used `-m(iphone|tv|watch)simulator-version-min`. Or > should we still set `__APPLE_EMBEDDED_SIMULATOR__` in the driver and allow > `-simulator` even without `-m(iphone|tv|watch)simulator-version-min`? The intention is that using a target triple with "simulator" in the environment should replace the use of -m*simulator-version-min. Repository: rC Clang https://reviews.llvm.org/D40682 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40682: [driver] Set the 'simulator' environment for Darwin when -msimulator-version-min is used
bob.wilson accepted this revision. bob.wilson added inline comments. This revision is now accepted and ready to land. Comment at: lib/Driver/ToolChains/Darwin.cpp:1457-1465 if (iOSVersion && (getTriple().getArch() == llvm::Triple::x86 || getTriple().getArch() == llvm::Triple::x86_64)) Platform = IPhoneOSSimulator; if (TvOSVersion && (getTriple().getArch() == llvm::Triple::x86 || getTriple().getArch() == llvm::Triple::x86_64)) Platform = TvOSSimulator; if (WatchOSVersion && (getTriple().getArch() == llvm::Triple::x86 || bob.wilson wrote: > arphaman wrote: > > bob.wilson wrote: > > > These checks should set the simulator target as well. > > This would break the `__APPLE_EMBEDDED_SIMULATOR__` macro, as that has to > > be specified only when is used `-m(iphone|tv|watch)simulator-version-min`. > > Or should we still set `__APPLE_EMBEDDED_SIMULATOR__` in the driver and > > allow `-simulator` even without `-m(iphone|tv|watch)simulator-version-min`? > The intention is that using a target triple with "simulator" in the > environment should replace the use of -m*simulator-version-min. Alex pointed out to me that the subsequent call to setTarget takes care of setting the triple, so this is already doing what I wanted. Repository: rC Clang https://reviews.llvm.org/D40682 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D40998: [driver][darwin] Take the OS version specified in "-target" as the target OS instead of inferring it from SDK / environment
bob.wilson requested changes to this revision. bob.wilson added inline comments. This revision now requires changes to proceed. Comment at: lib/Driver/ToolChains/Darwin.cpp:1518-1523 + // Warn about superfluous OS_DEPLOYMENT_TARGET environment variable. + Optional EnvTarget = + getDeploymentTargetFromEnvironmentVariables(getDriver(), getTriple()); + if (EnvTarget) +getDriver().Diag(clang::diag::warn_drv_unused_environment_variable) +<< EnvTarget->getAsString(Args, Opts); I don't think there should be a warning in this case. It is common (at least within Apple) to set the environment variable as a default but then override it for some cases. Warning would be really annoying, and for anyone using -Werror it will break their builds. Repository: rC Clang https://reviews.llvm.org/D40998 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36563: Add a getName accessor for ModuleMacros
bob.wilson created this revision.
Herald added a subscriber: mcrosier.
Swift would like to be able to access the name of a ModuleMacro. There was some
discussion of this in https://github.com/apple/swift-clang/pull/93, suggesting
that it makes sense to have this accessor in Clang.
https://reviews.llvm.org/D36563
Files:
clang/Lex/MacroInfo.h
Index: clang/Lex/MacroInfo.h
===
--- clang/Lex/MacroInfo.h
+++ clang/Lex/MacroInfo.h
@@ -510,6 +510,9 @@
ID.AddPointer(II);
}
+ /// Get the name of the macro.
+ IdentifierInfo *getName() const { return II; }
+
/// Get the ID of the module that exports this macro.
Module *getOwningModule() const { return OwningModule; }
Index: clang/Lex/MacroInfo.h
===
--- clang/Lex/MacroInfo.h
+++ clang/Lex/MacroInfo.h
@@ -510,6 +510,9 @@
ID.AddPointer(II);
}
+ /// Get the name of the macro.
+ IdentifierInfo *getName() const { return II; }
+
/// Get the ID of the module that exports this macro.
Module *getOwningModule() const { return OwningModule; }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36563: Add a getName accessor for ModuleMacros
bob.wilson added a comment. Committed in r310622 https://reviews.llvm.org/D36563 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D44753: [Preprocessor] Rename __is_{target -> host}_* function-like builtin macros
bob.wilson added a comment. Sorry that I missed your earlier comment about this. The confusion could only arise in the context of a tool (like a compiler) that is being used for cross-compilation. That is a small fraction of the audience for Clang, and we should design this in a way that makes the most sense for the majority of users. If there's a naming scheme that is better for both, then we should do that, but I don't think this is it. When dealing with a cross compiler, there is a need to distinguish the "target" where the compiler will run (which as you point out is typically referred to as the "host") from the "target" code produced by that cross compiler. There are two points in time: (1) when compiling the cross compiler, and (2) when running the cross compiler. In step (1), the compiler will be invoked with a "-target" option that specifies the "host". The preprocessor checks are compile-time checks, so there no way that one of these macros in the source code of the compiler itself could be referring to the target in step (2). The compiler option name will be "-target" regardless. Using "target" names in the macros is consistent with that compiler option name. When dealing with anything other than a cross compiler (or similar cross-target development tool), the "host" terminology is not commonly used. The obvious connection between these macros and the value specified by the "-target" option would be lost. I really don't think this is a good alternative. Repository: rC Clang https://reviews.llvm.org/D44753 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39446: [PGO] Detect more structural changes with the stable hash
bob.wilson added a comment.
I'm excited to see some progress on this, but since there is overhead to adding
a new hashing scheme, I think we should do more before introducing a new
scheme. One of the problems with the previous scheme is that is did not take
into account nesting. Distinguishing an if-statement from an if-else statement
is good but we also need to distinguish what code is inside the then-block,
else-block, and the code afterward. As it stands, I believe the following are
all hashed the same, even with this patch:
Loop after the if-else:
if (condition1())
x = 1;
else
x = 2;
while (condition2()) body();
Loop in the else-block:
if (condition1())
x = 1;
else
while (condition2()) body();
Loop in the then-block:
if (condition1()) {
while (condition2()) body();
} else
x = 2
It would be good to fix that.
https://reviews.llvm.org/D39446
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34529: [Driver] Check that the iOS deployment target is iOS 10 or earlier if the target is 32-bit
bob.wilson added a comment. The proposed error message does not provide any information about why the version is invalid. That could be confusing. Your comment in the code is more clear: "iOS 10 is the maximum deployment target for 32-bit targets". Can you say something like that in the error message? In the case where the version is inferred from the SDK, you're resetting Major=10 but leaving Minor and Micro unchanged. That seems wrong. Those should be set to the most recent release of iOS 10. Perhaps you could set those to big numbers, e.g., 99, so that they are sure to include any future iOS 10 releases. https://reviews.llvm.org/D34529 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34529: [Driver] Check that the iOS deployment target is iOS 10 or earlier if the target is 32-bit
bob.wilson added inline comments.
Comment at: lib/Driver/ToolChains/Darwin.cpp:1350
+Minor = 99;
+ }
} else if (Platform == TvOS) {
What about Micro = 99?
https://reviews.llvm.org/D34529
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34529: [Driver] Check that the iOS deployment target is iOS 10 or earlier if the target is 32-bit
bob.wilson accepted this revision. bob.wilson added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D34529 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
