[PATCH] D119655: [Driver][NetBSD] -r: imply -nostdlib like GCC

joerg added a comment. I'm ambivalent about this change. To me, it falls into the category of "stop passing random ld options to the compiler driver". Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119655/new/ https://reviews.llvm.org/D119655

[PATCH] D119655: [Driver][NetBSD] -r: imply -nostdlib like GCC

joerg added a comment. Well, it doesn't work with GCC either, that's why I don't care much about this change. It just attempts to legalize a user bug (using a linker option directly as a compiler flag). But I don't care enough to object either. Repository: rG LLVM Github Monorepo CHANGES SI

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

joerg added a comment. 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 header name is not mandated by any standard. It is not in any namespace

[PATCH] D35917: [mips] Implement -muninit-const-in-rodata

joerg added a comment. I don't see any reason why zero-initialised constants should be emitted in BSS. I know that GCC does that and I just fixed bugs in that because created wrong section flags for it. So yes, I'd prefer to revert this and fix the real problem. Repository: rL LLVM https:/

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

joerg added a comment. In https://reviews.llvm.org/D34158#836026, @jyknight wrote: > In https://reviews.llvm.org/D34158#827178, @joerg wrote: > > > (2) It adds magic behavior that can make debugging more difficult. > > Partially preprocessed sources for example could be compiled with plain -c >

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

joerg added a comment. In https://reviews.llvm.org/D34158#837281, @hfinkel wrote: > In https://reviews.llvm.org/D34158#837130, @joerg wrote: > > > In https://reviews.llvm.org/D34158#836026, @jyknight wrote: > > > > > In https://reviews.llvm.org/D34158#827178, @joerg wrote: > > > > > > > (2) It ad

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

joerg added a comment. In https://reviews.llvm.org/D34158#837444, @jyknight wrote: > Now, the "/tmp/foo-XXX.sh" also has a line labeled "Driver args: " with the > original command-line on it. If I understand correctly, you then like to take > this > simpler Driver command-line, and edit it man

[PATCH] D35200: Don't use mmap on Windows

joerg added a comment. The primary reason for using mmap is not so much performance, but reduced memory foot print. https://reviews.llvm.org/D35200 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listin

[PATCH] D36764: The following functions and tests work fine for powerpc64, so enable them.

joerg added a comment. Because PPC uses the TC variant. https://reviews.llvm.org/D36764 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36764: The following functions and tests work fine for powerpc64, so enable them.

joerg added a comment. divtc3 and friends. https://reviews.llvm.org/D36764 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

joerg added inline comments. Comment at: clang/test/Driver/cc1-spawnprocess.c:1 +// RUN: env -u CLANG_SPAWN_CC1 %clang -c %s -o /dev/null +// RUN: env CLANG_SPAWN_CC1=0 %clang -c %s -o /dev/null mgorny wrote: > `env -u` is not portable. I think just going for `en

[PATCH] D87615: [clang][Driver] Force stack realignment on 32-bit Solaris/x86

joerg requested changes to this revision. joerg added a comment. This revision now requires changes to proceed. I don't think this is the right place for this at all. Look at `X86Subtarget::initSubtargetFeatures` please. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:/

[PATCH] D87615: [X86] Fix stack alignment on 32-bit Solaris/x86

joerg accepted this revision. joerg added a comment. This revision is now accepted and ready to land. I'm still curious about the source of the vptr diff, but that's a minor question, otherwise. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87

[PATCH] D91760: [Driver] Default Generic_GCC aarch64 to use -fasynchronous-unwind-tables

joerg added a comment. I have no problem with the change, but please adjust the description to take about -funwind-tables. I don't think we do async by default, at least not by design. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91760/new/ http

[PATCH] D91760: [Driver] Default Generic_GCC aarch64 to use -fasynchronous-unwind-tables

joerg added a comment. The difference is whether we promise to be instruction precise or not. I'm not sure we do or want to promise that as default. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91760/new/ https://reviews.llvm.org/D91760

[PATCH] D88712: [CGBuiltin] Respect asm labels and redefine_extname for builtins with specialized emitting

joerg added a comment. Disabling builtin handling when `asm` is enabled would be a major annoyance for NetBSD. The interaction between symbol renaming and TLI is complicated. There are cases where it would make perfect sense and others were it would be harmful. Example of the latter is the way

[PATCH] D92307: [analyzer][StdLibraryFunctionsChecker] Fix typos in summaries of mmap and mmap64

