Page MenuHomePhabricator

beanz (Chris Bieneman)
User

Projects

User does not belong to any projects.

User Details

User Since
Aug 20 2014, 4:35 PM (256 w, 4 d)

Recent Activity

Sat, Jul 20

beanz requested changes to D65045: [CMake] LLVM_EXTERNAL_<proj>_SOURCE_DIR should be adjustable..

Adding ‘FORCE’ makes this not user overridable, which is the whole point of this being a cache variable.

Sat, Jul 20, 8:28 PM · Restricted Project

Wed, Jul 17

beanz added inline comments to D64837: [cmake] Convert the NATIVE llvm build process to be project agnostic.
Wed, Jul 17, 1:44 PM · Restricted Project

Thu, Jul 11

beanz accepted D64278: Rename libclang_shared to libclang-cpp.

This is fine with me. I have no real attachment to the name.

Thu, Jul 11, 2:38 PM · Restricted Project, Restricted Project
beanz accepted D64582: cmake: Fix install of libclang_shared.so when LLVM_INSTALL_TOOLCHAIN_ONLY=ON.

LGTM.

Thu, Jul 11, 2:35 PM · Restricted Project, Restricted Project
beanz accepted D64579: [clang-shlib] Fix clang-shlib for PRIVATE dependencies.

Yea, this makes sense even if it is a bit sad. We do use the --whole-archive approach for libLLVM, and it works. I was hoping to avoid it here in part to free up the ability to link libclang_shared before or in parallel to archiving the libclang*.a archives.

Thu, Jul 11, 2:15 PM · Restricted Project, Restricted Project
beanz accepted D64580: cmake: Add INSTALL_WITH_TOOLCHAIN option to add_*_library macros.

LGTM.

Thu, Jul 11, 2:10 PM · Restricted Project, Restricted Project

Tue, Jul 9

beanz added a comment to D62455: [cmake] Move LLVM_TOUCH_STATIC_LIBRARIES logic to llvm_update_compile_flags.

One thought, we should probably evaluate why those tools are using add_library instead of add_llvm_library. The right fix might be to move them over. This is exactly the kind of behavior that add_llvm_library and llvm_add_library were designed to handle.

Tue, Jul 9, 9:51 AM · Restricted Project

Mon, Jul 8

beanz added a comment to D62455: [cmake] Move LLVM_TOUCH_STATIC_LIBRARIES logic to llvm_update_compile_flags.

I don't like the idea of putting this in llvm_update_compile_flags. That function does exactly what its name says, and adding additional behavior to it seems undesirable. Where are we creating libraries with add_library that we need this?

Mon, Jul 8, 1:21 PM · Restricted Project
beanz committed rG7023bdc46fbb: Fix issues building libraries as more than one type with Xcode (authored by beanz).
Fix issues building libraries as more than one type with Xcode
Mon, Jul 8, 11:34 AM

Sun, Jul 7

beanz created D64300: Fix issues building libraries as more than one type with Xcode.
Sun, Jul 7, 2:17 PM · Restricted Project

Fri, Jul 5

beanz added a comment to D63881: [cmake] Fix build with BUILD_SHARED_LIBS=ON .

