Page MenuHomePhabricator

jfb (JF Bastien)
User

Projects

User does not belong to any projects.

User Details

User Since
Jan 27 2014, 9:36 AM (311 w, 4 d)

Recent Activity

Today

jfb accepted D72741: [MLIR] LLVM dialect: Add llvm.atomicrmw.
In D72741#1827039, @jfb wrote:

You're making a new IR, and that's exactly the right time to revisit basic things like this instead of copying LLVM IR.

Maybe what is causing confusion is that we also have the MLIR standard dialect, which is more of a new IR itself. We are actually quite careful about things going in the standard dialect and it isn't bound by LLVM IR (other that we try to think about how to map down to LLVM).

Another way to put it is that the LLVM Dialect is only a layer intended to be hiding the LLVM IR Builders: while in MLIR you lower down to the LLVM Dialect instead of emitting directly LLVM IR. The reason is that this composes better (you can lower progressively to LLVM and not as a one shot conversion). So the LLVM dialect "embeds" the LLVM IR inside MLIR.

Fri, Jan 17, 11:58 AM · Restricted Project
jfb added a comment to D72741: [MLIR] LLVM dialect: Add llvm.atomicrmw.

I don't think this commit should be about making changes to LLVM IR. I can do that in a followup, but this commit should be in sync with whatever LLVM IR is today. There seems to be consensus with this approach.

Fri, Jan 17, 11:11 AM · Restricted Project
jfb requested changes to D72741: [MLIR] LLVM dialect: Add llvm.atomicrmw.

@ftynse I believe this is ready to go, anything else you'd like?

Also, I don't yet have committer access, so I'll need some help getting it landed.

Fri, Jan 17, 10:51 AM · Restricted Project

Yesterday

jfb updated subscribers of D72741: [MLIR] LLVM dialect: Add llvm.atomicrmw.
Thu, Jan 16, 9:44 AM · Restricted Project

Tue, Jan 14

jfb updated subscribers of D72741: [MLIR] LLVM dialect: Add llvm.atomicrmw.
Tue, Jan 14, 4:43 PM · Restricted Project
jfb updated subscribers of D72741: [MLIR] LLVM dialect: Add llvm.atomicrmw.
Tue, Jan 14, 4:23 PM · Restricted Project
jfb added inline comments to D72741: [MLIR] LLVM dialect: Add llvm.atomicrmw.
Tue, Jan 14, 4:23 PM · Restricted Project

Mon, Jan 13

jfb added a comment to D71600: PowerPC 32-bit - forces 8 byte lock/lock_free decisions at compiled time.

I am still confused why you need the special rule for __atomic_is_lock_free (GCC/clang) and __c11_atomic_is_lock_free (clang).

https://github.com/gcc-mirror/gcc/blob/master/gcc/builtins.c#L7300-L7314 GCC __atomic_is_lock_free expands to either 1 or a library call to __atomic_is_lock_free, never 0. How did FreeBSD get around libatomic when it was still using GCC? In clang, We can probably extend the semantics of __c11_atomic_is_lock_free and use clang::TargetInfo::getMaxAtomicPromoteWidth, but I'll make more research in this area.

Can you be elaborate how do __atomic_is_lock_free/__c11_atomic_is_lock_free pull in libatomic.{a,so} dependency on FreeBSD powerpc?

On FreeBSD 11.x/12.x (GCC4), libatomic can be installed by user through "gcc9" ports package. The library is not shipped with the base system and it's not required to compile the base system (world + kernel) since nothing depends on 64 bit atomics an external call so __atomic_is_lock_free is never made.