joerg added a comment. off_t is s signed type. Please fix the description. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92307/new/ https://reviews.llvm.org/D92307 ___ cfe-commits mailing list cfe-commit

[PATCH] D103184: [AArch64] handle -Wa,-march=

joerg added inline comments. Comment at: clang/lib/Driver/ToolChains/Arch/AArch64.cpp:217 + // after -march. And while only using the the value of last -march, it + // includes all the options passed via -Wa,-march. + success = true; This comment is confusing.

[PATCH] D94355: [SimplifyCFG] Add relative switch lookup tables

joerg added a comment. First of all, I find this patch to be nearly impossible to read. It seems to mix a lot of refactoring with a functional change, making it very hard to focus on the core. The main difference to the jump table logic is that the latter knows that all referenced addresses ar

[PATCH] D96070: [clang] [driver] Enable static linking to libc++

joerg added a comment. The NetBSD part is most definitely not acceptable. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96070/new/ https://reviews.llvm.org/D96070 ___ cfe-commits mailing list cfe-commits

[PATCH] D96070: [clang] [driver] Enable static linking to libc++

joerg added a comment. I can't speak for other systems, but forcing libpthread to be linked is in general not desirable as it creates a non-trivial runtime cost, especially when is not used. That's still a pretty common use case of C++. Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

[PATCH] D100118: [clang] Add support for new builtin __arithmetic_fence to control floating point optimization, and new clang option fprotect-parens

joerg added inline comments. Comment at: clang/docs/UsersManual.rst:1484 + Where unsafe floating point optimizations are enabled, this option + determines whether the optimizer honors parentheses when floating-point + expressions are evaluated. If unsafe floating point opt

[PATCH] D100118: [clang] Add support for new builtin __arithmetic_fence to control floating point optimization, and new clang option fprotect-parens

joerg added a comment. Thanks, that's better. The clang front-end option is not directly relevant for the semantic of the intrinsic, so it would be better to explicitly specify it was not being fuseable unless the operand degenerates into a single operand. Otherwise the specification sounds rea

[PATCH] D107304: [clangd][query-driver] Extract GCC version from the driver output

joerg added a comment. What do you mean with "GCC macros are correct"? Clang is *not* GCC 5 and not 1:1 compatible with GCC 5. Project that have checks like that should arrive in 2010... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107304/new/ h

[PATCH] D107304: [clangd][query-driver] Extract GCC version from the driver output

joerg added a comment. Yes, there are quite a number of small differences in builtin support and even certain macros that just invite even more trouble than this. This is IMO begging for hard to trace down errors. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[PATCH] D107304: [clangd][query-driver] Extract GCC version from the driver output

joerg added a comment. This discussion is becoming pointless. Clang is not a 1:1 replacement for later GCC versions. Easiest example is the support for `__builtin_apply` and friends. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107304/new/ https:

[PATCH] D107242: [AIX] Define __HOS_AIX__ macro

joerg added a comment. I'm puzzled by this change. I don't think we have any case so far where the compiler behavior changes with the host OS and I don't think it should. What's the point / use case of this macro? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[PATCH] D107242: [AIX] Define __HOS_AIX__ macro

joerg added a comment. clang is fundamentally a cross-compiler only. I don't see any point for having host-specific branches in this case at all. Either the macro should be specified for the target all the time or not at all, but it should most definitely not depend on the host. That's actually

[PATCH] D107825: [AIX] Define __HOS_AIX__ macro only for AIX target

joerg accepted this revision. joerg added a comment. This revision is now accepted and ready to land. LGTM. Maybe include a small hint in the commit message that xlC never shipped cross-compiling support and the difference is therefore not observable anyway. Repository: rG LLVM Github Monorep

[PATCH] D104680: [clang] Eliminate relational function pointer comparisons in all C++ modes

joerg added a comment. Is there any reason for breaking C++03 code here? I don't see the advantage to that at all. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104680/new/ https://reviews.llvm.org/D104680

[PATCH] D73245: Depend stddef.h to provide max_align_t for C++11 and provide better fallback in

joerg marked 5 inline comments as done. joerg added inline comments. Comment at: libcxx/include/stddef.h:58 !defined(__DEFINED_max_align_t) && !defined(__NetBSD__) typedef long double max_align_t; #endif ldionne wrote: > Why do we need this at all anymore?