Reading through the old comments on this thread I think there are a few things going on here, but the core of the issue is you're using BUILD_SHARED_LIBS *outside* LLVM's build system. That is explicitly not supported (https://www.llvm.org/docs/CMake.html). We do not support BUILD_SHARED_LIBS for distributing LLVM in any form.

Fri, Jul 5, 5:20 AM · Restricted Project
beanz requested changes to D63881: [cmake] Fix build with BUILD_SHARED_LIBS=ON .
Fri, Jul 5, 4:46 AM · Restricted Project
beanz added a comment to D63881: [cmake] Fix build with BUILD_SHARED_LIBS=ON .

This is definitely not the right fix. We don’t want to set everything as Public dependencies. Our build system’s model for dependencies is that each component specified what it directly uses which is its Private dependencies. If you are having issues that probably means missing dependencies.

Fri, Jul 5, 4:40 AM · Restricted Project

Tue, Jul 2

beanz added a comment to D63503: cmake: Add CLANG_LINK_CLANG_DYLIB option.

One comment inline. Otherwise LGTM.

Tue, Jul 2, 11:45 AM · Restricted Project, Restricted Project

Jun 20 2019

beanz added a comment to D63544: Use object library if cmake supports it.

I really don't think this is the right solution.

Jun 20 2019, 11:00 AM · Restricted Project, Restricted Project

Jun 10 2019

beanz committed rGdc2c72eefa45: Setup testing target dependencies for default runtimes (authored by beanz).
Setup testing target dependencies for default runtimes
Jun 10 2019, 5:24 PM
beanz created D63107: Setup testing target dependencies for default runtimes.
Jun 10 2019, 4:43 PM · Restricted Project
beanz added inline comments to D61446: Generalize the pass registration mechanism used by Polly to any third-party tool.
Jun 10 2019, 10:41 AM · Restricted Project, Restricted Project

Jun 5 2019

beanz committed rGb67cb3cda05b: Use LTO capable linker (authored by beanz).
Use LTO capable linker
Jun 5 2019, 10:33 AM
beanz added inline comments to D61446: Generalize the pass registration mechanism used by Polly to any third-party tool.
Jun 5 2019, 10:25 AM · Restricted Project, Restricted Project

Jun 3 2019

beanz accepted D62820: [builtins] Use libtool for builtins when building for Apple platform.

LGTM

Jun 3 2019, 5:28 PM · Restricted Project, Restricted Project
beanz added a comment to D61446: Generalize the pass registration mechanism used by Polly to any third-party tool.

I have a thought for how we can make the CMake interface for this better. Instead of LLVM_EXTENSIONS being a global property if it were a pre-defined list like LLVM_ALL_PROJECTS, then the LLVM_LINK_<X>_INTO_TOOLS variables can be generated by LLVM's configuration instead of relying on Polly's to run. If you do that LLVM_LINK_POLLY_INTO_TOOLS could be set to On even if Polly isn't enabled to build, but that actually will be fine if you change the calls to target_link_libraries in the tools to use a generator expression something like: $<$<TARGET_EXISTS:Polly>,Polly>.

Jun 3 2019, 5:24 PM · Restricted Project, Restricted Project

May 31 2019

beanz accepted D62279: Use LTO capable linker.

This is good to land.

May 31 2019, 10:52 AM · Restricted Project, Restricted Project
beanz committed rG0c84dafd6b52: [CMake] Feed BUNDLE_PATH through llvm target wrappers (authored by beanz).
[CMake] Feed BUNDLE_PATH through llvm target wrappers
May 31 2019, 10:39 AM

May 30 2019

beanz committed rG760a9ee63c9c: Support codesigning bundles and forcing (authored by beanz).
Support codesigning bundles and forcing
May 30 2019, 3:26 PM
beanz created D62693: Support codesigning bundles and forcing.
May 30 2019, 10:47 AM · Restricted Project, Restricted Project

May 29 2019

beanz accepted D62637: [CMake] Set LLVM_PATH in the runtimes build.

Looks reasonable to me.

May 29 2019, 6:03 PM · Restricted Project
beanz committed rG96c500aab4f6: [CMake] [Runtimes] Set *_STANDALONE_BUILD (authored by beanz).
[CMake] [Runtimes] Set *_STANDALONE_BUILD
May 29 2019, 11:37 AM

May 28 2019

beanz added inline comments to D59334: [TSan][libdispatch] Enable linking and running of tests on Linux.
May 28 2019, 4:35 PM · Restricted Project, Restricted Project

May 24 2019

beanz created D62410: [CMake] [Runtimes] Set *_STANDALONE_BUILD.
May 24 2019, 10:28 AM · Restricted Project
beanz added a comment to D62353: [cmake] Add new LLVM_CACHE_VARIABLES variable to contains all variables passed to cmake on the commandline or in cache files..

Btw, where would you want to put them?

Pre-monorepo, it would depend on what the bot is building, but with the mono-repo we could just create a new top-level directory to contain them all. Name each file to match the bot config it runs.

May 24 2019, 10:20 AM · Restricted Project
beanz accepted D62343: [cmake] Remove old unused version of FindZ3.cmake from clang [NFC].

LGTM.

May 24 2019, 10:11 AM · Restricted Project
beanz updated subscribers of D62353: [cmake] Add new LLVM_CACHE_VARIABLES variable to contains all variables passed to cmake on the commandline or in cache files..

I have the same concerns as @labath, so we should certainly put it behind a flag.

May 24 2019, 9:59 AM · Restricted Project
beanz committed rG07745a131fa9: [CMake] Fix issues building runtimes (authored by beanz).
[CMake] Fix issues building runtimes
May 24 2019, 9:23 AM

May 23 2019

beanz added a comment to D62279: Use LTO capable linker.

@winksaville I've figured out how to resolve the gtest issue, but unfortunately that isn't good enough to get the check-runtimes target working. A change went in back in February (rL353601), which breaks running compiler-rt's tests when building a distribution in non-trivial ways, which will unfortunately be difficult to resolve.

May 23 2019, 1:23 PM · Restricted Project, Restricted Project
beanz added a comment to D57992: [CMake] Don't set <PROJECT>_STANDALONE_BUILD.

I know this is an old change, but there are actually some big problems with this. Compiler-RT in particular uses COMPILER_RT_STANDALONE_BUILD=Off to basically mean clang is a target in-tree to depend on. This completely breaks being able to run the compiler-rt tests from the runtimes directory because a bunch of the tests add dependencies on clang. See clang_compile and its uses in CompilerRTCompile.cmake.

May 23 2019, 1:20 PM · Restricted Project, Restricted Project
beanz added a comment to D62279: Use LTO capable linker.

Even with both gtest patches it still isn't working, ninja check-all is failing as before :(

May 23 2019, 12:40 PM · Restricted Project, Restricted Project
beanz committed rGe836096f01f1: [CMake] Fixing errors in r361513 (authored by beanz).
[CMake] Fixing errors in r361513
May 23 2019, 11:52 AM
beanz created D62336: [CMake] Fixing errors in r361513.
May 23 2019, 11:45 AM · Restricted Project
beanz accepted D62289: CMake: allow using LLVM_EXTERNAL_PROJECTS with LLVM_ENABLE_PROJECTS.

Makes sense to me.

May 23 2019, 10:38 AM · Restricted Project
beanz added a comment to D62279: Use LTO capable linker.

I've add gtest_main and gtest to CLANG_BOOTSTRAP_TARGETS as a guess, because that's where check-all is defined:
...
My guess is likely wrong, what do you advise?

May 23 2019, 10:29 AM · Restricted Project, Restricted Project
beanz committed rGc5ec2a2bc198: [CMake] Copy C++ headers before configuring runtimes build (authored by beanz).
[CMake] Copy C++ headers before configuring runtimes build
May 23 2019, 10:05 AM
beanz added a comment to D62215: Fixes to distribution example for X86_64 Arch Linux.

IIRC, on linux if LTO is ON I had to use ld.gold for both stage 1 and 2, although that maybe
just the way it ended up and it wasn't necessary. But I'll test whatever you come up with.

May 23 2019, 10:00 AM · Restricted Project, Restricted Project

May 22 2019

beanz added a comment to D62264: [CommandLine] WIP: Add new debug_opt that reduces down to size of DataType in Release builds..

A very interesting idea. A few thoughts inline.

May 22 2019, 3:09 PM · Restricted Project
beanz committed rGed0036796164: [Runtimes] If LLVM_INCLUDE_TESTS=On depend on gtest (authored by beanz).
[Runtimes] If LLVM_INCLUDE_TESTS=On depend on gtest
May 22 2019, 2:41 PM
beanz updated the diff for D62269: [Runtimes] If LLVM_INCLUDE_TESTS=On depend on gtest.

Fixing this patch

May 22 2019, 2:15 PM · Restricted Project
beanz added a comment to D62269: [Runtimes] If LLVM_INCLUDE_TESTS=On depend on gtest.

Just came to my attention this explodes on some versions of CMake. I'm looking at it and will update once I find a better approach.

May 22 2019, 2:10 PM · Restricted Project
beanz added a comment to D62215: Fixes to distribution example for X86_64 Arch Linux.

D62269 should fix the missing dependency on gtest.

May 22 2019, 1:43 PM · Restricted Project, Restricted Project
beanz created D62269: [Runtimes] If LLVM_INCLUDE_TESTS=On depend on gtest.
May 22 2019, 1:43 PM · Restricted Project
beanz added a comment to D62215: Fixes to distribution example for X86_64 Arch Linux.

With this error I guessed that I needed to build LLVMgold.so, but then I also determined
I needed to link everything with ld.gold to complete the build process, which is why I
uploaded this patch.

May 22 2019, 1:42 PM · Restricted Project, Restricted Project

May 21 2019

beanz committed rGb5417301917f: Fix target property to make BUILD_SHARED_LIBS work (authored by beanz).
Fix target property to make BUILD_SHARED_LIBS work
May 21 2019, 4:50 PM
beanz added a comment to rGdbc2a12c7311: Fix BUILD_SHARED_LIBS for clang which broke in D61909.

@sbc100, I think I see what I did wrong in my last attempt at a fix. I was reading the wrong CMake property. rL361334 should resolve that. Can you give it a try and let me know if that resolves the problem for you?

May 21 2019, 4:50 PM
beanz added a comment to D61909: Add Clang shared library with C++ exports.

@beanz But if libclang_shared is intended to be a shippable binary and BUILD_SHARED_LIBS is only intended to be an option used in developer builds, and libclang_shared while not causing conflicts (thanks for the info on how that works by the way) would be redundant in a local developer build one would see BUILD_SHARED_LIBS enabled in wouldn't it? And also to me it doesn't really make sense to enable the intended shippable binary in a build that is specifically enabling a developer option, and so is obviously not intended for shipment (I get that in the arch case it is intended for shipment but that case is them using an option they shouldn't not the option being one that should be used when the built products are intended for redistribution).

May 21 2019, 4:34 PM · Restricted Project
beanz added a comment to rGdbc2a12c7311: Fix BUILD_SHARED_LIBS for clang which broke in D61909.

@sbc100 they do not reproduce for me locally. They are Linux-specific and I’m on Darwin.

May 21 2019, 4:30 PM
beanz added a comment to D61909: Add Clang shared library with C++ exports.

@beanz Well I took libclang_shared as effectively an equivalent to the libLLVM.so that's created with that dylib option, and when BUILD_SHARED_LIBS is enabled that library is not created, in fact the option to create that library conflicts with BUILD_SHARED_LIBS.

That conflict is because of how I implemented libLLVM, and is something I'm looking to change. I'm hoping to put up a patch this week that will update how we generate libLLVM to be more like how libclang_shared is built.

May 21 2019, 2:34 PM · Restricted Project
beanz added a comment to D62215: Fixes to distribution example for X86_64 Arch Linux.

Adding "libcxxabi" to LLVM_ENABLE_RUNTIMES is fine, but the other changes to the DistributionExample are only needed because you chose to use gold, which is a configuration-specific decision that is not representative of how most people will build, therefore it shouldn't be in the example.

May 21 2019, 2:16 PM · Restricted Project, Restricted Project
beanz committed rGfb2a0765118d: [CMake] One more stab at fixing BUILD_SHARED_LIBS (authored by beanz).
[CMake] One more stab at fixing BUILD_SHARED_LIBS
May 21 2019, 10:29 AM
beanz accepted D62172: [cmake] Add custom command to touch archives so ninja won't rebuild them..

One small change needed, otherwise LGTM.

May 21 2019, 10:01 AM · Restricted Project
beanz added a comment to D61608: Fix YAML parser's Document::skip for null nodes.

@Bigcheese, ping?

May 21 2019, 9:54 AM · Restricted Project
beanz added a comment to D62172: [cmake] Add custom command to touch archives so ninja won't rebuild them..

It is silly that CMake doesn't expose the ninja version, but it is easy enough to get it. It should be something like:

May 21 2019, 9:37 AM · Restricted Project
beanz committed rGda60a16bc7fb: [docs] Add new document on building distributions (authored by beanz).
[docs] Add new document on building distributions
May 21 2019, 9:28 AM
beanz added a comment to D62040: [docs] Add new document on building distributions.

This has had plenty of time to percolate, so I'm going to land it shorty. Any additional feedback we can incorporate in follow-up patches.

May 21 2019, 9:27 AM · Restricted Project
beanz committed rGdbc2a12c7311: Fix BUILD_SHARED_LIBS for clang which broke in D61909 (authored by beanz).
Fix BUILD_SHARED_LIBS for clang which broke in D61909
May 21 2019, 8:56 AM
beanz added a comment to D61909: Add Clang shared library with C++ exports.

@E5ten, I just pushed r361271, which should resolve your issue.

May 21 2019, 8:55 AM · Restricted Project
beanz added a comment to D61909: Add Clang shared library with C++ exports.

@E5ten, I see what is going on. Give me 30 minutes and I’ll have a fix pushed.

May 21 2019, 8:35 AM · Restricted Project
beanz accepted D62176: [CMake] Specify component for all target types.
May 21 2019, 12:10 AM · Restricted Project

May 20 2019

beanz added a comment to D62172: [cmake] Add custom command to touch archives so ninja won't rebuild them..

I have a long story about this issue... Ask me about it sometime :).

May 20 2019, 10:48 PM · Restricted Project
beanz accepted D62171: build: Enable CMake Policy 0077.
May 20 2019, 10:09 PM · Restricted Project
beanz added a comment to D62171: build: Enable CMake Policy 0077.

@hintonda also, re-running the CMake command line and setting -D... is always a FORCE

May 20 2019, 10:09 PM · Restricted Project
beanz added a comment to D62172: [cmake] Add custom command to touch archives so ninja won't rebuild them..

@thakis Didn't we patch Ninja for this?

May 20 2019, 9:54 PM · Restricted Project
beanz added a comment to D62063: CMake changes to get Windows self-host with PGO working.

The CMake goop all looks reasonable to me. I think the new "correct" way to alter the compiler flags is add_compile_options, but since LLVM pretty heavily relies on modifying CMAKE_<LANG>_FLAGS it is probably better to be consistent than to start doing something new here.

May 20 2019, 9:49 PM · Restricted Project
beanz updated the diff for D62155: [CMake] Copy C++ headers before configuring runtimes build.

Thank goodness @smeenai can spell because obviously I can't.

May 20 2019, 9:29 PM · Restricted Project, Restricted Project
beanz retitled D62155: [CMake] Copy C++ headers before configuring runtimes build from [CMake] Copy C++ headers during config on Darwin to [CMake] Copy C++ headers before configuring runtimes build.
May 20 2019, 4:12 PM · Restricted Project, Restricted Project
beanz added a reviewer for D62155: [CMake] Copy C++ headers before configuring runtimes build: EricWF.

Looping in @EricWF.

May 20 2019, 4:12 PM · Restricted Project, Restricted Project
beanz updated the diff for D62155: [CMake] Copy C++ headers before configuring runtimes build.

A new take on how to do this, even more fun this time!

May 20 2019, 4:07 PM · Restricted Project, Restricted Project
beanz added a comment to D62155: [CMake] Copy C++ headers before configuring runtimes build.

I just came up with a different and slightly crazier approach to this. I'm going to test it out and will upload a new version of this patch shortly.

May 20 2019, 3:57 PM · Restricted Project, Restricted Project
beanz updated the diff for D62040: [docs] Add new document on building distributions.

Updates based on feedback from @smeenai

May 20 2019, 12:48 PM · Restricted Project
beanz updated the diff for D62155: [CMake] Copy C++ headers before configuring runtimes build.

Updates based on review feedback.

May 20 2019, 12:42 PM · Restricted Project, Restricted Project
beanz added inline comments to D62155: [CMake] Copy C++ headers before configuring runtimes build.
May 20 2019, 12:33 PM · Restricted Project, Restricted Project
beanz added a reviewer for D62105: [CommandLine] Remove OptionCategory and SubCommand caches from the Option class.: zturner.

Adding @zturner in case he has any thoughts.

May 20 2019, 11:12 AM · Restricted Project, Restricted Project
beanz added a comment to D62040: [docs] Add new document on building distributions.

@winksaville I just committed an update to the DistributionExamples a minute ago that should get them working as long as you aren't using a Darwin host. If you are using a Darwin host you need D62155 as well.

May 20 2019, 11:10 AM · Restricted Project
beanz committed rGc1ad143f95da: [CMake] Update DistributionExample for mono repo (authored by beanz).
[CMake] Update DistributionExample for mono repo
May 20 2019, 11:09 AM
beanz created D62155: [CMake] Copy C++ headers before configuring runtimes build.
May 20 2019, 11:05 AM · Restricted Project, Restricted Project

May 19 2019

beanz added a comment to D62040: [docs] Add new document on building distributions.

@winksaville, sorry those cache files pre-date the monorepo, and they haven’t been updated to support it. I will try and get a patch to update them today.

May 19 2019, 2:45 PM · Restricted Project

May 18 2019

beanz added inline comments to D62091: [CommandLine] Reduce size of Option class.
May 18 2019, 11:16 AM · Restricted Project
beanz added inline comments to D62091: [CommandLine] Reduce size of Option class.
May 18 2019, 11:04 AM · Restricted Project
beanz accepted D62091: [CommandLine] Reduce size of Option class.
May 18 2019, 10:58 AM · Restricted Project
beanz added a comment to D62091: [CommandLine] Reduce size of Option class.

The main thing is to make sure that if you have proper integer sized members they are properly aligned and packed, which this seems to do well.

May 18 2019, 10:58 AM · Restricted Project

May 17 2019

beanz added a comment to D62091: [CommandLine] Reduce size of Option class.

This is pretty cool. Since these are usually allocated as globals, this is saving pages of dirtied memory.

May 17 2019, 7:30 PM · Restricted Project
beanz accepted D62032: build: use clang-cl for runtimes when targeting Windows.

LGTM. Makes sense.

May 17 2019, 1:02 PM · Restricted Project
beanz added a comment to D61909: Add Clang shared library with C++ exports.

@sylvestre.ledru, I’ve added you on D62040, which is an attempt to document best practices for generating releases.

May 17 2019, 11:57 AM · Restricted Project
beanz added a reviewer for D62040: [docs] Add new document on building distributions: sylvestre.ledru.
May 17 2019, 11:56 AM · Restricted Project
beanz added a comment to D61909: Add Clang shared library with C++ exports.

I would leave it out of any distribution (at least for now).

May 17 2019, 11:16 AM · Restricted Project
beanz added a comment to D62040: [docs] Add new document on building distributions.

Hmm, I've never set LLVM_ENABLE_RUNTIMES, I just build with a previously built clang. The script lives in utils/release/build_llvm_package.bat

May 17 2019, 11:07 AM · Restricted Project
beanz added a comment to D62040: [docs] Add new document on building distributions.

The Windows packages currently includes at least the asan runtime (and a bunch of other stuff like openmp that people ask for but I'm never quite sure if it works).

May 17 2019, 10:40 AM · Restricted Project
beanz added a comment to D62040: [docs] Add new document on building distributions.

@hans I have some ideas on how to make package work. There is a simple solution to make it work as long as you're not using any runtime components, but I have an idea on how to make it work with runtime components.

May 17 2019, 10:28 AM · Restricted Project
beanz updated the diff for D62040: [docs] Add new document on building distributions.

Fixing the wording around LLVM_INSTALL_TOOLCHAIN_ONLY

May 17 2019, 9:32 AM · Restricted Project
beanz added inline comments to D62040: [docs] Add new document on building distributions.
May 17 2019, 9:25 AM · Restricted Project
beanz added a comment to D62040: [docs] Add new document on building distributions.

But the install target is kind of a standard cmake thing; wouldn't it be better if we made that target good? That's also what the cmake "package" target uses, which is what we use to build the Windows installer.

May 17 2019, 9:24 AM · Restricted Project

May 16 2019

beanz committed rGa971003e4677: Revert Refactor constant evaluation of typeid(T) to track a symbolic type_info… (authored by beanz).
Revert Refactor constant evaluation of typeid(T) to track a symbolic type_info…
May 16 2019, 10:47 PM