Page MenuHomePhabricator

gokturk (Gokturk Yuksek)
User

Projects

User does not belong to any projects.

User Details

User Since
Oct 11 2019, 4:43 PM (5 w, 1 d)

Recent Activity

Mon, Nov 11

gokturk added a comment to D69869: [clang-tools-extra] fix the check for if '-latomic' is necessary.

Let me try my best to explain what's happening. The goal of this check is to determine whether passing -latomic is required.

Mon, Nov 11, 4:00 PM · Restricted Project

Tue, Nov 5

gokturk created D69869: [clang-tools-extra] fix the check for if '-latomic' is necessary.
Tue, Nov 5, 2:38 PM · Restricted Project
gokturk requested review of D69003: [dsymutil] Explicitly link against libatomic when necessary.
Tue, Nov 5, 2:29 PM · Restricted Project
gokturk updated the diff for D69003: [dsymutil] Explicitly link against libatomic when necessary.

Here's the proposed patch. I think this is the least invasive of all. Just check if passing '-latomic' is required inside dsymutil's cmake file.

Tue, Nov 5, 2:20 PM · Restricted Project
gokturk added a comment to D69003: [dsymutil] Explicitly link against libatomic when necessary.

You don't need to add the library to dsymutil. It is not code in dsymutil that depends on libatomic, it is code in LLVMSupport that dsymutil is using. By fixing this in LLVMSupport, any library linked against LLVMSupport will have a linkage to libatomic added.

Please don't spend 20 hours testing this patch. I'm very pretty sure this isn't the correct fix.

Tue, Nov 5, 2:20 PM · Restricted Project
gokturk added a reviewer for D68899: llvm/cmake/config.guess: add support for riscv32 and riscv64: beanz.
Tue, Nov 5, 10:01 AM · Restricted Project
gokturk added a comment to D69003: [dsymutil] Explicitly link against libatomic when necessary.

You don't need to add the library to dsymutil. It is not code in dsymutil that depends on libatomic, it is code in LLVMSupport that dsymutil is using. By fixing this in LLVMSupport, any library linked against LLVMSupport will have a linkage to libatomic added.

Please don't spend 20 hours testing this patch. I'm very pretty sure this isn't the correct fix.

dsymutil makes explicit use of libatomic (https://github.com/llvm/llvm-project/blob/release/9.x/llvm/tools/dsymutil/dsymutil.cpp#L542):

    std::atomic_char AllOK(1);
...
      auto LinkLambda = [&,
                         OutputFile](std::shared_ptr<raw_fd_ostream> Stream) {
        AllOK.fetch_and(linkDwarf(*Stream, BinHolder, *Map, *OptionsOrErr));
        Stream->flush();
        if (Verify && !NoOutput)
          AllOK.fetch_and(verify(OutputFile, Map->getTriple().getArchName()));
      };

Doesn't this create a link time dependency on libatomic?

Tue, Nov 5, 9:52 AM · Restricted Project

Fri, Oct 25

gokturk updated the diff for D69003: [dsymutil] Explicitly link against libatomic when necessary.

I'm updating the patch for completeness. system_libs doesn't propagate, so I had to get the system libs property of Support in dsymutil:

Fri, Oct 25, 3:15 PM · Restricted Project
gokturk added a comment to D69003: [dsymutil] Explicitly link against libatomic when necessary.

You don't need to add the library to dsymutil. It is not code in dsymutil that depends on libatomic, it is code in LLVMSupport that dsymutil is using. By fixing this in LLVMSupport, any library linked against LLVMSupport will have a linkage to libatomic added.

Please don't spend 20 hours testing this patch. I'm very pretty sure this isn't the correct fix.

Fri, Oct 25, 3:06 PM · Restricted Project
gokturk added a comment to D68899: llvm/cmake/config.guess: add support for riscv32 and riscv64.

Ping

Fri, Oct 25, 12:48 PM · Restricted Project
gokturk created D69444: [Support] Check for atomics64 when deciding if '-latomic' is needed.
Fri, Oct 25, 12:38 PM · Restricted Project
gokturk updated the diff for D69003: [dsymutil] Explicitly link against libatomic when necessary.

I think that @beanz is right about that. The reason for the HAVE_LIBATOMIC is because it is managing the current configuration checks. It may be possible to simplify it further in the future, but that area of the code has a HAVE_LIBATOMIC. That will setup system_libs properly and dsymutil can use system_libs as a parameter to LINK_LIBS option.

Fri, Oct 25, 12:38 PM · Restricted Project

Oct 16 2019

gokturk added a comment to D69003: [dsymutil] Explicitly link against libatomic when necessary.

I'm not sure this is the correct fix.

Probably a more correct fix would be to change line 29 of llvm/lib/Support/CMakeLists.txt to:

if( LLVM_ENABLE_THREADS AND (HAVE_LIBATOMIC OR HAVE_CXX_LIBATOMICS64))
Oct 16 2019, 3:08 PM · Restricted Project
gokturk added a comment to D69003: [dsymutil] Explicitly link against libatomic when necessary.

I've started a build on the affected VM (riscv64gc) to see if it solved the problem. It typically takes ~20 hours until the error is produced.

Also see https://reviews.llvm.org/D68964, related to this issue.

Oct 16 2019, 12:03 PM · Restricted Project

Oct 15 2019

gokturk added a comment to D69003: [dsymutil] Explicitly link against libatomic when necessary.

I've started a build on the affected VM (riscv64gc) to see if it solved the problem. It typically takes ~20 hours until the error is produced.

Oct 15 2019, 1:46 PM · Restricted Project
gokturk created D69003: [dsymutil] Explicitly link against libatomic when necessary.
Oct 15 2019, 1:46 PM · Restricted Project
gokturk added a comment to D68964: cmake/modules/CheckAtomic.cmake: catch false positives in RISC-V.

I think we have potentially 3 separate bugs:

  1. In LLVM, the check for libatomic has false positives in RISC-V (which the patch is addressing).
  2. In LLVM, the result of the libatomic check is not being used to add '-latomic' to LDFLAGS.
Oct 15 2019, 1:46 PM · Restricted Project
gokturk added a comment to D68964: cmake/modules/CheckAtomic.cmake: catch false positives in RISC-V.

Please do not merge.

Oct 15 2019, 1:19 PM · Restricted Project
gokturk added a comment to D68964: cmake/modules/CheckAtomic.cmake: catch false positives in RISC-V.

libc++ also has a version of this check (https://github.com/llvm/llvm-project/blob/master/libcxx/cmake/Modules/CheckLibcxxAtomic.cmake). Does that need to be adjusted as well?

Oct 15 2019, 7:49 AM · Restricted Project

Oct 14 2019

gokturk created D68964: cmake/modules/CheckAtomic.cmake: catch false positives in RISC-V.
Oct 14 2019, 5:19 PM · Restricted Project

Oct 11 2019

gokturk added reviewers for D68899: llvm/cmake/config.guess: add support for riscv32 and riscv64: erichkeane, rengolin, mgorny.
Oct 11 2019, 5:00 PM · Restricted Project
gokturk created D68899: llvm/cmake/config.guess: add support for riscv32 and riscv64.
Oct 11 2019, 5:00 PM · Restricted Project