[PATCH] D73245: Depend stddef.h to provide max_align_t for C++11 and provide better fallback in

joerg marked 2 inline comments as done. joerg added inline comments. Comment at: libcxx/test/std/utilities/meta/meta.trans/meta.trans.other/aligned_storage.pass.cpp:275 +#if TEST_STD_VER >= 11 +const size_t alignment = TEST_ALIGNOF(std::max_align_t) > 16 ? +16 : TEST

[PATCH] D73245: Depend stddef.h to provide max_align_t for C++11 and provide better fallback in

joerg added a comment. Herald added a subscriber: broadwaylamb. Ping? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73245/new/ https://reviews.llvm.org/D73245 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cg

[PATCH] D73245: Depend stddef.h to provide max_align_t for C++11 and provide better fallback in

joerg added a comment. Ping2? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73245/new/ https://reviews.llvm.org/D73245 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73245: Depend stddef.h to provide max_align_t for C++11 and provide better fallback in

joerg added a comment. It is used both in `` and `` and the use in the latter is currently unconditional AFAICT. I don't have a problem splitting the conditional to avoid the typedef. That would address the ODR concern? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73245/new/ https://

[PATCH] D73245: Depend stddef.h to provide max_align_t for C++11 and provide better fallback in

joerg marked an inline comment as done. joerg added inline comments. Comment at: libcxx/include/new:229-237 +#if !defined(_LIBCPP_CXX03_LANG) +using __libcpp_max_align_t = max_align_t; +#else +union __libcpp_max_align_t { + void * __f1; + long long int __f2; + long double __f3

[PATCH] D73245: Depend stddef.h to provide max_align_t for C++11 and provide better fallback in

joerg updated this revision to Diff 245853. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73245/new/ https://reviews.llvm.org/D73245 Files: libcxx/include/cstddef libcxx/include/new libcxx/include/stddef.h Index: libcxx/include/stddef.h ===

[PATCH] D73245: Depend stddef.h to provide max_align_t for C++11 and provide better fallback in

joerg updated this revision to Diff 246524. joerg added a comment. Do not depend on max_align_t in C++03 mode in the test cases. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73245/new/ https://reviews.llvm.org/D73245 Files: libcxx/include/cstddef libcxx/include/new libcxx/includ

[PATCH] D73245: Depend stddef.h to provide max_align_t for C++11 and provide better fallback in

joerg added a comment. libc++ has no idea what a correct max_align_t is. The internal definition works due to historic requirements that all three fundamental types are supported for new/delete, but we don't have any such guarantees for every other context. A correctly implemented stddef.h does

[PATCH] D70416: [Driver] Make -static-libgcc imply static libunwind

joerg added a comment. This is normally done by using `-Bstatic`/`-Bdynamic` around the library. See `tools::addOpenMPRuntime`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70416/new/ https://reviews.llvm.org/D70416 ___

[PATCH] D73245: Depend stddef.h to provide max_align_t for C++11 and provide better fallback in

joerg added a comment. Ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73245/new/ https://reviews.llvm.org/D73245 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73245: Depend stddef.h to provide max_align_t for C++11 and provide better fallback in

joerg added a comment. In D73245#1918287 , @ldionne wrote: > In D73245#1918148 , @EricWF wrote: > > > I've already stated my disapproval of this patch. Libc++ has never and will > > never provide nor value C++03 co

[PATCH] D73245: Depend stddef.h to provide max_align_t for C++11 and provide better fallback in

joerg added inline comments. Comment at: libcxx/include/new:240 return __align > __STDCPP_DEFAULT_NEW_ALIGNMENT__; +#elif defined(_LIBCPP_CXX03_LANG) + return __align > alignment_of<__libcpp_max_align_t>::value; ldionne wrote: > So, IIUC what you're saying, `

[PATCH] D76186: Fix compatibility for __builtin_stdarg_start

joerg created this revision. The `__builtin_stdarg_start` is the legacy spelling of `__builtin_va_start`. It should behave exactly the same, but for the last 9 years it would behave subtly different for diagnostics. Follow the change from 29ad95b23217 to require custom type checking. https://

[PATCH] D73245: Depend stddef.h to provide max_align_t for C++11 and provide better fallback in

joerg updated this revision to Diff 250937. joerg edited the summary of this revision. joerg added a comment. Require `__STDCPP_NEW_ALIGNMENT__` in C++03 mode. Prefer it over `max_align_t` in a number of tests when allocation alignment is desired. Adjust some tests to do minimal sanity checking

