[PATCH] D58216: Support attribute used in member funcs of class templates II

2019-03-07 Thread Bob Wilson via Phabricator via cfe-commits
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.llv

[PATCH] D46550: Support Swift calling convention for PPC64 targets

2018-05-25 Thread Bob Wilson via Phabricator via cfe-commits
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/cf

[PATCH] D46550: Support Swift calling convention for PPC64 targets

2018-05-07 Thread Bob Wilson via Phabricator via cfe-commits
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

[PATCH] D46550: Support Swift calling convention for PPC64 targets

2018-05-07 Thread Bob Wilson via Phabricator via cfe-commits
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://rev

[PATCH] D44753: [Preprocessor] Rename __is_{target -> host}_* function-like builtin macros

2018-03-21 Thread Bob Wilson via Phabricator via cfe-commits
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

[PATCH] D41425: [darwin][driver] Warn about mismatching --version-min rather than superfluous --version-min compiler option

2017-12-19 Thread Bob Wilson via Phabricator via cfe-commits
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.

[PATCH] D40998: [driver][darwin] Take the OS version specified in "-target" as the target OS instead of inferring it from SDK / environment

2017-12-15 Thread Bob Wilson via Phabricator via cfe-commits
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] D41076: [driver][darwin] Set the 'simulator' environment when it's specified in '-target'

2017-12-15 Thread Bob Wilson via Phabricator via cfe-commits
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 || -

[PATCH] D41087: [Preprocessor] Implement __is_target_{arch|vendor|os|environment} function-like builtin macros

2017-12-15 Thread Bob Wilson via Phabricator via cfe-commits
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] D41087: [Preprocessor] Implement __is_target_{arch|vendor|os|environment} function-like builtin macros

2017-12-14 Thread Bob Wilson via Phabricator via cfe-commits
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

[PATCH] D40998: [driver][darwin] Take the OS version specified in "-target" as the target OS instead of inferring it from SDK / environment

2017-12-09 Thread Bob Wilson via Phabricator via cfe-commits
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 EnvTa

[PATCH] D40682: [driver] Set the 'simulator' environment for Darwin when -msimulator-version-min is used

2017-12-04 Thread Bob Wilson via Phabricator via cfe-commits
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().getAr

[PATCH] D40682: [driver] Set the 'simulator' environment for Darwin when -msimulator-version-min is used

2017-11-30 Thread Bob Wilson via Phabricator via cfe-commits
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

[PATCH] D40682: [driver] Set the 'simulator' environment for Darwin when -msimulator-version-min is used

2017-11-30 Thread Bob Wilson via Phabricator via cfe-commits
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 || getTrip

[PATCH] D39446: [PGO] Detect more structural changes with the stable hash

2017-10-31 Thread Bob Wilson via Phabricator via cfe-commits
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

[PATCH] D36563: Add a getName accessor for ModuleMacros

2017-08-10 Thread Bob Wilson via Phabricator via cfe-commits
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] D36563: Add a getName accessor for ModuleMacros

2017-08-09 Thread Bob Wilson via Phabricator via cfe-commits
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://revi

[PATCH] D34529: [Driver] Check that the iOS deployment target is iOS 10 or earlier if the target is 32-bit

2017-06-30 Thread Bob Wilson via Phabricator via cfe-commits
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/listin

[PATCH] D34529: [Driver] Check that the iOS deployment target is iOS 10 or earlier if the target is 32-bit

2017-06-30 Thread Bob Wilson via Phabricator via cfe-commits
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-com

[PATCH] D34529: [Driver] Check that the iOS deployment target is iOS 10 or earlier if the target is 32-bit

2017-06-29 Thread Bob Wilson via Phabricator via cfe-commits
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 m