[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] D123345: Treat `std::move`, `forward`, and `move_if_noexcept` as builtins.

joerg added a comment. The patch contains at least one user visible change that would be quite surprising: it is no longer possible to intentionally set a break point on `std::move`. Thinking more about it, what about a slightly different implementation strategy? Provide a compiler built-in `_

[PATCH] D123345: Treat `std::move`, `forward`, and `move_if_noexcept` as builtins.

joerg added a comment. As is, I think this conflicts with `-ffreestanding` assumptions or at the very least the spirit. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123345/new/ https://reviews.llvm.org/D123345 ___

[PATCH] D122077: [InstCombine] Fold (ctpop(X) == 1) | (X == 0) into ctpop(X) < 2

joerg added a comment. Why is this fold preferable to `(X & (X-1)) == 0`? At least on all architectures without native population count, the binary-and based test is preferable and it might even be better with it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[PATCH] D121512: [Support] Change zlib::compress to return void

joerg added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121512/new/ https://reviews.llvm.org/D121512 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-b

[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] 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] D118021: [Driver] Use libatomic for 32-bit SPARC atomics support

joerg added inline comments. Comment at: clang/lib/Driver/ToolChains/Solaris.cpp:135 } +// LLVM lacks atomics support on 32-bit SPARC, so forcibly link with +// libatomic as a workaround. This comment is misleading. It's not so much that LLVM doesn't

[PATCH] D116088: [compiler-rt] Implement ARM atomic operations for architectures without SMP support

joerg added a comment. Correct me if I'm wrong, but I don't think this stubs are async signal safe nor will they work for preemptive multitasking systems? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116088/new/ https://reviews.llvm.org/D116088

[PATCH] D116882: [docs] [clang] Small documentation change for compilation databases

joerg accepted this revision. joerg added inline comments. This revision is now accepted and ready to land. Comment at: clang/docs/JSONCompilationDatabase.rst:33 +Clang has the ablity to generate compilation database fragments via +the :option:`-MJ argument >`. You can contantina

[PATCH] D113372: [Driver] Add CLANG_DEFAULT_PIE_ON_LINUX to emulate GCC --enable-default-pie

joerg added a comment. Last update introduced a lot of unrelated changes? But the actual intended change seems fine now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113372/new/ https://reviews.llvm.org/D113372 __

[PATCH] D113372: [Driver] Add CLANG_DEFAULT_PIE to emulate GCC --enable-default-pie

joerg added inline comments. Comment at: clang/CMakeLists.txt:230 +option(CLANG_DEFAULT_PIE "Default to -fPIE and -pie (Linux only)" OFF) +if(CLANG_DEFAULT_PIE) This option should really be called something like CLANG_DEFAULT_PIE_ON_LINUX or so. It should be e

[PATCH] D114564: Fix the use of -fno-approx-func along with -Ofast or -ffast-math

joerg added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2760 case options::OPT_fno_honor_nans: HonorNaNs = false;break; case options::OPT_fapprox_func: ApproxFunc = true;break; case options::OPT_fno_approx_func:

[PATCH] D112413: [X86] Add -mskip-rax-setup support to align with GCC

joerg added inline comments. Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:99 +cl::desc("Skips setting up the RAX register when SSE is disabled and there " + "are no variable arguments passed in vector registers."), +cl::Hidden); The de

[PATCH] D111863: [libunwind] Add an interface for dynamic .eh_frame registration

joerg added a comment. In D111863#3072023 , @housel wrote: > It's also worth noting that FreeBSD's version of libgcc exception handling is > actually based on the libunwind code, with a local patch >

[PATCH] D111863: [libunwind] Add an interface for dynamic .eh_frame registration

joerg added a comment. In D111863#3071651 , @lhames wrote: > In D111863#3071353 , @joerg wrote: > >> I would strongly prefer if ORCv2 doesn't depend on this. It essentially >> forces libunwind to parse the whole

[PATCH] D111863: [libunwind] Add an interface for dynamic .eh_frame registration

joerg added a comment. I would strongly prefer if ORCv2 doesn't depend on this. It essentially forces libunwind to parse the whole section just to find the delimiters of the FDEs. That's a lot of unnecessary work as JIT use normally allows registering functions individually. Repository: rG

[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] 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] 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] 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] 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. 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] D106577: [clang] Define __STDC_ISO_10646__

joerg added a comment. > I still don't fully understand the original comment from Joerg. The encoding > of `L'a'` cannot change at runtime; it's a literal whose encoding is decided > entirely at compile time. @joerg -- did you mean that Clang produces a > different literal encoding depending on

[PATCH] D106577: [clang] Define __STDC_ISO_10646__

joerg added a comment. In D106577#2899715 , @aaron.ballman wrote: > In D106577#2899711 , @joerg wrote: > >> This patch is certainly wrong for NetBSD as the wchar_t encoding is up to >> the specific locale charse

[PATCH] D106577: [clang] Define __STDC_ISO_10646__

joerg added a comment. This patch is certainly wrong for NetBSD as the wchar_t encoding is up to the specific locale charset and *not* UCS-2 or UCS-4 for certain legacy encodings like the various shift encodings in East Asia. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

[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] 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] 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] 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] D101122: introduce flag -fsanitize-address-detect-stack-use-after-return-mode. No functional change.

joerg added a comment. There are two approaches that work for reviewing: starting from clang and going down or starting from compiler-rt and going up the layers. I'd prefer to do the latter as it means meaningful testing can be done easier. There are two natural steps here: clang flag+IR genera

[PATCH] D101122: introduce flag -fsanitize-address-detect-stack-use-after-return-mode. No functional change.

joerg added a comment. This review doesn't make sense to me. Why add a flag that isn't affecting anything? Why add a flag that isn't even tested? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101122/new/ https://reviews.llvm.org/D101122 _

[PATCH] D93031: Enable fexec-charset option

joerg added a comment. "Keeping the original spelling around" would assume that the input is not using a stateful encoding. That seems worse as assumption than giving the canonical output in UTF-8 and shifting the problem to the user's editor? Repository: rG LLVM Github Monorepo CHANGES SIN

[PATCH] D100054: Handle flags such as -m32 when computing the triple prefix for programs

joerg added a comment. This sounds wrong. If you are using 'x86_64-freebsd' as triple and -m32, it should still call 'x86_64-freebsd-ld', but it is the responsibility of the driver to ensure that also the right set of linker flags are passed as well. Compare `netbsd::Linker::ConstructJob` for o

[PATCH] D98574: [Sparc] Define the same macros for -mcpu=v9 as GCC on Linux and the BSDs

joerg added a comment. The NetBSD part looks ok, but also lacks proper testing. I'm not sure anyone but Linux cares at all otherwise as they lack 32bit SPARC support. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98574/new/ https://reviews.llvm.or

[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] 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] 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] 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] 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] 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] 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] 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] 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] D80300: [Driver] Add DEFAULT_DYLD_PREFIX and DEFAULT_RPATH to complement DEFAULT_SYSROOT

joerg added a comment. I don't agree with the justification at all, but it also seems that noone else cares about the build option creep here. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80300/new/ https://reviews.llvm.org/D80300

[PATCH] D80300: [Driver] Add DEFAULT_DYLD_PREFIX and DEFAULT_RPATH to complement DEFAULT_SYSROOT

joerg added a comment. I see that more as a short-coming in the existing DEFAULT_SYSROOT behavior and less an argument for making more cases like it. So the general idea is that for turnkey toolchains, we want to allow customizing the "default" target of the toolchain to hard-code options like

[PATCH] D80300: [Driver] Add DEFAULT_DYLD_PREFIX and DEFAULT_RPATH to complement DEFAULT_SYSROOT

joerg added a comment. I do agree with the feature request, but I'm not sure about the implementation. It doesn't seem to work well with the cross-compiling support in the driver as clearly shown by the amount of tests that need patching. Is cross-compiling a concern for you at all? Otherwise I

[PATCH] D77697: libc++: adjust modulemap for non-modular C

joerg added a comment. @ldionne I was updating libc++ from d42baff45d9700a199982ba0ac04dbc6c6d911bb and LLVM itself from 38aebe5c04ab4cb3695dc1bcc60b9a7b55215aff

[PATCH] D77697: libc++: adjust modulemap for non-modular C

joerg added a comment. This fixes the module build of clang for me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77697/new/ https://reviews.llvm.org/D77697 ___ cfe-commits mailing list cfe-commits@lis

[PATCH] D73245: Avoid using std::max_align_t in pre-C++11 mode

This revision was not accepted when it landed; it landed in state "Needs Revision". This revision was automatically updated to reflect the committed changes. Closed by commit rG98f77828a98f: Avoid using std::max_align_t in pre-C++11 mode (authored by joerg). Herald added a project: libc++. Herald

[PATCH] D73245: Avoid using std::max_align_t in pre-C++11 mode

joerg added a comment. ping2 Louis, did I answer your questions? 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-bi

[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 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 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] 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 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 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 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] 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 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. 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] 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 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 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 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 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. 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 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: 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] 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 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] 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] 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] 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] D67638: Improve __builtin_constant_p lowering

This revision was automatically updated to reflect the committed changes. Closed by commit rG529f4ed401ea: Improve __builtin_constant_p lowering (authored by joerg). Changed prior to commit: https://reviews.llvm.org/D67638?vs=220398&id=224796#toc Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D68410: [AttrDocs] document always_inline

joerg added a comment. I wonder if we should actually enumerate evil here, i.e. give the situations in which inlining actually fails. As mentioned on IRC, I wonder if we shouldn't aim for the stronger semantics and at least warn by default of any situation that prevents always_inline from doing

[PATCH] D67638: Improve __builtin_constant_p lowering

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

[PATCH] D67638: Improve __builtin_constant_p lowering

joerg created this revision. joerg added a reviewer: rsmith. Herald added subscribers: kristina, kbarton, nemanjai. Herald added a project: clang. Herald added a subscriber: wuzish. __builtin_constant_p used to be short-cut evaluated to false when building with -O0. This is undesirable as it mean

[PATCH] D60943: Delay diagnosing asm constraints that require immediates until after inlining

joerg added a comment. The combination of D60942 , D06943 and D65280 solves the problems for me on all targets I have. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60943/new/ https://reviews.ll

[PATCH] D60943: Delay diagnosing asm constraints that require immediates until after inlining

joerg added a comment. I understand, but the current version just doesn't work anyway to delay it. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60943/new/ https://reviews.llvm.org/D60943 ___ cfe-commits mailing list

[PATCH] D60943: Delay diagnosing asm constraints that require immediates until after inlining

joerg added a comment. You lost the changes to lib/Sema/SemaStmtAsm.cpp to actually do the delaying for immediate operands? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60943/new/ https://reviews.llvm.org/D60943 __

[PATCH] D59744: Fix i386 ABI "__m64" type bug

joerg added a comment. I think MMX code is obscure enough at this point that it doesn't matter much either way. Even less across DSO boundaries. That's why I don't really care either way. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59744/new/ https://reviews.llvm.org/D59744 _

[PATCH] D60748: Fix i386 struct and union parameter alignment

joerg added inline comments. Comment at: lib/CodeGen/TargetInfo.cpp:1501 +// +// Exclude other System V OS (e.g Darwin, PS4 and FreeBSD) since we don't +// want to spend any effort dealing with the ramifications of ABI breaks. krytarowski wrote: > Dar

[PATCH] D60943: Delay diagnosing "n" constraint until after inlining

joerg added a comment. I'm in the process of testing this, but feedback will take a bit. On the more meaty parts of this change, I think further iterations will be necessary in-tree to extend this to the other constraints. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D59118: creduce script for clang crashes

joerg added a comment. For it to be really useful for the majority of bugs, it would be nice to figure out automatically how to get the preprocessing step done and filter out the # lines afterwards. That part alone significantly cuts down the creduce time. CHANGES SINCE LAST ACTION https://r

[PATCH] D58821: Inline asm constraints: allow ICE-like pointers for the "n" constraint (PR40890)

joerg added a comment. Well, that was a sample to illustrate the point. A full working (and now failing) example is: static inline void outl(unsigned port, unsigned data) { if (__builtin_constant_p(port) && port < 0x100) { __asm volatile("outl %0,%w1" : : "a"(data), "n"(port)); }

[PATCH] D58821: Inline asm constraints: allow ICE-like pointers for the "n" constraint (PR40890)

joerg added a comment. The other problem is that we don't use the CFG machinery to prune dead branches. Consider the x86 in/out instructions: one variant takes an immediate, the other a register. The classic way to deal with that is something like static inline void outl(unsigned port, uint32

[PATCH] D58821: Inline asm constraints: allow ICE-like pointers for the "n" constraint (PR40890)

joerg added a comment. Can you include a patch for something like (int *)0xdeadbeef on amd64? That's a valid value for "n", but clearly too large for int. Thanks for looking at this, it is one of the two large remaining show stoppers for the asm constraint check. CHANGES SINCE LAST ACTION

[PATCH] D58649: Fix inline assembler constraint validation

joerg marked an inline comment as done. joerg added inline comments. Comment at: include/clang/Basic/TargetInfo.h:860 + if (!ImmSet.empty()) +return ImmSet.count(Value.getZExtValue()) != 0; + return !ImmRange.isConstrained || (Value.sge(ImmRange.Min) && Value.s

[PATCH] D58649: Fix inline assembler constraint validation

This revision was automatically updated to reflect the committed changes. Closed by commit rL354937: Fix inline assembler constraint validation (authored by joerg, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llv

[PATCH] D58649: Fix inline assembler constraint validation

joerg created this revision. joerg added a reviewer: compnerd. Herald added a subscriber: eraman. The current constraint logic is both too lax and too strict. It fails for input outside the [INT_MIN..INT_MAX] range, but it also implicitly accepts 0 as value when it should not. Adjust logic to ha

[PATCH] D58592: [clang] [ToolChains/NetBSD] Support relative libc++ header path

joerg added a comment. I'm not in favor of this. It adds overhead for the system compiler and generally makes the logic more complicated. This seems to be another hack around the fact that the driver has no clear notion of "use system runtime" vs "use custom runtime". Repository: rC Clang

[PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

joerg added a comment. As discussed with dankm on IRC, I still would like to see the correct behavior going into 8.0, i.e. not change it later. Since this also matters for potential faster implementations later, it seems like a good idea to do it now. The changes are well-localized. (1) Do pat

[PATCH] D56647: [WIP] [ELF] Implement --copy-dt-needed-entries

joerg added a comment. As first step, it goes into the right direction. I would explicitly set --as-needed for all those indirectly loaded objects. If people want to retain the questionable behavior of newer GNU tools, it could be a separate flag so that a final round can warn if an indirectly

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

joerg added a comment. Right, I'm not aware of anyone but FreeBSD really using the OSABI field. For FreeBSD it was a long standing hack around limitations in the ELF kernel loader. I'm not even sure if FreeBSD use(d) to set the OSABI field for the intermediate object files. CHANGES SINCE LAST

[PATCH] D49466: Initial implementation of -fmacro-prefix-map and -ffile-prefix-map

joerg added a comment. That's the other reason why I find the GCC specification as string prefix confusing. I still say we should just go with mapping of path names and then the order question mostly goes away. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D49466

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

joerg added inline comments. Comment at: ELF/Driver.cpp:779 +// TODO: verify the triple somehow? +Config->TargetTriple = llvm::Triple(Prefix); + } mgorny wrote: > joerg wrote: > > See ToolChain::getTargetAndModeFromProgramName in clang. > Yes, I've based

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

joerg added inline comments. Comment at: ELF/Driver.cpp:381 +} +Config->SearchPaths.push_back("/usr/lib"); + } Those need to be sysroot-relative of course. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56215/new/ https://reviews.llvm.org/D5621

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

joerg added a comment. @ruiu: No, it is exactly what you want, since it allows you to point lld into the normal sysroot. Cross-compiling is the default case for the NetBSD toolchain. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56215/new/ https://reviews.llvm.org/D56215 __

[PATCH] D56215: [lld] [ELF] Include default search paths for NetBSD driver

joerg added inline comments. Comment at: ELF/Driver.cpp:779 +// TODO: verify the triple somehow? +Config->TargetTriple = llvm::Triple(Prefix); + } See ToolChain::getTargetAndModeFromProgramName in clang. CHANGES SINCE LAST ACTION https://reviews.llvm

  1   2   3   >