[PATCH] D76186: Fix compatibility for __builtin_stdarg_start

joerg closed this revision. joerg added a comment. Committed as 0b999f76575f0196d3cd01c0781b2513b0a1af15 without link. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76186/new/ https://reviews.llvm.org/D76186 ___ cfe-commits mailing list cfe

[PATCH] D73245: Depend stddef.h to provide max_align_t for C++11 and provide better fallback in

joerg updated this revision to Diff 242317. joerg retitled this revision from "Don't define std::max_align_t if not used in C++03 mode" to "Depend stddef.h to provide max_align_t for C++11 and provide better fallback in ". joerg edited the summary of this revision. Herald added a subscriber: kryt

[PATCH] D73245: Depend stddef.h to provide max_align_t for C++11 and provide better fallback in

joerg added a comment. Ping? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73245/new/ https://reviews.llvm.org/D73245 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72869: Add __warn_memset_zero_len builtin as a workaround for glibc issue

joerg added a comment. Can this be restricted to Linux? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72869/new/ https://reviews.llvm.org/D72869 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D73245: Don't define std::max_align_t if not used in C++03 mode

joerg created this revision. joerg added a reviewer: mclow.lists. Herald added a subscriber: christof. max_align_t has been introduced by C++11 and C99. When an older language mode is explicitly requested, the system headers might not provide. Don't define it in this case unless other headers ()

[PATCH] D73245: Don't define std::max_align_t if not used in C++03 mode

joerg added a comment. libc++ generally tries to play nice with C++03 code. It doesn't go out of its way to break it and keeping it usable helps dealing with a lot of rusty old code. That's what the patch is all about, not breaking things for no good reason. CHANGES SINCE LAST ACTION https:

[PATCH] D73245: Don't define std::max_align_t if not used in C++03 mode

joerg added a comment. Let me clarify the situation for a moment: (1) libc++ does try to work in C++03 mode. See the separate implementation of for pre-C++11. It is also desirable to support. This is completely beside the question of TR1 support. (2) The only reason why max_align_t is currently

[PATCH] D44774: [Driver] Allow use of -fsyntax-only together with -MJ

joerg added a comment. IMO we should explicitly error out. That combination is nonsense to me. Creating useless JSON database fragments is not an improvement. Repository: rC Clang https://reviews.llvm.org/D44774 ___ cfe-commits mailing list cfe-

[PATCH] D44774: [Driver] Allow use of -fsyntax-only together with -MJ

joerg added a comment. Oh, we certainly should never be hitting an assertion on front-end flags. As such, there is a problem to fix here. I still maintain that the combination of flags is non-sense, so the question is: (1) Should be silently ignore -MJ? That would be useful for your use case, b

[PATCH] D44921: [PowerPC] Option for secure plt mode

joerg added a comment. GCC supports -mbss-plt to get the legacy behavior. Not sure if anyone actually uses it though. https://reviews.llvm.org/D44921 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/list

[PATCH] D44494: [libunwind] Support __register_frame with a full .eh_frame section

joerg added a comment. The "struct object" is an implementation detail of the unwind implementation. You are guaranteed historically to get at least 8 longs / 8 pointers for internal use statically allocated in each object. What is stored inside is up to the unwind implementation. https://rev

[PATCH] D45240: [ARM] Compute a target feature which corresponds to the ARM version.

joerg added a comment. Can you make sure that we handle the older ARM versions correctly as well, i.e. v4, v5 and v6? I take it we still have test cases for the arm <-> thumb transition? That's the one part of the triple logic that is really non-trivial. Repository: rC Clang https://reviews

[PATCH] D148975: -fdebug-prefix-map=: make the last win when multiple prefixes match

joerg added a comment. For me long matching prefix makes more sense, but if the same prefix is used multiple times, the last option should win. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148975/new/ https://reviews.llvm.org/D148975 ___

[PATCH] D26564: Use PIC relocation mode by default for PowerPC64 ELF

This revision was automatically updated to reflect the committed changes. Closed by commit rL289744: Use PIC relocation mode by default for PowerPC64 ELF (authored by joerg). Changed prior to commit: https://reviews.llvm.org/D26564?vs=77670&id=81495#toc Repository: rL LLVM https://reviews.l