The problem appeared when I switched to clang in a new ISO, this added an undesired external libatomic dependency to build FreeBSD base sources (world): clang was emitting a call to external __atomic_is_lock_free implementation when building clang itself from base sources. (user would need to install gcc9 to be able to build FreeBSD base source using clang, it's not acceptable.

Mon, Jan 13, 10:16 AM · Restricted Project, Restricted Project, Restricted Project
jfb updated subscribers of D72579: Evaluate __{c11_,}atomic_is_lock_free to 0 (avoid libcall) if larger than MaxAtomicPromoteWidth.

This changes the expression to a constant expression as well, right? You should point this out in the commit message.

Mon, Jan 13, 10:16 AM · Restricted Project
jfb updated subscribers of D72579: Evaluate __{c11_,}atomic_is_lock_free to 0 (avoid libcall) if larger than MaxAtomicPromoteWidth.
Mon, Jan 13, 10:16 AM · Restricted Project

Wed, Jan 8

jfb added a comment to D68115: Zero initialize padding in unions.

@vitalybuka could we move this patch forward?

Wed, Jan 8, 2:23 PM · Restricted Project
jfb added a comment to D70326: [docs] LLVM Security Group and Process.

We (Microsoft) are interested in participating in this process.

Wed, Jan 8, 10:30 AM · Restricted Project

Tue, Jan 7

jfb added a comment to D70326: [docs] LLVM Security Group and Process.

I've not read this in detail or followed the list, but I wanted to add that I believe it's important that we have some form of public acknowledgement for the people that have reported security vulnerabilities in there as well.

Tue, Jan 7, 9:38 PM · Restricted Project
jfb added a comment to D70326: [docs] LLVM Security Group and Process.

We should explicitly state that patches to LLVM sent to the group are subject to the standard LLVM developer policy/license. This is important so members of the security group can use any patches.

We should prominently state that all messages and attachments will be publicly disclosed after any embargo expires. This is important so issue reporters don't send code under NDAs/etc.

Tue, Jan 7, 9:38 PM · Restricted Project
jfb updated the diff for D70326: [docs] LLVM Security Group and Process.
  • Address feedback
Tue, Jan 7, 9:30 PM · Restricted Project

Sun, Jan 5

jfb updated subscribers of D72240: Implement C++20 std::atomic_ref and test.
Sun, Jan 5, 9:03 PM · Restricted Project

Fri, Jan 3

jfb added a comment to D70910: [compiler-rt] Add a critical section when flushing gcov counters.

OK that helps a bit. I still don't understand though: why do you only need to protect flush and nothing else? You're protecting specific variables from being accesses concurrently only though flush, and not through the other paths that access those variables. Are these variables never accessed concurrently from these other functions?

Fri, Jan 3, 1:49 PM · Restricted Project, Restricted Project

Thu, Dec 19

jfb updated subscribers of D71726: Let clang atomic builtins fetch add/sub support floating point types.
Thu, Dec 19, 2:53 PM
jfb updated subscribers of D71726: Let clang atomic builtins fetch add/sub support floating point types.

This generally seems fine. Does it work on most backends? I want to make sure it doesn't fail in backends :)

Thu, Dec 19, 2:53 PM

Dec 12 2019

jfb committed rGd779dda7d0f9: Fix index while building following bitstream reader API update (authored by jfb).
Fix index while building following bitstream reader API update
Dec 12 2019, 2:04 PM
jfb committed rGaf934467296e: Fix APINotes following bitstream reader API update (authored by jfb).
Fix APINotes following bitstream reader API update
Dec 12 2019, 2:04 PM
jfb added a comment to D70910: [compiler-rt] Add a critical section when flushing gcov counters.

@jfb, sorry I thought my commit message was clear enough.
So when coverage is enabled a call to __gcov_flush is inserted just before forkcall:
https://github.com/llvm/llvm-project/blob/master/llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp#L659
So if the parent process is forked in different threads then __gcov_flush is called from different threads.
And so different global variables are accessed asynchronously:
https://github.com/llvm/llvm-project/blob/master/compiler-rt/lib/profile/GCDAProfiling.c#L104-L108
Initially we saw crashes in our CI: https://bugzilla.mozilla.org/show_bug.cgi?id=1599436
The above test case leads to multiple errors: double-free, "cannot merge previous GCDA file..."
And in debbuging firefox, I saw output_file == NULL (https://github.com/llvm/llvm-project/blob/master/compiler-rt/lib/profile/GCDAProfiling.c#L603) where it mustn't.

Dec 12 2019, 11:19 AM · Restricted Project, Restricted Project
jfb added a comment to D70910: [compiler-rt] Add a critical section when flushing gcov counters.

@jfb sorry but it fixes issues with fork:

  • we had a lot of crashes in Firefox CI when dumping the GCDAs and so I tried this patch on the CI and no more weird crashes.
  • and a minimal test case to reproduce it: ` #include <iostream> #include <thread> #include <vector> #include <unistd.h>

    void foo() { fork(); }

    int main() { std::vector<std::thread> pool;

    for (int i = 0; i < 10; i++) { pool.emplace_back(std::thread(foo)); }

    for (auto & t : pool) { t.join(); }

    return 0; } `

    If I compile and run it (clang++-9 -std=c++11 foo.cpp -oo -lpthread --coverage && ./o) I get systematically a SEGFAULT and with the patch no more crashes. The problem is not with flush_fn_list itself but it's when the functions in the list (gcda dumper) are called concurrently.
Dec 12 2019, 9:55 AM · Restricted Project, Restricted Project

Dec 9 2019

jfb added a comment to D70910: [compiler-rt] Add a critical section when flushing gcov counters.

When you say "when the process is forked in different threads" you mean threads, not fork, right? Because this won't fix any issues you're seeing with fork.

Dec 9 2019, 8:57 PM · Restricted Project, Restricted Project
jfb updated the summary of D49091: Warn about usage of __has_include/__has_include_next in macro expansions.
Dec 9 2019, 11:02 AM · Restricted Project

Dec 6 2019

jfb added a reviewer for D71128: [NVPTX][FIX] Expand atomics we cannot handle natively in the ISA: __simt__.
Dec 6 2019, 9:55 AM · Restricted Project
jfb added a comment to D71091: Make sure that the implicit arguments for direct methods have been setup.

You should upload patches with context :)

Dec 6 2019, 9:08 AM · Restricted Project

Dec 5 2019

jfb added a reviewer for D66822: Hardware cache line size builtins: __simt__.
Dec 5 2019, 9:26 AM · Restricted Project

Nov 16 2019

jfb updated the summary of D70326: [docs] LLVM Security Group and Process.
Nov 16 2019, 2:36 PM · Restricted Project

Nov 15 2019

jfb created D70326: [docs] LLVM Security Group and Process.
Nov 15 2019, 10:59 AM · Restricted Project

Nov 13 2019

jfb added inline comments to D63131: arm64_32: implement the desired ABI for the ILP32 triple..
Nov 13 2019, 1:56 PM · Restricted Project

Nov 8 2019

jfb accepted D63131: arm64_32: implement the desired ABI for the ILP32 triple..

Minor comments, but otherwise LGTM.

Nov 8 2019, 4:01 AM · Restricted Project

Oct 21 2019

jfb added a comment to D66046: Add new tautological compare warning for bitwise-or with a non-zero constant.

Mr Trieu, what do you think about adding some or all of the Wtautological-compare warnings to Wall

Oct 21 2019, 2:04 PM · Restricted Project

Oct 17 2019

jfb accepted D69148: Disable exit-on-SIGPIPE in lldb.

More of an FYI, @jordan_rose might be interested in this.

Oct 17 2019, 4:59 PM · Restricted Project, Restricted Project
jfb added inline comments to D69148: Disable exit-on-SIGPIPE in lldb.
Oct 17 2019, 4:21 PM · Restricted Project, Restricted Project

Oct 16 2019

jfb added a comment to D68964: cmake/modules/CheckAtomic.cmake: catch false positives in RISC-V.

This is bogus -- the RISCV GCC implementation is broken. It should not be mixing libatomic function calls with direct atomics.

Oct 16 2019, 10:00 AM · Restricted Project

Oct 14 2019

jfb accepted D68964: cmake/modules/CheckAtomic.cmake: catch false positives in RISC-V.

This is pretty brittle... but 🤷‍♂️

Oct 14 2019, 9:02 PM · Restricted Project

Sep 27 2019

jfb added a comment to D68115: Zero initialize padding in unions.

The entire point of this feature is to add guardrails to the language. What do people expect in the real world? Is there a cost to meeting these expectations? If we put the pattern (0x00 or 0xaa) in the technically undef space, what comes out?

Sep 27 2019, 7:26 PM · Restricted Project
jfb accepted D67436: CodeGen: set correct result for atomic compound expressions.

Separately, does this do floating-point add / sub properly? We added them too C++20.

Sep 27 2019, 9:28 AM · Restricted Project

Sep 13 2019

jfb added a comment to D67399: [ARM] Follow AACPS standard for volatile bitfields.

Indeed our main concern is regarding the access widths of loads. As mentioned by @rjmccall, most volatile bitfields are used to perform memory mapped I/O, and some hardware only support them with a specific access width.
The spurious load I am more than glad to leave it disable behind a command flag, so it will only appear if the user requests it. See that volatile accesses might have side effects, and for example, an I/O read counter holding an odd number could define that the data is still being processed.

Sep 13 2019, 8:48 AM · Restricted Project

Sep 12 2019

jfb accepted D64146: [Clang Interpreter] Initial patch for the constexpr interpreter.

Sounds like this is ready to land again! Thanks for fixing everything.

Sep 12 2019, 3:22 PM · Restricted Project, Restricted Project

Sep 10 2019

jfb added inline comments to D67399: [ARM] Follow AACPS standard for volatile bitfields.
Sep 10 2019, 9:01 AM · Restricted Project
jfb updated subscribers of D67399: [ARM] Follow AACPS standard for volatile bitfields.
Sep 10 2019, 9:00 AM · Restricted Project

Sep 6 2019

jfb added a comment to D67248: [InstCombine] pow(x, +/- 0.0) -> 1.0.

Sorry - I didn't see this review, only the bug report that came in today:
https://bugs.llvm.org/show_bug.cgi?id=43233

I made an identical code change, but I didn't include as many tests. Feel free to commit those or let me know if you want me to.

Sep 6 2019, 9:31 AM · Restricted Project
jfb committed rG52614dfc7fd0: [InstCombine] pow(x, +/- 0.0) -> 1.0 (authored by jfb).
[InstCombine] pow(x, +/- 0.0) -> 1.0
Sep 6 2019, 9:28 AM
jfb committed rL371224: [InstCombine] pow(x, +/- 0.0) -> 1.0.
[InstCombine] pow(x, +/- 0.0) -> 1.0
Sep 6 2019, 9:25 AM
jfb added a comment to D67248: [InstCombine] pow(x, +/- 0.0) -> 1.0.
Sep 6 2019, 9:25 AM · Restricted Project
jfb closed D67248: [InstCombine] pow(x, +/- 0.0) -> 1.0.
Sep 6 2019, 9:25 AM · Restricted Project

Sep 5 2019

jfb updated the diff for D67248: [InstCombine] pow(x, +/- 0.0) -> 1.0.
  • Address arsenm's comments
Sep 5 2019, 5:39 PM · Restricted Project
jfb created D67248: [InstCombine] pow(x, +/- 0.0) -> 1.0.
Sep 5 2019, 5:27 PM · Restricted Project

Sep 4 2019

jfb added inline comments to D66397: [Diagnostics] Improve -Wxor-used-as-pow.
Sep 4 2019, 3:04 PM · Restricted Project
jfb committed rL370958: Request commit access for jfb.
Request commit access for jfb
Sep 4 2019, 12:17 PM

Sep 3 2019

jfb requested changes to D66822: Hardware cache line size builtins.

Sorry for the delayed response, I was on vacation. Thanks for tackling it!

Sep 3 2019, 3:20 PM · Restricted Project
jfb added a comment to D67023: Diagnose use of ATOMIC_VAR_INIT.
In D67023#1654070, @jfb wrote:

I refer you to http://wg21.link/p0883
I don’t think this diagnostic should be added to clang until p0883 is fully implemented, even just for C. Otherwise we leave users with no portable way to do the right thing without diagnostic.

P0883 has not been adopted, that I can tell (it has strong support, but isn't [expected to be] a part of C++2a)?

Sep 3 2019, 3:05 PM

Sep 1 2019

jfb added a comment to D67023: Diagnose use of ATOMIC_VAR_INIT.

I refer you to http://wg21.link/p0883
I don’t think this diagnostic should be added to clang until p0883 is fully implemented, even just for C. Otherwise we leave users with no portable way to do the right thing without diagnostic.

Sep 1 2019, 3:15 PM

Aug 30 2019

jfb added a comment to D67023: Diagnose use of ATOMIC_VAR_INIT.

Is atomic initialization now correct in all modes (including C++) without this macro? I don’t think we should diagnose until such a time because some code uses to macro to be portably correct. IIRC we only ended up fixing C++ in 20 with Nico’s paper (after Olivier and I failed repeatedly to do so.

Aug 30 2019, 5:26 PM
jfb added inline comments to D66959: Update libc++ release notes.
Aug 30 2019, 1:28 AM · Restricted Project

Aug 20 2019

jfb added a comment to D64817: [CMake] Fix LLVM build non-determinism on RHEL.

binutils 2.22 was released in 2011. Do we want to support binutils that old? If not, we should probably not do such change, but rather bump the version requirement https://llvm.org/docs/GettingStarted.html#host-c-toolchain-both-compiler-and-standard-library

CC @hans why may know more about these problems.

I don't really. JF is the master of supported versions of things.

Aug 20 2019, 2:13 PM · Restricted Project

Aug 15 2019

jfb added a comment to D66313: Re-introduce the RWMutexImpl for macOS < 10.12.

Could you document this limitation in CodingStandards.rst, like what I removed for C++11 in https://reviews.llvm.org/D66195#change-S42p9rAdiuZJ ?

Aug 15 2019, 4:00 PM · Restricted Project
jfb added a comment to D66310: Fix nm on GCC 5.1 after the C++14 move.

Sounds like you get to claim this as a fix for https://bugs.llvm.org/show_bug.cgi?id=24115 too :)

Aug 15 2019, 2:11 PM · Restricted Project
jfb committed rGc984dde170bb: Fix nm on GCC 5.1 after the C++14 move (authored by jfb).
Fix nm on GCC 5.1 after the C++14 move
Aug 15 2019, 1:39 PM
jfb committed rL369045: Fix nm on GCC 5.1 after the C++14 move.
Fix nm on GCC 5.1 after the C++14 move
Aug 15 2019, 1:37 PM
jfb closed D66310: Fix nm on GCC 5.1 after the C++14 move.
Aug 15 2019, 1:37 PM · Restricted Project
jfb updated the diff for D66310: Fix nm on GCC 5.1 after the C++14 move.
  • Use if instead of ternary
Aug 15 2019, 1:37 PM · Restricted Project
jfb added a comment to D66195: Move to C++14.

Thanks, that helped! Next one (build step 3810/3982, so hopefully not too much left after this):

FAILED: tools/llvm-nm/CMakeFiles/llvm-nm.dir/llvm-nm.cpp.o 
/b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc510trusty/bin/g++  -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools/llvm-nm -I/b/s/w/ir/cache/builder/src/third_party/llvm/llvm/tools/llvm-nm -I/usr/include/libxml2 -Iinclude -I/b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include -DLLVM_FORCE_HEAD_REVISION -fvisibility-inlines-hidden -Werror=date-time -std=c++14 -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wno-maybe-uninitialized -Wdelete-non-virtual-dtor -Wno-comment -fdiagnostics-color -ffunction-sections -fdata-sections -O3    -UNDEBUG  -fno-exceptions -fno-unwind-tables -fno-asynchronous-unwind-tables -fno-rtti -MD -MT tools/llvm-nm/CMakeFiles/llvm-nm.dir/llvm-nm.cpp.o -MF tools/llvm-nm/CMakeFiles/llvm-nm.dir/llvm-nm.cpp.o.d -o tools/llvm-nm/CMakeFiles/llvm-nm.dir/llvm-nm.cpp.o -c /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/tools/llvm-nm/llvm-nm.cpp
In file included from /b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc510trusty/include/c++/5.1.0/bits/stl_algobase.h:71:0,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc510trusty/include/c++/5.1.0/memory:62,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/ADT/Optional.h:22,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/ADT/STLExtras.h:19,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/ADT/StringRef.h:12,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/ADT/StringSwitch.h:15,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/tools/llvm-nm/llvm-nm.cpp:18:
/b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc510trusty/include/c++/5.1.0/bits/predefined_ops.h: In instantiation of ‘constexpr bool __gnu_cxx::__ops::_Iter_comp_iter<_Compare>::operator()(_Iterator1, _Iterator2) [with _Iterator1 = {anonymous}::NMSymbol; _Iterator2 = {anonymous}::NMSymbol; _Compare = std::function<bool(const {anonymous}::NMSymbol&, const {anonymous}::NMSymbol&)>]’:
/b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc510trusty/include/c++/5.1.0/functional:1982:6:   required by substitution of ‘template<class _Res, class ... _ArgTypes> template<class _Functor> using _Invoke = decltype (std::__callable_functor(declval<_Functor&>())((declval<_ArgTypes>)()...)) [with _Functor = __gnu_cxx::__ops::_Iter_comp_iter<std::function<bool(const {anonymous}::NMSymbol&, const {anonymous}::NMSymbol&)> >; _Res = bool; _ArgTypes = {const {anonymous}::NMSymbol&, const {anonymous}::NMSymbol&}]’
/b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc510trusty/include/c++/5.1.0/functional:1992:56:   required by substitution of ‘template<class _Res, class ... _ArgTypes> template<class _Functor> using _Callable = std::__and_<std::function<_Res(_ArgTypes ...)>::_NotSelf<_Functor>, std::__check_func_return_type<std::function<_Res(_ArgTypes ...)>::_Invoke<_Functor>, _Res> > [with _Functor = __gnu_cxx::__ops::_Iter_comp_iter<std::function<bool(const {anonymous}::NMSymbol&, const {anonymous}::NMSymbol&)> >; _Res = bool; _ArgTypes = {const {anonymous}::NMSymbol&, const {anonymous}::NMSymbol&}]’
/b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc510trusty/include/c++/5.1.0/functional:2057:9:   required by substitution of ‘template<class _Functor, class> std::function<_Res(_ArgTypes ...)>::function(_Functor) [with _Functor = __gnu_cxx::__ops::_Iter_comp_iter<std::function<bool(const {anonymous}::NMSymbol&, const {anonymous}::NMSymbol&)> >; <template-parameter-1-2> = <missing>]’
/b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc510trusty/include/c++/5.1.0/bits/predefined_ops.h:130:46:   required from ‘constexpr __gnu_cxx::__ops::_Iter_comp_iter<_Compare> __gnu_cxx::__ops::__iter_comp_iter(_Compare) [with _Compare = std::function<bool(const {anonymous}::NMSymbol&, const {anonymous}::NMSymbol&)>]’
/b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc510trusty/include/c++/5.1.0/bits/stl_algo.h:4729:70:   required from ‘void std::sort(_RAIter, _RAIter, _Compare) [with _RAIter = __gnu_cxx::__normal_iterator<{anonymous}::NMSymbol*, std::vector<{anonymous}::NMSymbol> >; _Compare = std::function<bool(const {anonymous}::NMSymbol&, const {anonymous}::NMSymbol&)>]’
/b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/ADT/STLExtras.h:1125:12:   required from ‘void llvm::sort(IteratorTy, IteratorTy, Compare) [with IteratorTy = __gnu_cxx::__normal_iterator<{anonymous}::NMSymbol*, std::vector<{anonymous}::NMSymbol> >; Compare = std::function<bool(const {anonymous}::NMSymbol&, const {anonymous}::NMSymbol&)>]’
/b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/ADT/STLExtras.h:1130:13:   required from ‘void llvm::sort(Container&&, Compare) [with Container = std::vector<{anonymous}::NMSymbol>&; Compare = std::function<bool(const {anonymous}::NMSymbol&, const {anonymous}::NMSymbol&)>]’
/b/s/w/ir/cache/builder/src/third_party/llvm/llvm/tools/llvm-nm/llvm-nm.cpp:724:31:   required from here
/b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc510trusty/include/c++/5.1.0/bits/predefined_ops.h:123:31: error: no match for ‘operator*’ (operand type is ‘{anonymous}::NMSymbol’)
         { return bool(_M_comp(*__it1, *__it2)); }
                               ^
In file included from /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/ADT/APFloat.h:19:0,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/IR/Type.h:17,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/IR/DerivedTypes.h:23,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/IR/Function.h:29,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/tools/llvm-nm/llvm-nm.cpp:21:
/b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/ADT/APInt.h:2097:14: note: candidate: llvm::APInt llvm::operator*(llvm::APInt, uint64_t)
 inline APInt operator*(APInt a, uint64_t RHS) {
              ^
/b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/ADT/APInt.h:2097:14: note:   candidate expects 2 arguments, 1 provided
/b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/ADT/APInt.h:2102:14: note: candidate: llvm::APInt llvm::operator*(uint64_t, llvm::APInt)
 inline APInt operator*(uint64_t LHS, APInt b) {
              ^
/b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/ADT/APInt.h:2102:14: note:   candidate expects 2 arguments, 1 provided
In file included from /b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc510trusty/include/c++/5.1.0/bits/stl_algobase.h:71:0,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc510trusty/include/c++/5.1.0/memory:62,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/ADT/Optional.h:22,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/ADT/STLExtras.h:19,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/ADT/StringRef.h:12,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/ADT/StringSwitch.h:15,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/tools/llvm-nm/llvm-nm.cpp:18:
/b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc510trusty/include/c++/5.1.0/bits/predefined_ops.h:123:39: error: no match for ‘operator*’ (operand type is ‘{anonymous}::NMSymbol’)
         { return bool(_M_comp(*__it1, *__it2)); }
                                       ^
In file included from /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/ADT/APFloat.h:19:0,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/IR/Type.h:17,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/IR/DerivedTypes.h:23,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/IR/Function.h:29,
                 from /b/s/w/ir/cache/builder/src/third_party/llvm/llvm/tools/llvm-nm/llvm-nm.cpp:21:
/b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/ADT/APInt.h:2097:14: note: candidate: llvm::APInt llvm::operator*(llvm::APInt, uint64_t)
 inline APInt operator*(APInt a, uint64_t RHS) {
              ^
/b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/ADT/APInt.h:2097:14: note:   candidate expects 2 arguments, 1 provided
/b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/ADT/APInt.h:2102:14: note: candidate: llvm::APInt llvm::operator*(uint64_t, llvm::APInt)
 inline APInt operator*(uint64_t LHS, APInt b) {
              ^
/b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/ADT/APInt.h:2102:14: note:   candidate expects 2 arguments, 1 provided
Aug 15 2019, 1:18 PM · Restricted Project
jfb created D66310: Fix nm on GCC 5.1 after the C++14 move.
Aug 15 2019, 1:15 PM · Restricted Project
jfb added a comment to D66259: Migrate llvm::make_unique to std::make_unique.

FWIW I think this particular bug was https://gcc.gnu.org/PR65942

Aug 15 2019, 1:15 PM · Restricted Project
jfb committed rG7a210d65edc6: Fix lld on GCC 5.1 after the C++14 move (authored by jfb).
Fix lld on GCC 5.1 after the C++14 move
Aug 15 2019, 10:48 AM
jfb committed rL369023: Fix lld on GCC 5.1 after the C++14 move.
Fix lld on GCC 5.1 after the C++14 move
Aug 15 2019, 10:48 AM
jfb closed D66306: Fix lld on GCC 5.1 after the C++14 move.
Aug 15 2019, 10:48 AM · Restricted Project
jfb updated the diff for D66306: Fix lld on GCC 5.1 after the C++14 move.
  • Fall through as suggested.
Aug 15 2019, 10:45 AM · Restricted Project
jfb added inline comments to D66244: Compiler.h: remove old GCC checks, update docs.
Aug 15 2019, 10:37 AM · Restricted Project
jfb accepted D66256: [AIX] For XL, pick GCC-compatible std & default warning options.
Aug 15 2019, 10:31 AM · Restricted Project
jfb added a comment to D66195: Move to C++14.
In D66195#1631797, @jfb wrote:

I have a repro, will try to figure out a workaround. It's fixed in 5.2. Talking to Jonathan Wakely we'd probably be better off on 5.5, but that'll be a separate discussion.

I have a fix. Building and testing... Will send a patch soon.

Aug 15 2019, 10:25 AM · Restricted Project
jfb created D66306: Fix lld on GCC 5.1 after the C++14 move.
Aug 15 2019, 10:25 AM · Restricted Project
jfb added a comment to D66259: Migrate llvm::make_unique to std::make_unique.

It looks like this breaks building with gcc5.1:

Why are you using 5.1? That's dumb. There are tons of bugs in 5.1 that are fixed in later 5.x releases, especially if you're using C++14.

That's a really bad choice of minimum version to insist on supporting.

Source: me, because I broke lots of stuff in 5.1 and then fixed it.

Aug 15 2019, 10:18 AM · Restricted Project
jfb added a comment to D66195: Move to C++14.

I have a repro, will try to figure out a workaround. It's fixed in 5.2. Talking to Jonathan Wakely we'd probably be better off on 5.5, but that'll be a separate discussion.

Aug 15 2019, 10:12 AM · Restricted Project
jfb added a comment to D66195: Move to C++14.
In D66195#1631746, @jfb wrote:

Ugh that's an ugly one...

/b/s/w/ir/cache/builder/src/third_party/llvm/llvm/include/llvm/ADT/STLExtras.h:1322:19:   required from ‘void llvm::stable_sort(R&&, Compare) [with R = llvm::MutableArrayRef<lld::elf::InputSection*>&; Compare = std::function<bool(lld::elf::InputSectionBase*, lld::elf::InputSectionBase*)>]’
/b/s/w/ir/cache/builder/src/third_party/llvm/lld/ELF/LinkerScript.cpp:347:44:   required from here
/b/s/w/ir/cache/builder/src/third_party/llvm-build-tools/gcc510trusty/include/c++/5.1.0/bits/predefined_ops.h:123:46: error: no match for call to ‘(std::function<bool(lld::elf::InputSectionBase*, lld::elf::InputSectionBase*)>) (lld::elf::InputSectionBase&, lld::elf::InputSectionBase&)’

Looks like we do the following:

template <typename R, typename Compare>
void stable_sort(R &&Range, Compare C) {
  std::stable_sort(adl_begin(Range), adl_end(Range), C);
}

static void sortSections(MutableArrayRef<InputSection *> vec,
                         SortSectionPolicy k) {
  if (k != SortSectionPolicy::Default && k != SortSectionPolicy::None)
    llvm::stable_sort(vec, getComparator(k));
}

static std::function<bool(InputSectionBase *, InputSectionBase *)>
getComparator(SortSectionPolicy k) {
  switch (k) {
  case SortSectionPolicy::Alignment:
    return [](InputSectionBase *a, InputSectionBase *b) {
      // ">" is not a mistake. Sections with larger alignments are placed
      // before sections with smaller alignments in order to reduce the
      // amount of padding necessary. This is compatible with GNU.
      return a->alignment > b->alignment;
    };
  case SortSectionPolicy::Name:
    return [](InputSectionBase *a, InputSectionBase *b) {
      return a->name < b->name;
    };
  case SortSectionPolicy::Priority:
    return [](InputSectionBase *a, InputSectionBase *b) {
      return getPriority(a->name) < getPriority(b->name);
    };
  default:
    llvm_unreachable("unknown sort policy");
  }
}

Has anyone seen this before? Any known workarounds?

I haven't seen it before... taking a look now.

Aug 15 2019, 10:01 AM · Restricted Project
jfb added a comment to D66195: Move to C++14.

Ugh that's an ugly one...

Aug 15 2019, 9:33 AM · Restricted Project
jfb added a comment to D66195: Move to C++14.
In D66195#1629329, @jfb wrote:

Perhaps add a note in docs/ReleaseNotes.rst as well?

I can, but that seems odd: aren’t release notes for users of LLVM, not developers? In other words, what would someone reading this note take away from it, and do based on reading it?

I think the releases are also used by folks doing various kinds of out-of-tree LLVM stuff, and this change might let them use C++14 in their code by default.

I agree with Hans that there should be an LLVM release note. Another action for an LLVM user (who, to be clear, is a developer of an LLVM-based compiler) would be to update their host toolchain before trying to migrate to the newer LLVM version.

Aug 15 2019, 9:32 AM · Restricted Project
jfb accepted D55562: Atomics: support min/max orthogonally.
Aug 15 2019, 9:17 AM · Restricted Project
jfb added a comment to D66271: [CMake] Check for C++14 instead of C++11.

Thanks for fixing this!

Aug 15 2019, 9:07 AM · Restricted Project, Restricted Project

Aug 14 2019

jfb committed rGcad8356d699b: Remove LVALUE / RVALUE workarounds (authored by jfb).
Remove LVALUE / RVALUE workarounds
Aug 14 2019, 3:50 PM
jfb committed rL368939: Remove LVALUE / RVALUE workarounds.
Remove LVALUE / RVALUE workarounds
Aug 14 2019, 3:50 PM
jfb closed D66240: Remove LVALUE / RVALUE workarounds.
Aug 14 2019, 3:50 PM · Restricted Project, Restricted Project
jfb added a comment to D66259: Migrate llvm::make_unique to std::make_unique.

BTW, if this breaks stuff maybe it's better to do it one project at a time, and remove the helper at the very end.

Aug 14 2019, 3:13 PM · Restricted Project
jfb added a comment to D66240: Remove LVALUE / RVALUE workarounds.
Aug 14 2019, 3:11 PM · Restricted Project, Restricted Project
jfb accepted D66259: Migrate llvm::make_unique to std::make_unique.

Thanks!

Aug 14 2019, 3:10 PM · Restricted Project
jfb added inline comments to D66259: Migrate llvm::make_unique to std::make_unique.
Aug 14 2019, 3:02 PM · Restricted Project
jfb added a comment to D66256: [AIX] For XL, pick GCC-compatible std & default warning options.
In D66256#1630326, @jfb wrote:

Can XL enable all the same flags which GCC can?

It can except the modules stuff, which should would be detected via the CHECK_CXX_SOURCE_COMPILES.

There's a lot under that elseif!

Most of it is the modules stuff.

At that point, isn't XL GCC compatible?

It isn't, because the object-file generation options (e.g., -fno-unwind-tables) differ.

I guess I should say "default warning" options in the commit message, because the LLVM_ENABLE_WARNINGS and LLVM_ENABLE_PEDANTIC modes aren't enabled with this patch.

Aug 14 2019, 2:46 PM · Restricted Project
jfb added a comment to D66256: [AIX] For XL, pick GCC-compatible std & default warning options.

Can XL enable all the same flags which GCC can? There's a lot under that elseif! At that point, isn't XL GCC compatible?

Aug 14 2019, 2:31 PM · Restricted Project
jfb committed rG9953c74fb657: Use std::is_final directly (authored by jfb).
Use std::is_final directly
Aug 14 2019, 1:15 PM
jfb committed rL368910: Use std::is_final directly.
Use std::is_final directly
Aug 14 2019, 1:13 PM
jfb committed rGee6f3dd14dec: SwapByteOrder.h: don't check for unsupported GCC versions (authored by jfb).
SwapByteOrder.h: don't check for unsupported GCC versions
Aug 14 2019, 12:59 PM
jfb committed rL368909: SwapByteOrder.h: don't check for unsupported GCC versions.
SwapByteOrder.h: don't check for unsupported GCC versions
Aug 14 2019, 12:58 PM
jfb committed rG6ff2a1c878eb: MathExtras.h: don't check for unsupported GCC versions (authored by jfb).
MathExtras.h: don't check for unsupported GCC versions
Aug 14 2019, 12:57 PM
jfb committed rL368908: MathExtras.h: don't check for unsupported GCC versions.
MathExtras.h: don't check for unsupported GCC versions
Aug 14 2019, 12:56 PM
jfb created D66244: Compiler.h: remove old GCC checks, update docs.
Aug 14 2019, 12:54 PM · Restricted Project
jfb created D66240: Remove LVALUE / RVALUE workarounds.
Aug 14 2019, 12:35 PM · Restricted Project, Restricted Project
jfb committed rGc2649928533b: Revert "Un-break the bots" (authored by jfb).
Revert "Un-break the bots"
Aug 14 2019, 12:21 PM