User Details
- User Since
- Feb 5 2013, 8:31 AM (491 w, 5 h)
Fri, Jul 1
I checked this on my arm64 mac:
- These test are already failing on main branch due to the mismatch with -DCOMPILER_RT_HAS_FLOAT16
- They are still failing with this patch (due to conditionalizing the macro definition on x86)
- If I remove the conditional and always define COMPILER_RT_HAS_FLOAT16 when building the builtins for darwin, the tests pass for me. I don't know which ABI we want here for arm[64], but it confirms we need to be consistent about the conditional define.
Thu, Jun 30
Wed, Jun 29
Tue, Jun 28
Reverted in eab2a06f0fde due to the Darwin test failures.
This broke some compiler-rt tests on Darwin:
https://green.lab.llvm.org/green/job/clang-stage1-RA/29920/
Thu, Jun 23
Fri, Jun 17
Thu, Jun 16
Wed, Jun 15
Mon, Jun 13
Couple more minor things, but basically LGTM.
Wed, Jun 8
Removed use of std::unique_ptr in DependencyScanningTool.cpp, per review feedback.
Tue, Jun 7
Attempt to fix Windows path issue in test.
May 12 2022
Apr 29 2022
Apr 28 2022
LGTM, although I made a suggestion about the spelling.
Apr 22 2022
Minor suggestion for the test case, but otherwise LGTM.
Apr 6 2022
Apr 5 2022
Jan 25 2022
Jan 24 2022
Does check-orc-rt need to be added to COMPILER_RT_TEST_SUITES too? I guess not, otherwise we wouldn't have seen this missing dependency in the first place?
Jan 18 2022
Jan 14 2022
Make sure the test failure gets fixed, but otherwise LGTM.
x64 debian > LLVM-Unit.Support/_/SupportTests::VFSFromYAMLTest.RelativePaths
Jan 13 2022
Each VFS may have its own working directory, so it could be surprising if we use the OS working directory instead of that. This is complicated by the fact the VFS working directory may not be set yet during parsing the yaml (I haven't checked). I'm not really sure what to recommend here. If we do change this, we should document the new behaviour in the doc comment for RedirectingFileSystem though.
I don't know, I'm a bit skeptical we want to make it so easy to ignore errors so easily. I'd rather require clients to explicitly ignore the error.
Jan 12 2022
Jan 4 2022
Handling "" as enabling all logging is a bit strange to me -- what was the reasoning for that? LGTM other than than the return value from initializeDebug that I mentioned.
Dec 23 2021
Dec 22 2021
Dec 16 2021
Updated to enable the visibility macro when using BUILD_SHARED_LIBS and remove the MLIR-specific change, per review.
Dec 15 2021
Never mind, I got it working on Linux. BUILD_SHARED_LIBS of MLIR doesn't seem to work correctly on macOS. New patch with a fix: https://reviews.llvm.org/D115825
@mehdi_amini x86_64 macOS
Dec 14 2021
@mehdi_amini I'm seeing the same failure output in the llvm.c test even with my changes reverted and a clean build. Any idea what I might be missing? I am building with the following arguments -- they're about as close as I could get to that bot without having CUDA.
@mehdi_amini thanks!
Dec 10 2021
Since the new approach is no longer "opt-in", I tested:
- Default build (static, LLVMInitialize* is external)
- -DCMAKE_CXX_VISIBILITY_PRESET=hidden (static, LLVMInitialize* is hidden -- this is the new thing)
- -DBUILD_SHARED_LIBS=YES (dynamic, LLVMInitialize* is external due to not overriding visibility)
- -LLVM_LINK_LLVM_DYLIB=YES (dynamic, LLVMInitialize* is external due to LLVM_EXTERNAL_VISIBILITY)
Updated to set LLVM_EXTERNAL_VISIBILITY only when building the LLVM dylib.
Dec 9 2021
Ping, does anyone have any feedback on this?
Nov 19 2021
Nov 18 2021
Nov 17 2021
Nov 16 2021
Nov 12 2021
Nov 11 2021
Part of me wants to say that we should make these visibility macros conditional on the existing LLVM_ENABLE_DYLIB (sp?) option. If the user is statically linking LLVM, we should drop these annotations and allow the global visibility preset to take precedence.
Nov 10 2021
but maybe linking more than one copy of LLVM is rare enough that the issue hasn't come up before?
Nov 4 2021
Nov 2 2021
Oct 27 2021
Oct 21 2021
It looks like this caused an assertion failure on buildbots, would you mind reverting?