[PATCH] D28047: Remove the '-disable-llvm-passes' flag (which I didn't even know existed, and I suspect many others aren't aware of either) and strength '-disable-llvm-optzns' to do the same thing.

joerg added a comment. There are two issues I have with the flags right now. The first one you are addressing with this patch. The second issue is they remain CC1-only flags. Can we introduce a proper "-emit-raw-llvm" flag to work like "-emit-llvm" but without the additional IR processing. We c

[PATCH] D31992: [clangd] Escape only necessary characters in JSON output

joerg added a comment. I'm strongly against this patch. Can you give an actual test case for the problematic behavior? Repository: rL LLVM https://reviews.llvm.org/D31992 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.

[PATCH] D31992: [clangd] Escape only necessary characters in JSON output

joerg added a comment. In https://reviews.llvm.org/D31992#725963, @klimek wrote: > If I understand correctly, that's " and \ for JSON and ", \ and all > non-printable characters (which unfortunately requires understanding of > unicode to solve this fully correctly) in YAML. I'd modify that sl

[PATCH] D31992: [clangd] Escape only necessary characters in JSON output

joerg added a comment. The short version is perfectly fine as long as works for both JSON and YAML. Less output is always good :) https://reviews.llvm.org/D31992 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/

[PATCH] D31992: [clangd] Escape only necessary characters in JSON output

joerg added a comment. Just to avoid any confusion: this should be the generic YAML escape routine in llvm/lib/Support, i.e. IMO we don't want to have separate YAML and JSON escape routines. Effective, change YAMLParser.cpp line 697 to use \u and drop the whole UTF-8 handling part. https://re

[PATCH] D31992: [clangd] Escape only necessary characters in JSON output

joerg added a comment. We already have a couple of case that expect the encoding to be compatible. I'm not very attached to the additional special cases from YAML, but having either a common escape function OR a JSON escape in LLVM/Support is quite important. https://reviews.llvm.org/D31992

[PATCH] D29032: [mips] Define macros related to -mabicalls in the preprocessor

joerg added a comment. I'm not terribly attached to using __ABICALLS__ for NetBSD, but let me check back with some of the MIPS folks. I would prefer __mips_abicalls to be always defined though, independent of the historic behavior. https://reviews.llvm.org/D29032 ___

[PATCH] D29032: [mips] Define macros related to -mabicalls in the preprocessor

joerg added a comment. Always defining __mips_abicalls is certainly the saner choice. I'm discussing with the NetBSD/MIPS guys whether we just want to go with it all the time and drop our custom changes for __ABICALLS__. Chances are quite good that we are going in that direction. https://revi

[PATCH] D20561: Warn when taking address of packed member

joerg added a comment. It is not true that all false positives have been fixed, some of the more complex cases involving nested data structures are still open. @royger: Your example is missing explicit alignment. packed has two side effects: remove internal padding and set the alignment to 1. T

[PATCH] D20561: Warn when taking address of packed member

joerg added a comment. The last review left out the case of naturally aligned packed members, IIRC. I have to go over the warning list in NetBSD again, but I'm moderately sure it is not fixed. The better example is: struct __attribute__((__packed__)) bar { uint16_t x1; uint16_t x

[PATCH] D20561: Warn when taking address of packed member

joerg added a comment. Yes, the project is interested on reducing the number of false positives. The example you gave is *not* a FP, but exactly the kind of situation the warning is supposed to trigger on. Repository: rL LLVM https://reviews.llvm.org/D20561 __

[PATCH] D29542: [TargetInfo] Adjust x86-32 atomic support to the CPU used

joerg added a comment. At this point, I don't think there is any use on pretending that i386-as-default makes sense. So I would request that the i386 case should be made explicit or just dropped, with a preference for the latter. https://reviews.llvm.org/D29542 _

[PATCH] D29542: [TargetInfo] Adjust x86-32 atomic support to the CPU used

joerg added a comment. Generic should imply i486+. I don't think any general purpose system supports i386 at this point, simply because it has an annoying number of bugs in critical components. The i486 (esp. the non-crippled ones) are reasonable easy to support and there are still people aroun

[PATCH] D29630: [libcxx] Threading support: externalize sleep_for()

joerg added inline comments. Comment at: include/__threading_support:367 + ts.tv_sec = ts_sec_max; + ts.tv_nsec = giga::num - 1; + } I don't think giga::num makes things any clear compared to just spelling it out. Comment at: include/

[PATCH] D29630: [libcxx] Threading support: externalize sleep_for()

