[PATCH] D30427: Fix whitespace before token-paste of an argument.

2017-02-27 Thread James Y Knight via Phabricator via cfe-commits
jyknight created this revision. The whitespace should come from the argument name in the macro expansion, rather than from the token passed to the macro (same as it does when not pasting). Added a new test case for the change in behavior to stringize_space.c. FileCheck'ized

[PATCH] D24688: Introduce "hosted" module flag.

2017-01-07 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. Since this requires everyone generating llvm IR to add this module attribute for optimal codegen, it should be documented in release notes and the LangRef, I think. I mean: it's unfortunate that it's needed at all, but at the very least it should be possible for

[PATCH] D30427: Fix whitespace before token-paste of an argument.

2017-03-27 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. Ping. https://reviews.llvm.org/D30427 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27387: [libc++] Add a key function for bad_function_call

2017-03-16 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. Since this is only adding a new export to libc++, not changing/breaking existing ones, I don't think it should be grouped with the ABI-breaking changes in v2. The usual promise is that upgrading libc++.so.1 will not break apps compiled against an older set of

[PATCH] D31022: Implement P0298R3: `std::byte`

2017-04-03 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. I believe this needs compiler support, too, in order to treat namespace std { enum class byte : unsigned char {}; } as directly having tbaa type "omnipotent char" instead of a subtype. That is, given: void foo(char* x, int *y) { x[1] = char(y[0] & 0xff);

[PATCH] D30427: Fix whitespace before token-paste of an argument.

2017-03-06 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. Ping https://reviews.llvm.org/D30427 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34158: For standards compatibility, preinclude if the file is available

2017-07-28 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. In https://reviews.llvm.org/D34158#823316, @fedor.sergeev wrote: > Hmm... I tried this patch and now the following worries me: > > - it passes -finclude-if-exists stdc-predef.h on all platforms (say, > including my Solaris platform that has no system stdc-predef.h)

[PATCH] D34158: For Linux/gnu compatibility, preinclude if the file is available

2017-08-10 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. In https://reviews.llvm.org/D34158#838454, @mibintc wrote: > This patch is responding to @jyknight 's concern about preprocessed input. > The patch as it stands doesn't have this issue. I added 2 test cases, one > using option -x cpp-output, and another for a source

[PATCH] D34158: For standards compatibility, preinclude if the file is available

2017-07-10 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. This version is still disabling upon -nostdinc, which doesn't make sense to me. You didn't remove that, nor respond explaining why you think it does make sense? https://reviews.llvm.org/D34158 ___ cfe-commits mailing list

[PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude

2017-07-06 Thread James Y Knight via Phabricator via cfe-commits
jyknight added inline comments. Comment at: lib/Driver/ToolChains/Linux.cpp:755 +!Version.isOlderThan(4, 8, 0)) { + // For gcc >= 4.8.x, clang will preinclude + // -ffreestanding suppresses this behavior. jyknight wrote: > I don't see why it

[PATCH] D34294: Rework libcxx strerror_r handling.

2017-07-19 Thread James Y Knight via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL308528: Rework libcxx strerror_r handling. (authored by jyknight). Repository: rL LLVM https://reviews.llvm.org/D34294 Files: libcxx/trunk/src/system_error.cpp Index:

[PATCH] D34158: to support gcc 4.8 (and newer) compatibility on Linux, preinclude

2017-07-05 Thread James Y Knight via Phabricator via cfe-commits
jyknight added inline comments. Comment at: lib/Driver/ToolChains/Linux.cpp:755 +!Version.isOlderThan(4, 8, 0)) { + // For gcc >= 4.8.x, clang will preinclude + // -ffreestanding suppresses this behavior. I don't see why it makes any sense to

[PATCH] D31022: Implement P0298R3: `std::byte`

2017-07-05 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. Ping. I don't think this got resolved, and I really wouldn't like to see released in this state...can you either revert this from the library, or implement the compiler support, before the 5.0 release branch? In https://reviews.llvm.org/D31022#716702, @jyknight

[PATCH] D34158: For Linux/gnu compatibility, preinclude if the file is available

2017-08-08 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. In https://reviews.llvm.org/D34158#827178, @joerg wrote: > I had a long discussion with James about this on IRC without reaching a clear > consensus. I consider forcing this behavior on all targets to be a major bug. > It should be opt-in and opt-in only: > > (1) The

[PATCH] D34158: For Linux/gnu compatibility, preinclude if the file is available

2017-08-09 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. Just to restate: the ideal outcome of this discussion for me would be to resolve things such that _ALL_ libc implementations will feel comfortable using this technique to provide the C11-required predefined macros. I'd love for linux, freebsd, macos, solaris, etc etc

[PATCH] D33020: [Myriad] Pass -Xclang and -mllvm flags to moviCompile

2017-05-10 Thread James Y Knight via Phabricator via cfe-commits
jyknight accepted this revision. jyknight added a comment. This revision is now accepted and ready to land. Will submit for you since you don't have an svn account. https://reviews.llvm.org/D33020 ___ cfe-commits mailing list

[PATCH] D29117: SPARC: allow usage of floating-point registers in inline ASM

2017-05-12 Thread James Y Knight via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL302913: [SPARC] Support 'f' and 'e' inline asm constraints. (authored by jyknight). Changed prior to commit: https://reviews.llvm.org/D29117?vs=85708=98782#toc Repository: rL LLVM

[PATCH] D33020: [Myriad] Pass -Xclang and -mllvm flags to moviCompile

2017-05-10 Thread James Y Knight via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL302738: [Myriad] Pass -Xclang and -mllvm flags to moviCompile (authored by jyknight). Changed prior to commit: https://reviews.llvm.org/D33020?vs=98369=98538#toc Repository: rL LLVM

[PATCH] D34294: Rework libcxx strerror_r handling.

2017-06-20 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. EricWF, wdyt? https://reviews.llvm.org/D34294 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34294: Rework libcxx strerror_r handling.

2017-06-16 Thread James Y Knight via Phabricator via cfe-commits
jyknight created this revision. Herald added subscribers: krytarowski, srhines. The set of #ifdefs used to handle the two incompatible variants of strerror_r were not complete (they didn't handle newlib appropriately). Rather than attempting to make the ifdefs more complex, make them unnecessary

[PATCH] D34294: Rework libcxx strerror_r handling.

2017-06-16 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. In https://reviews.llvm.org/D34294#782806, @krytarowski wrote: > New one is harder to comprehend and less portable (usage of `__atribute__`). I can't disagree more strongly. This is fundamentally portable C++ code -- __attribute__((unused)) is simply warning

[PATCH] D32146: PR32476: __nop_locale_mgmt.h not needed with newlib 2.5+

2017-06-14 Thread James Y Knight via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL305394: PR32476: __nop_locale_mgmt.h not needed with newlib 2.5+ (authored by jyknight). Changed prior to commit: https://reviews.llvm.org/D32146?vs=102265=102562#toc Repository: rL LLVM

[PATCH] D34105: Define _GNU_SOURCE for rtems c++

2017-06-14 Thread James Y Knight via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL305399: Define _GNU_SOURCE for rtems c++ (authored by jyknight). Changed prior to commit: https://reviews.llvm.org/D34105?vs=102188=102565#toc Repository: rL LLVM https://reviews.llvm.org/D34105

[PATCH] D30427: Fix whitespace before token-paste of an argument.

2017-05-04 Thread James Y Knight via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL302195: Fix whitespace before token-paste of an argument. (authored by jyknight). Changed prior to commit: https://reviews.llvm.org/D30427?vs=89918=97881#toc Repository: rL LLVM

[PATCH] D35755: [Solaris] gcc toolchain handling revamp

2017-12-19 Thread James Y Knight via Phabricator via cfe-commits
jyknight accepted this revision. jyknight added a comment. This revision is now accepted and ready to land. This looks reasonable on the face of it. I'm assuming you know the layout for Solaris, and it doesn't seem to change the behavior of non-Solaris, so LGTM.

[PATCH] D34158: For Linux/gnu compatibility, preinclude if the file is available

2017-10-24 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. I'd still _very_ much like a solution that is acceptable for all libc to use, and have that on by default. However, I guess this is fine. Only concern I have is that it seems odd that so many test-cases needed to be changed. What fails without those test changes?

[PATCH] D37954: Try to shorten system header paths when using -MD depfiles

2017-10-21 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. I think the diagnosis on the original issue was incorrect. It seems to me that it was caused by the prefix being set as "/bin" instead of "/usr/bin", because clang _doesn't_ actually canonicalize its prefix, even when -no-canonical-prefixes isn't specified! All it

[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers

2018-05-08 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. In https://reviews.llvm.org/D42933#1090384, @jfb wrote: > In https://reviews.llvm.org/D42933#1090286, @smeenai wrote: > > > I'd be fine with adding an option to relax the printf checking if the size > > and alignment of the specifier and the actual type match, even if

[PATCH] D42933: [Sema] Avoid -Wformat warning for NSInteger/NSUInteger 'int' values with %zu/%zi long specifiers

2018-05-08 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. In https://reviews.llvm.org/D42933#1091817, @jfb wrote: > In https://reviews.llvm.org/D42933#1091809, @jyknight wrote: > > > I also think that special casing these two specifiers doesn't make sense. > > The problem is a general issue -- and one I've often found

[PATCH] D47138: [Sparc] Use the leon arch for Leon3's when using an external assembler

2018-05-21 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. There are more leon-based v8 CPUs listed in clang/lib/Basic/Targets/Sparc.cpp, which need to be listed here, also. Separately, I think it'd be a good idea to refactor to put this info into the SparcCPUInfo struct, so that it's harder for them to get out of sync.

[PATCH] D47137: [Sparc] Add floating-point register names

2018-05-21 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. Can you add a test that we support this? clang/test/CodeGen/sparcv8-inline-asm.c would be a good place to add a test case similar to that in clang/test/CodeGen/aarch64-inline-asm.c : test_gcc_registers. Repository: rC Clang https://reviews.llvm.org/D47137

[PATCH] D47138: [Sparc] Use the leon arch for Leon3's when using an external assembler

2018-05-22 Thread James Y Knight via Phabricator via cfe-commits
jyknight accepted this revision. jyknight added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D47138#1107637, @dcederman wrote: > I did not find a good way to access the SparcCPUInfo struct from here. No > other arch under Toolchains seems to access

[PATCH] D47137: [Sparc] Add floating-point register names

2018-05-23 Thread James Y Knight via Phabricator via cfe-commits
jyknight added inline comments. Comment at: lib/Basic/Targets/Sparc.cpp:32 +"f22", "f23", "f24", "f25", "f26", "f27", "f28", "f29", "f30", "f31", "f32", +"f33", "f34", "f35", "f36", "f37", "f38", "f39", "f40", "f41", "f42", "f43", +"f44", "f45", "f46", "f47",

[PATCH] D47894: [clang]: Add support for "-fno-delete-null-pointer-checks"

2018-06-08 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. In https://reviews.llvm.org/D47894#1125961, @srhines wrote: > In https://reviews.llvm.org/D47894#1125728, @jyknight wrote: > > > In https://reviews.llvm.org/D47894#1125653, @efriedma wrote: > > > > > The problem would come from propagating nonnull-ness from something

[PATCH] D47894: [clang]: Add support for "-fno-delete-null-pointer-checks"

2018-06-07 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. In https://reviews.llvm.org/D47894#1125653, @efriedma wrote: > The problem would come from propagating nonnull-ness from something which > isn't inherently nonnull. For example, strlen has a nonnull argument, so > `strlen(NULL)` is UB, therefore given `int z =

[PATCH] D42742: [clangd] Use pthread instead of thread_local to support more runtimes.

2018-02-01 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. Is there some way to figure out what's going on in clang-x86_64-linux-selfhost-modules? I believe there should be no linux builders which are missing this function -- it was added to libgcc in 4.8, and we don't support older versions, so I think missing this function

[PATCH] D49927: [libc++] Exclude posix_l/strtonum fallback inclusion for newlib > 2.4

2018-07-27 Thread James Y Knight via Phabricator via cfe-commits
jyknight accepted this revision. jyknight added a comment. This revision is now accepted and ready to land. Typo in commit message? They were added to 2.5, not 2.4 (the code is right, just the comment is wrong). Repository: rCXX libc++ https://reviews.llvm.org/D49927

[PATCH] D47894: [clang]: Add support for "-fno-delete-null-pointer-checks"

2018-07-17 Thread James Y Knight via Phabricator via cfe-commits
jyknight accepted this revision. jyknight added a comment. This revision is now accepted and ready to land. Please refer to the commit for the LLVM half of the change in the commit message for this. LGTM, other than some minor suggestions for the help texts. Comment at:

[PATCH] D47894: [clang]: Add support for "-fno-delete-null-pointer-checks"

2018-07-16 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. In https://reviews.llvm.org/D47894#1158811, @manojgupta wrote: > @efriedma @jyknight Does the change match your expectations where warnings > are still generated but codeGen does not emit nonnull attribute? Yes, this seems sensible IMO. Comment

[PATCH] D52918: Emit CK_NoOp casts in C mode, not just C++.

2018-10-05 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. In https://reviews.llvm.org/D52918#1256420, @aaron.ballman wrote: > Patch is missing tests -- perhaps you could dump the AST and check the > casting notation from the output? It would appear that which casts get emitted in C mode is almost completely untested. I

[PATCH] D52939: ExprConstant: Propagate correct return values from constant evaluation.

2018-10-05 Thread James Y Knight via Phabricator via cfe-commits
jyknight created this revision. jyknight added a reviewer: rsmith. The constant evaluation now returns false whenever indicating failure would be appropriate for the requested mode, instead of returning "true" for success, and depending on the caller examining the various status variables after

[PATCH] D52918: Emit CK_NoOp casts in C mode, not just C++.

2018-10-05 Thread James Y Knight via Phabricator via cfe-commits
jyknight updated this revision to Diff 168477. jyknight added a comment. Added test. https://reviews.llvm.org/D52918 Files: clang/lib/AST/ExprConstant.cpp clang/lib/Sema/SemaExpr.cpp clang/test/Sema/c-casts.c Index: clang/test/Sema/c-casts.c

[PATCH] D52919: Emit diagnostic note when calling an invalid function declaration.

2018-10-05 Thread James Y Knight via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC343867: Emit diagnostic note when calling an invalid function declaration. (authored by jyknight, committed by ). Changed prior to commit: https://reviews.llvm.org/D52919?vs=168417=168490#toc

[PATCH] D52918: Emit CK_NoOp casts in C mode, not just C++.

2018-10-05 Thread James Y Knight via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC343892: Emit CK_NoOp casts in C mode, not just C++. (authored by jyknight, committed by ). Changed prior to commit: https://reviews.llvm.org/D52918?vs=168477=168541#toc Repository: rL LLVM

[PATCH] D52918: Emit CK_NoOp casts in C mode, not just C++.

2018-10-05 Thread James Y Knight via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL343892: Emit CK_NoOp casts in C mode, not just C++. (authored by jyknight, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D53199: Fix the behavior of clang's -w flag.

2018-10-12 Thread James Y Knight via Phabricator via cfe-commits
jyknight created this revision. jyknight added a reviewer: rsmith. It is intended to disable _all_ warnings, even those upgraded to errors via `-Werror=warningname` or `#pragma clang diagnostic error' https://reviews.llvm.org/D53199 Files: clang/lib/Basic/DiagnosticIDs.cpp

[PATCH] D52939: ExprConstant: Propagate correct return values from constant evaluation.

2018-10-14 Thread James Y Knight via Phabricator via cfe-commits
jyknight updated this revision to Diff 169641. jyknight marked 11 inline comments as done. jyknight added a comment. Address most comments. https://reviews.llvm.org/D52939 Files: clang/include/clang/AST/Expr.h clang/lib/AST/ExprConstant.cpp Index: clang/lib/AST/ExprConstant.cpp

[PATCH] D52939: ExprConstant: Propagate correct return values from constant evaluation.

2018-10-14 Thread James Y Knight via Phabricator via cfe-commits
jyknight added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:12-32 // Constant expression evaluation produces four main results: // // * A success/failure flag indicating whether constant folding was successful. //This is the 'bool' return value used by

[PATCH] D52919: Emit diagnostic note when calling an invalid function declaration.

2018-10-04 Thread James Y Knight via Phabricator via cfe-commits
jyknight created this revision. jyknight added a reviewer: rsmith. The comment said it was intentionally not emitting any diagnostic because the declaration itself was already diagnosed. However, everywhere else that wants to not emit a diagnostic without an extra note emits

[PATCH] D52918: Emit CK_NoOp casts in C mode, not just C++.

2018-10-04 Thread James Y Knight via Phabricator via cfe-commits
jyknight created this revision. jyknight added a reviewer: rsmith. Previously, it had been using CK_BitCast even for casts that only change const/restrict/volatile. Now it will use CK_Noop where appropriate. This is an alternate solution to r336746. https://reviews.llvm.org/D52918 Files:

[PATCH] D52924: Make __builtin_object_size use the EM_IgnoreSideEffects evaluation mode.

2018-10-04 Thread James Y Knight via Phabricator via cfe-commits
jyknight created this revision. jyknight added a reviewer: rsmith. And, since EM_OffsetFold is now unused, remove it. While builtin_object_size intends to ignore the presence of side-effects in its argument, the EM_OffsetFold mode was NOT configured to ignore side-effects. Rather it was

[PATCH] D52926: Rename EM_ConstantExpressionUnevaluated to EM_ConstantFoldUnevaluated, which is actually what the code is currently doing.

2018-10-04 Thread James Y Knight via Phabricator via cfe-commits
jyknight created this revision. jyknight added a reviewer: rsmith. And, configure it to explicitly request equivalent behavior to ConstantFold, instead of getting that behavior accidentally. While it seems that diagnose_if and enable_if were intended to require a constexpr argument, the code was

[PATCH] D52924: Make __builtin_object_size use the EM_IgnoreSideEffects evaluation mode.

2018-10-09 Thread James Y Knight via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC344110: ExprConstant: Make __builtin_object_size use EM_IgnoreSideEffects. (authored by jyknight, committed by ). Herald added a subscriber: kristina. Changed prior to commit:

[PATCH] D52703: Allow ifunc resolvers to accept arguments

2018-10-09 Thread James Y Knight via Phabricator via cfe-commits
jyknight accepted this revision. jyknight added a comment. This revision is now accepted and ready to land. Given that there's no technical reason for the compiler to prohibit this (it was just clang trying to diagnose a probable user-error, which turns out to not be as probable as ), this

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

2018-12-12 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. In D54355#1328557 , @void wrote: > The issue is that "`n`" is expecting an immediate value, but the result of > the ternary operator isn't calculated by the front-end, because it doesn't > "know" that the evaluation of

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

2018-12-12 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. That example cannot be expected to ever evaluate the expression as "1" -- it doesn't in GCC, nor should it in Clang. An asm constraint of "n" or "i" (but not, e.g., "nr") must require a constant expression, and evaluating the argument as a constant expression

[PATCH] D55616: Emit ASM input in a constant context

2018-12-12 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. This seems like a good start, but not complete. "n" and "i" both should require that their argument is a constant expression. For "n", it actually must be an immediate constant integer, so setRequiresImmediate() should be used there. For "i", you may use an lvalue

[PATCH] D54355: Use is.constant intrinsic for __builtin_constant_p

2018-12-11 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. In D54355#1327237 , @craig.topper wrote: > Here's the test case that we have https://reviews.llvm.org/P8123 gcc seems > to accept it at O0 Smaller test case: extern unsigned long long __sdt_unsp; void foo() {

[PATCH] D55057: [Headers] Make max_align_t match GCC's implementation.

2018-11-29 Thread James Y Knight via Phabricator via cfe-commits
jyknight added inline comments. Comment at: lib/Headers/__stddef_max_align_t.h:40 __attribute__((__aligned__(__alignof__(long double; +#ifdef __i386__ + __float128 __clang_max_align_nonce3 Can you fix clang to consistently define

[PATCH] D55057: [Headers] Make max_align_t match GCC's implementation.

2018-11-29 Thread James Y Knight via Phabricator via cfe-commits
jyknight added inline comments. Comment at: lib/Headers/__stddef_max_align_t.h:40 __attribute__((__aligned__(__alignof__(long double; +#ifdef __i386__ + __float128 __clang_max_align_nonce3 uweigand wrote: > jyknight wrote: > > Can you fix clang to

[PATCH] D55150: Emit warnings from the driver for use of -mllvm or -Xclang options.

2018-11-30 Thread James Y Knight via Phabricator via cfe-commits
jyknight created this revision. jyknight added reviewers: rsmith, chandlerc. Herald added a subscriber: arphaman. This warning, Wexperimental-driver-option, is on by default, exempt from -Werror, and may be disabled. Additionally, change the documentation to note that these options are not

[PATCH] D55150: Emit warnings from the driver for use of -mllvm or -Xclang options.

2018-12-05 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. In D55150#1318082 , @chandlerc wrote: > I think this should be `internal-driver-option` and the text updated? I don't > think these are necessarily experimental, they're internals of the compiler > implementation, and not a

[PATCH] D55150: Emit warnings from the driver for use of -mllvm or -Xclang options.

2018-12-06 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. In D55150#1321829 , @efriedma wrote: > I'm not sure that putting a warning that can be disabled really helps here; > anyone who needs the option will just disable the warning anyway, and then > users adding additional options

[PATCH] D55150: Emit warnings from the driver for use of -mllvm or -Xclang options.

2018-12-06 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. In D55150#1321759 , @george.karpenkov wrote: > Using `-Xclang` is the only way to pass options to the static analyzer, I > don't think we should warn on it. Well,, that seems unfortunate if we have the only supported

[PATCH] D54547: PTH-- Remove feature entirely-

2018-12-03 Thread James Y Knight via Phabricator via cfe-commits
jyknight accepted this revision. jyknight added a comment. This revision is now accepted and ready to land. Since you've already submitted a fix to Boost, https://github.com/boostorg/build/commit/3385fe2aa699a45e722a1013658f824b6a7c761f, I think this is fine to remove. CHANGES SINCE LAST

[PATCH] D55057: [Headers] Make max_align_t match GCC's implementation.

2018-12-03 Thread James Y Knight via Phabricator via cfe-commits
jyknight added inline comments. Comment at: lib/Headers/__stddef_max_align_t.h:40 __attribute__((__aligned__(__alignof__(long double; +#ifdef __i386__ + __float128 __clang_max_align_nonce3 EricWF wrote: > uweigand wrote: > > jyknight wrote: > > >

[PATCH] D55150: Emit warnings from the driver for use of -mllvm or -Xclang options.

2018-12-06 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. In D55150#1321046 , @kristina wrote: > Personally I'm against this type of warning as it's likely anyone using > `-mllvm` is actually intending to adjust certain behaviors across one or more > passes with a lot of switches

[PATCH] D55150: Emit warnings from the driver for use of -mllvm or -Xclang options.

2018-12-06 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. In D55150#1321705 , @kristina wrote: > > There is a cost to having people encode these flags into their build > > systems -- it can then cause issues if we ever change these internal flags. > > I do not think any Clang

[PATCH] D53199: Fix the behavior of clang's -w flag.

2019-01-10 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53199/new/ https://reviews.llvm.org/D53199 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57330: Adjust documentation for git migration.

2019-01-28 Thread James Y Knight via Phabricator via cfe-commits
jyknight created this revision. jyknight added reviewers: jlebar, rnk, mehdi_amini. Herald added subscribers: jsji, jfb, arphaman, christof, delcypher, hiraditya, nhaehnle, jvesely, nemanjai, kubamracek, arsenm. Herald added a reviewer: bollu. Herald added a reviewer: serge-sans-paille. This

[PATCH] D57330: Adjust documentation for git migration.

2019-01-28 Thread James Y Knight via Phabricator via cfe-commits
jyknight updated this revision to Diff 183891. jyknight added a comment. Fix some warnings I added. Restore TestSuiteMakefileGuide.rst, which apparently isn't 100% obsolete. (But it is incorrect, and I'm not sure exactly how to fix it, so I just left a FIXME). CHANGES SINCE LAST ACTION

[PATCH] D59509: Make static constructors + destructors minsize + cold (except for in -O0)

2019-04-08 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. Looks reasonable to me. Comment at: clang/test/CodeGen/static-attr.cpp:4 + +// WITHOUT-NOT: cold minsize noinline +// WITH: define internal void @__cxx_global_var_init() [[ATTR:#[0-9]]] lebedev.ri wrote: > This is fragile, it may have

[PATCH] D57914: [Driver] Allow enum SanitizerOrdinal to represent more than 64 different sanitizer checks, NFC.

2019-03-02 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. The intricate initialization-order workarounds apparently don't work in all build modes, so I've updated this code to have constexpr functions and initializations in r355278. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57914/new/

[PATCH] D58154: Add support for -fpermissive.

2019-03-06 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. Ah -- I now understand your concern, and managing user expectations appropriately does seem like a potential concern. I did not look too closely before into what exactly GCC categorized as "permerror" (errors that can be disabled with -fpermissive) vs what Clang

[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

2019-02-22 Thread James Y Knight via Phabricator via cfe-commits
jyknight created this revision. jyknight added a reviewer: philip.pfaffe. Herald added subscribers: cfe-commits, jdoerfert, jfb, dexonsmith, steven_wu, hiraditya, mehdi_amini. Herald added projects: clang, LLVM. Just as as llvm IR supports explicitly specifying numeric value ids for

[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

2019-02-22 Thread James Y Knight via Phabricator via cfe-commits
jyknight updated this revision to Diff 187963. jyknight added a comment. Add some wording to LangRef and clang-format. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58548/new/ https://reviews.llvm.org/D58548 Files:

[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

2019-02-22 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. In D58548#1407164 , @dexonsmith wrote: > I like this idea, and I don’t think the textual IR central is too important. > A few things: > > - Changes to the IR should always be discussed on llvm-dev. Did this already > happen?

[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

2019-02-22 Thread James Y Knight via Phabricator via cfe-commits
jyknight updated this revision to Diff 187994. jyknight marked 4 inline comments as done. jyknight added a comment. Minor tweaks per comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58548/new/ https://reviews.llvm.org/D58548 Files:

[PATCH] D58154: Add support for -fpermissive.

2019-03-05 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. The errors disabled by this feature are default-error warnings -- you can already get the same effect by using -Wno-. Why is it bad to additionally allow -fpermissive to disable them? (If we have any diagnostics which are currently default-on-warnings which should not

[PATCH] D58154: Add support for -fpermissive.

2019-03-04 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. Hm -- in GCC, -fpermissive has nothing at all to do with -pedantic/-pedantic-errors, but I suppose it should be fine to do this way. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58154/new/ https://reviews.llvm.org/D58154

[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

2019-03-18 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. Sorry, forgot to re-ping this in a timely manner. :) The discussion on mailing list concluded positively, so waiting for an LG here. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58548/new/

[PATCH] D59346: [X86] Add gcc rotate intrinsics to ia32intrin.h

2019-03-18 Thread James Y Knight via Phabricator via cfe-commits
jyknight accepted this revision. jyknight added a comment. Looks good. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59346/new/ https://reviews.llvm.org/D59346 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D58548: IR: Support parsing numeric block ids, and emit them in textual output.

2019-03-22 Thread James Y Knight via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC356789: IR: Support parsing numeric block ids, and emit them in textual output. (authored by jyknight, committed by ). Changed prior to commit: https://reviews.llvm.org/D58548?vs=187994=191913#toc

[PATCH] D59711: PR41183: Don't emit Wstrict-prototypes warning for an implicit function declaration.

2019-03-22 Thread James Y Knight via Phabricator via cfe-commits
jyknight created this revision. jyknight added reviewers: rsmith, arphaman. Herald added a project: clang. Herald added a subscriber: cfe-commits. This case should emit an -Wimplicit-function-declaration warning. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D59711 Files:

[PATCH] D57767: [opaque pointer types] Cleanup CGBuilder's Create*GEP.

2019-02-09 Thread James Y Knight via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL353629: [opaque pointer types] Cleanup CGBuilders Create*GEP. (authored by jyknight, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D58091: Customize warnings for missing built-in type

2019-02-13 Thread James Y Knight via Phabricator via cfe-commits
jyknight added a comment. I think this warning (-Wbuiltin-requires-header) doesn't really make sense as its own warning. We already have two related (on-by-default) warnings. For declarations: test.c:1:6: warning: incompatible redeclaration of library function 'exit'

[PATCH] D58120: [Builtins] Treat `bcmp` as a builtin.

2019-02-13 Thread James Y Knight via Phabricator via cfe-commits
jyknight accepted this revision. jyknight added a comment. This revision is now accepted and ready to land. Looks reasonable to me. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58120/new/ https://reviews.llvm.org/D58120

[PATCH] D57801: [opaque pointer types] Pass through function types for TLS initialization and global destructor calls.

2019-02-06 Thread James Y Knight via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC353355: [opaque pointer types] Pass through function types for TLS (authored by jyknight, committed by ). Changed prior to commit: https://reviews.llvm.org/D57801?vs=185467=185677#toc Repository: rC

[PATCH] D57804: [opaque pointer types] Make EmitCall pass Function Types to CreateCall/Invoke.

2019-02-06 Thread James Y Knight via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC353356: [opaque pointer types] Make EmitCall pass Function Types to (authored by jyknight, committed by ). Changed prior to commit: https://reviews.llvm.org/D57804?vs=185476=185678#toc Repository:

[PATCH] D57794: Fix MSVC constructor call extension after b92d290e48e9 (r353181).

2019-02-05 Thread James Y Knight via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL353246: Fix MSVC constructor call extension after b92d290e48e9 (r353181). (authored by jyknight, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to

[PATCH] D57794: Fix MSVC constructor call extension after b92d290e48e9 (r353181).

2019-02-05 Thread James Y Knight via Phabricator via cfe-commits
jyknight created this revision. jyknight added reviewers: thakis, rsmith. Herald added subscribers: cfe-commits, mstorsjo. Herald added a project: clang. The assert added to EmitCall there was triggering in Windows Chromium builds, due to a mismatch of the return type. The MSVC constructor call

[PATCH] D57801: [opaque pointer types] Pass through function types for TLS initialization and global destructor calls.

2019-02-05 Thread James Y Knight via Phabricator via cfe-commits
jyknight created this revision. jyknight added a reviewer: dblaikie. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D57801 Files: clang/lib/CodeGen/CGCXX.cpp clang/lib/CodeGen/CGCXXABI.h

[PATCH] D57804: [opaque pointer types] Make EmitCall pass Function Types to CreateCall/Invoke.

2019-02-05 Thread James Y Knight via Phabricator via cfe-commits
jyknight created this revision. jyknight added a reviewer: dblaikie. Herald added a project: clang. Herald added a subscriber: cfe-commits. Also, remove the getFunctionType() function from CGCallee, since it accesses the pointee type of the value. The only use was in EmitCall, so just inline it

[PATCH] D57450: [RISCV] Set MaxAtomicInlineWidth and MaxAtomicPromoteWidth for RV32/RV64 targets with atomics

2019-02-19 Thread James Y Knight via Phabricator via cfe-commits
jyknight added inline comments. Comment at: lib/Basic/Targets/RISCV.h:94 +if (HasA) + MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 32; + } MaxAtomicPromoteWidth is an incompatible ABI-change kind of thing. We should set that to the maximum atomic

[PATCH] D57330: Adjust documentation for git migration.

2019-01-29 Thread James Y Knight via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. jyknight marked 2 inline comments as done. Closed by commit rC352514: Adjust documentation for git migration. (authored by jyknight, committed by ). Changed prior to commit:

[PATCH] D53199: Fix the behavior of clang's -w flag.

2019-01-29 Thread James Y Knight via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL352535: Fix the behavior of clangs -w flag. (authored by jyknight, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D53199?vs=183966=184142#toc

[PATCH] D57330: Adjust documentation for git migration.

2019-01-29 Thread James Y Knight via Phabricator via cfe-commits
jyknight marked 8 inline comments as done. jyknight added a comment. In D57330#1375096 , @labath wrote: > I am not sure we should be recommending to people to place the build folder > under the llvm-project checkout. Is that how people use the monorepo

[PATCH] D53199: Fix the behavior of clang's -w flag.

2019-01-28 Thread James Y Knight via Phabricator via cfe-commits
jyknight added inline comments. Comment at: clang/lib/Basic/DiagnosticIDs.cpp:460-463 + // Honor -w: this disables all messages mapped to Warning severity, and *also* + // any diagnostics which are not Error/Fatal by default (that is, they were + // upgraded by any of the

[PATCH] D53199: Fix the behavior of clang's -w flag.

2019-01-28 Thread James Y Knight via Phabricator via cfe-commits
jyknight updated this revision to Diff 183966. jyknight marked 2 inline comments as done. jyknight added a comment. Fix to not disable remarks, reword comment, adjust implementation-of-module.m test-case. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D53199/new/

[PATCH] D57315: [opaque pointer types] Add a FunctionCallee wrapper type, and use it.

2019-01-31 Thread James Y Knight via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rC352791: [opaque pointer types] Add a FunctionCallee wrapper type, and use it. (authored by jyknight, committed by ). Herald added a subscriber: cfe-commits. Changed prior to commit:

  1   2   3   4   5   >