joerg accepted this revision. joerg added a comment. One small issue left, otherwise LGTM. Comment at: include/__threading_support:187 +_LIBCPP_THREAD_ABI_VISIBILITY +void __libcpp_thread_sleep_for(const chrono::nanoseconds& ns); + Drop the name here. https:/

[PATCH] D34790: [NewPM] Add a flag -fexperimental-new-pass-manager=on/off/debug for printing debug output.

joerg added a comment. As I said in the discussion about similar flags for GVN, I'm generally fine with. We should have a big fat warning in the Release Notes that all -fexperimental-* flags are exactly that -- temporary and not intended for long term consumption. I.e. "we can and will remove t

[PATCH] D34926: Deprecation flag group + use for BB vectorizer options

joerg created this revision. Herald added a subscriber: rengolin. As discussed on IRC when https://reviews.llvm.org/D34846 came up, removing flags is problematic from a UX experience. This change introduces a new option group similar to the group GCC optimizer flags like -fexpensive-optimizatio

[PATCH] D34926: Deprecation flag group + use for BB vectorizer options

This revision was automatically updated to reflect the committed changes. Closed by commit rL306965: Add an option group for deprecated warnings. Add the removed (authored by joerg). Changed prior to commit: https://reviews.llvm.org/D34926?vs=104998&id=105004#toc Repository: rL LLVM https:/

[PATCH] D35190: __builtin_constant_p should consider the parameter of a constexpr function as constant

joerg added a comment. What if the constexpr function is called with in a non-constexpr context, e.g. with a random global variable as argument? https://reviews.llvm.org/D35190 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.ll

[PATCH] D35462: Also add the option -no-pie (like -nopie)

joerg accepted this revision. joerg added a comment. This revision is now accepted and ready to land. Please mark it as alias for -nopie, otherwise fine. https://reviews.llvm.org/D35462 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://

[PATCH] D35577: Add -flookup-tables and -fno-lookup-tables flags

joerg added inline comments. Comment at: test/CodeGen/nouselookuptable.c:1 +// RUN: %clang_cc1 -S -fno-lookup-tables %s -emit-llvm -o - | FileCheck %s + Check positive flag and the default case as well. Comment at: test/CodeGen/nouselookuptable

[PATCH] D35780: Introduce -nostdlib++ flag to disable linking the C++ standard library.

joerg added a comment. I don't really like this. The reason why -lm is added explicitly on many targets is because the C++ STL typically depends on it and that means for static linking and broken ELF linkers, it will be necessary to link against it explicitly. There is also the question on whet

[PATCH] D28543: Elliminates uninitialized warning for volitile varibles.

joerg added a comment. Please fix the spelling errors in the titel / summary before commit. I somewhat agree with Hal -- I think this is too aggressive. Common use cases for local volatile include atomic ops or returns-twice functions like setjmp/longjmp. Disabling the warning in those cases ha

[PATCH] D29843: [CodeGen] Treat auto-generated __dso_handle symbol as HiddenVisibility

joerg requested changes to this revision. joerg added inline comments. This revision now requires changes to proceed. Comment at: lib/CodeGen/CodeGenModule.cpp:2402 + return GetOrCreateLLVMGlobal(Name, llvm::PointerType::getUnqual(Ty), nullptr, + No

[PATCH] D29032: [mips] Define macros related to -mabicalls in the preprocessor

joerg added a comment. No need to preserve the BSD behavior for NetBSD. https://reviews.llvm.org/D29032 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30025: [compiler-rt] [builtins] Fix building atomic.c with GCC

joerg added a comment. Note that the normal compiler-rt functions have a different name than the builtins they provide, at least from the C frontend view. Repository: rL LLVM https://reviews.llvm.org/D30025 ___ cfe-commits mailing list cfe-commi

[PATCH] D30733: [Driver] Add arch-specific rpath for libc++

joerg added a comment. Installing libc++ along clang makes a lot of sense for non-system compilers. It doesn't imply any version locking like in GCC, i.e. you are still free to install whatever version of libunwind and libc++ you want. I'm somewhat conflicted about this patch. I think it is an

[PATCH] D27138: Extend CompilationDatabase by a field for the output filename

joerg created this revision. joerg added a reviewer: klimek. joerg added a subscriber: cfe-commits. joerg set the repository for this revision to rL LLVM. In bigger projects like an Operating System, the same source code is often compiled in slightly different ways. This could be the difference b

[PATCH] D27140: Allow clang to write compilation database records

joerg created this revision. joerg added reviewers: klimek, rsmith. joerg added a subscriber: cfe-commits. joerg set the repository for this revision to rL LLVM. joerg added a dependency: D27138: Extend CompilationDatabase by a field for the output filename. When integrating compilation database

[PATCH] D27138: Extend CompilationDatabase by a field for the output filename

joerg added inline comments. Comment at: lib/Tooling/JSONCompilationDatabase.cpp:266 +nodeToCommandLine(Syntax, std::get<2>(CommandsRef[I])), +Output ? Output->getValue(OutputStorage) : ""); } klimek wrote: > Optional: I'd probably let the node

[PATCH] D27138: Extend CompilationDatabase by a field for the output filename

joerg added inline comments. Comment at: lib/Tooling/JSONCompilationDatabase.cpp:266 +nodeToCommandLine(Syntax, std::get<2>(CommandsRef[I])), +Output ? Output->getValue(OutputStorage) : ""); } klimek wrote: > joerg wrote: > > klimek wrote: > >

[PATCH] D27138: Extend CompilationDatabase by a field for the output filename

joerg added inline comments. Comment at: lib/Tooling/JSONCompilationDatabase.cpp:266 +nodeToCommandLine(Syntax, std::get<2>(CommandsRef[I])), +Output ? Output->getValue(OutputStorage) : ""); } klimek wrote: > joerg wrote: > > klimek wrote: > >

[PATCH] D27140: Allow clang to write compilation database records

joerg removed rL LLVM as the repository for this revision. joerg updated this revision to Diff 79423. joerg added a comment. Use llvm::yaml::escape. https://reviews.llvm.org/D27140 Files: include/clang/Driver/Options.td lib/Driver/Tools.cpp Index: include/clang/Driver/Options.td ==

[PATCH] D27138: Extend CompilationDatabase by a field for the output filename

joerg added a comment. Can I interprete that as LGTM otherwise? Repository: rL LLVM https://reviews.llvm.org/D27138 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27140: Allow clang to write compilation database records

joerg updated this revision to Diff 79482. joerg added a comment. Move implementation into the Clang class, keep track of the raw_fd_stream. This avoids reopening it on multiple inputs and removes the need for append mode. Short circuit the function when -### is present. Add proper diagnostics o

[PATCH] D27138: Extend CompilationDatabase by a field for the output filename

joerg added a comment. It's not the directory, but the output file. That's optional since it is a new addition and I don't want to invalidate all existing JSON databases. Repository: rL LLVM https://reviews.llvm.org/D27138 ___ cfe-commits mailin

[PATCH] D27138: Extend CompilationDatabase by a field for the output filename

joerg added a comment. Which struct are we talking about, `CompileCommandRef` or `CompileCommand`? It is a pointer in the former and a plain StringRef in the latter. I don't think making it a pointer in both is an advantage, i.e. distinguishing empty input from missing field is not valuable in

[PATCH] D27066: Fix crash with unsupported architectures in Linux/Gnu target triples.

joerg accepted this revision. joerg added a reviewer: joerg. joerg added a comment. LGTM with a small cleanup. Comment at: lib/Driver/Tools.cpp:10018 + const char *LDMOption = getLDMOption(ToolChain.getTriple(), Args); + if (!LDMOption) { +D.Diag(diag::err_target_unknown_

[PATCH] D27304: [PATCH] [Sema][X86] Don't allow floating-point return types when SSE is disabled

joerg added a comment. I think this is the absolutely wrong place to put such logic. It really can not be anywhere but the backend. https://reviews.llvm.org/D27304 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bi

[PATCH] D27304: [PATCH] [Sema][X86] Don't allow floating-point return types when SSE is disabled

joerg added a comment. The frontend is the wrong place as it doesn't even know if the register is ever going to be used. E.g. if it is a static function, all instances could be inlined away. https://reviews.llvm.org/D27304 ___ cfe-commits mailing

[PATCH] D27138: Extend CompilationDatabase by a field for the output filename

This revision was automatically updated to reflect the committed changes. Closed by commit rL288436: Extend CompilationDatabase by a field for the output filename (authored by joerg). Changed prior to commit: https://reviews.llvm.org/D27138?vs=79320&id=79993#toc Repository: rL LLVM https://

<    1   2   3   >