Page MenuHomePhabricator

beanz (Chris Bieneman)
User

Projects

User does not belong to any projects.

User Details

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

Recent Activity

Today

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

Yesterday

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 :).

Mon, May 20, 10:48 PM · Restricted Project
beanz accepted D62171: build: Enable CMake Policy 0077.
Mon, May 20, 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

Mon, May 20, 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?

Mon, May 20, 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.

Mon, May 20, 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.

Mon, May 20, 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.
Mon, May 20, 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.

Mon, May 20, 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!

Mon, May 20, 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.

Mon, May 20, 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

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

Updates based on review feedback.

Mon, May 20, 12:42 PM · Restricted Project, Restricted Project
beanz added inline comments to D62155: [CMake] Copy C++ headers before configuring runtimes build.
Mon, May 20, 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.

Mon, May 20, 11:12 AM · 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.

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

Sun, May 19

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.

Sun, May 19, 2:45 PM · Restricted Project

Sat, May 18

beanz added inline comments to D62091: [CommandLine] Reduce size of Option class.
Sat, May 18, 11:16 AM · Restricted Project
beanz added inline comments to D62091: [CommandLine] Reduce size of Option class.
Sat, May 18, 11:04 AM · Restricted Project
beanz accepted D62091: [CommandLine] Reduce size of Option class.
Sat, May 18, 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.

Sat, May 18, 10:58 AM · Restricted Project

Fri, May 17

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.

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

LGTM. Makes sense.

Fri, May 17, 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.

Fri, May 17, 11:57 AM · Restricted Project
beanz added a reviewer for D62040: [docs] Add new document on building distributions: sylvestre.ledru.
Fri, May 17, 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).

Fri, May 17, 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

Fri, May 17, 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).

Fri, May 17, 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.

Fri, May 17, 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

Fri, May 17, 9:32 AM · Restricted Project
beanz added inline comments to D62040: [docs] Add new document on building distributions.
Fri, May 17, 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.

Fri, May 17, 9:24 AM · Restricted Project

Thu, May 16

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…
Thu, May 16, 10:47 PM
beanz committed rGa5a4124c494c: Revert [c++20] P1327R1: Support for typeid applied to objects of polymorphic… (authored by beanz).
Revert [c++20] P1327R1: Support for typeid applied to objects of polymorphic…
Thu, May 16, 10:46 PM
beanz added a comment to D62033: Change llvm_add_library to always use name for static libraries.

LLVM generally has a policy of not leaving dead or unused code in tree. Changing to an error seems like the right approach given our coding guidelines.

Thu, May 16, 9:19 PM · Restricted Project
beanz committed rG876e39937efb: Re-land: Add Clang shared library with C++ exports (authored by beanz).
Re-land: Add Clang shared library with C++ exports
Thu, May 16, 9:18 PM
beanz added inline comments to D62040: [docs] Add new document on building distributions.
Thu, May 16, 8:43 PM · Restricted Project
beanz updated the diff for D62040: [docs] Add new document on building distributions.

Updates based on @winksaville's feedback

Thu, May 16, 8:43 PM · Restricted Project
beanz added a comment to D62033: Change llvm_add_library to always use name for static libraries.

I don't know of anywhere that we use llvm_add_library to generate both static and shared libraries of the same code. The runtime libraries that are generated both ways roll their own solution to the problem.

Thu, May 16, 5:03 PM · Restricted Project
beanz added a comment to D62033: Change llvm_add_library to always use name for static libraries.

I don't know that we want to support building component libraries as both STATIC and SHARED except in very specific and limited situations.

Thu, May 16, 4:15 PM · Restricted Project
beanz updated the diff for D62040: [docs] Add new document on building distributions.

Adding information about distribution targets and LLVM_INSTALL_TOOLCHAIN_ONLY.

Thu, May 16, 4:09 PM · Restricted Project
beanz added a comment to D62040: [docs] Add new document on building distributions.

This is great! Might it be worth mentioning LLVM_INSTALL_TOOLCHAIN_ONLY somewhere?

Thu, May 16, 4:01 PM · Restricted Project
beanz created D62040: [docs] Add new document on building distributions.
Thu, May 16, 3:38 PM · Restricted Project
beanz committed rG10fba12e504d: Add Clang shared library with C++ exports (authored by beanz).
Add Clang shared library with C++ exports
Thu, May 16, 3:04 PM
beanz added a comment to D62033: Change llvm_add_library to always use name for static libraries.

Why are the clang libraries being built STATIC and SHARED?

Thu, May 16, 3:00 PM · Restricted Project
beanz added a comment to D61909: Add Clang shared library with C++ exports.

I will land this patch as-is today. I'll try and get a doc patch out for review today or tomorrow. @winksaville, I'm going to add a section to the document about building LLVM as a shared library for inclusion with tool distributions.

Thu, May 16, 3:00 PM · Restricted Project
beanz accepted D62032: build: use clang-cl for runtimes when targeting Windows.

LGTM.

Thu, May 16, 2:55 PM · Restricted Project
beanz added a comment to D61909: Add Clang shared library with C++ exports.

Please add documentation if you want people to use it :)

Thu, May 16, 11:10 AM · Restricted Project
beanz added a comment to D61909: Add Clang shared library with C++ exports.

I should note, this and many more aspects of the build system were topics I discussed in a talk at the 2016 LLVM Dev meeting when we switched off autoconf:
https://www.youtube.com/watch?v=StF77Cx7pz8

Thu, May 16, 10:44 AM · Restricted Project
beanz added a comment to D61909: Add Clang shared library with C++ exports.

There is a simpler example distribution configuration, but sadly there isn't documentation. That is something I can fix.

Thu, May 16, 10:41 AM · Restricted Project

Wed, May 15

beanz added a comment to D61909: Add Clang shared library with C++ exports.

Sorry, maybe I didn't make myself clear.

Wed, May 15, 3:11 PM · Restricted Project
beanz added a comment to D61870: Make cl::HideUnrelatedOptionsless error-prone.

@beanz
As pointed out by @hintonda, current default argument is *never* explicitly used in the codebase.

Wed, May 15, 1:08 PM · Restricted Project
beanz added a comment to D61909: Add Clang shared library with C++ exports.

IMHO "BUILD_CLANG_DYLIB" is needed. As you have it now libclang_shared.so is always builds on UNIX systems, which I believe means that all linux distros would have both increasing their sizes.

Wed, May 15, 12:36 PM · Restricted Project
beanz added a comment to D61870: Make cl::HideUnrelatedOptionsless error-prone.

This patch doesn't actually include how the new code will be used, and the review doesn't include enough context for me to understand what is going on.

Wed, May 15, 11:26 AM · Restricted Project
beanz added a comment to D61909: Add Clang shared library with C++ exports.

Questions:

  • Should we only build libclang_shared.so if LLVM_BUILD_LLVM_DYLIB is ON?

LLVM_BUILD_LLVM_DYLIB is actually not the important option to think about because it has no impact on this, rather LLVM_LINK_LLVM_DYLIB. That really depends on what tradeoffs you want. With that option off LLVM will be linked statically into the Clang library, which in the common case is fine because you can resolve all the LLVM & Clang APIs against it. That will be faster to load, but a larger binary distribution. With the option on, libclang_shared will link libLLVM, which will result in slower loading times, but smaller distributions. Both are workable situations.

Wed, May 15, 11:09 AM · Restricted Project

Tue, May 14

beanz updated the diff for D61909: Add Clang shared library with C++ exports.

Changed to lowercase 'c' to match other clang libraries.

Tue, May 14, 1:49 PM · Restricted Project
beanz added a comment to D61909: Add Clang shared library with C++ exports.

@winksaville whether or not PIC is required for shared libraries varies by platform. These days LLVM defaults to -fPIC, and I'm not sure we actually support running LLVM without -fPIC on systems that require shared libraries to be PIC.

Tue, May 14, 1:49 PM · Restricted Project
beanz added a comment to D61804: Support building shared and static libraries simultaneously.

@winksaville that was my bad. I left off the new files in the commit because I forgot git-diff doesn't grab untracked files...

Tue, May 14, 8:52 AM · Restricted Project, Restricted Project
beanz created D61909: Add Clang shared library with C++ exports.
Tue, May 14, 8:51 AM · Restricted Project

Mon, May 13

beanz added a comment to D61804: Support building shared and static libraries simultaneously.

My change should not have decreased build time from trunk, nor should it have reduced the number of build steps. That patch should generate lib/libClang_shared.so, which will export the C++ API. libClang is unchanged since changing it would break existing clients of the library.

Mon, May 13, 12:55 PM · Restricted Project, Restricted Project
beanz added a comment to D61804: Support building shared and static libraries simultaneously.

When you say you don't think the build system should do this, what do you mean?

Mon, May 13, 8:50 AM · Restricted Project, Restricted Project

Sun, May 12

beanz added a comment to D61804: Support building shared and static libraries simultaneously.

As an additional note, Arch linux should not be building clang with BUILD_SHARED_LIBS nor should it be distributing those ,so files. That isn't a supported configuration for Clang deployment.

Sun, May 12, 8:46 AM · Restricted Project, Restricted Project
beanz added a comment to D61804: Support building shared and static libraries simultaneously.

I question the general utility of this patch, and I don't really think this does anything we want the build system doing.

Sun, May 12, 8:14 AM · Restricted Project, Restricted Project

Fri, May 10

beanz added inline comments to D61660: [cmake] Make google benchmark project handle libraries properly when built in-tree.
Fri, May 10, 12:11 PM · Restricted Project

Thu, May 9

beanz added a comment to D61660: [cmake] Make google benchmark project handle libraries properly when built in-tree.

Using a macro seems reasonable if a bit icky.

Thu, May 9, 10:28 AM · Restricted Project
beanz added a comment to D61660: [cmake] Make google benchmark project handle libraries properly when built in-tree.

I think the two of you are speaking past each other with different goals.

Thu, May 9, 10:07 AM · Restricted Project

Wed, May 8

beanz added inline comments to D61660: [cmake] Make google benchmark project handle libraries properly when built in-tree.
Wed, May 8, 10:00 AM · Restricted Project
beanz added a comment to D61660: [cmake] Make google benchmark project handle libraries properly when built in-tree.

Could you change the title and commit message to reference that this isn't just about using llvm_add_library it is more broadly about making the benchmark library behave like a proper LLVM library target when built in-tree.

Wed, May 8, 9:41 AM · Restricted Project

Tue, May 7

beanz committed rGbec30c4af1e3: [CMake] Detecting python modules should be cached (authored by beanz).
[CMake] Detecting python modules should be cached
Tue, May 7, 2:46 PM

Thu, May 2

beanz accepted D61425: Install import libraries.

Yep. This looks like the right solution. Un-used install destinations are effectively ignored by CMake, so there is no reason to selectively choose only one.

Thu, May 2, 11:05 AM · Restricted Project
beanz added a comment to D61387: [CMake][benchmark] Cache result of CXXFeatureCheck.

Libc++'s test cases aren't buildable without LLVM. Why should the benchmarks be?

Thu, May 2, 10:59 AM · Restricted Project

Wed, May 1

beanz added a comment to D61387: [CMake][benchmark] Cache result of CXXFeatureCheck.

That kinda makes it worse. We don't duplicate gtest in all the tools that use it, why are we duplicating this library?

Wed, May 1, 4:10 PM · Restricted Project
beanz created D61402: [CMake] Detecting python modules should be cached.
Wed, May 1, 2:24 PM · Restricted Project
beanz added a comment to D61387: [CMake][benchmark] Cache result of CXXFeatureCheck.

@lebedev.ri I actually disagree with prefixing capabilities check variables. If other projects are checking the same thing you want them to use the same variable so that they don't all check the same thing over and over again.

Wed, May 1, 2:21 PM · Restricted Project
beanz updated the diff for D61387: [CMake][benchmark] Cache result of CXXFeatureCheck.

Updating based on review feedback.

Wed, May 1, 10:51 AM · Restricted Project
beanz created D61387: [CMake][benchmark] Cache result of CXXFeatureCheck.
Wed, May 1, 10:08 AM · Restricted Project

Tue, Apr 30

beanz accepted D61356: [compiler-rt] Rework the object build support.

This all looks reasonable to me. It is kinda frustrating that CMake doesn't support any way to control the output name of an object file, or even the install name of one. I suspect that is probably because some of CMake's generation targets (like Xcode) don't give CMake explicit control over output object file names.

Tue, Apr 30, 7:31 PM · Restricted Project, Restricted Project

Fri, Apr 26

beanz accepted D48069: [cmake] Disable a GCC optimization when building LLVM for MIPS.

Seems reasonable to me.

Fri, Apr 26, 3:00 PM · Restricted Project

Thu, Apr 25

beanz added a comment to D48069: [cmake] Disable a GCC optimization when building LLVM for MIPS.

Is there a specific version range of GCC that this miscompile occurs with?

Thu, Apr 25, 12:55 PM · Restricted Project

Mon, Apr 22

beanz accepted D60926: [CMake] Replace the sanitizer support in runtimes build with multilib.

I really like this. Using + to add variants seems much widely useful.

Mon, Apr 22, 1:21 PM · Restricted Project, Restricted Project

Apr 19 2019

beanz committed rG2436237895b7: [CMake] Add fuzzer as a component for runtime builds (authored by beanz).
[CMake] Add fuzzer as a component for runtime builds
Apr 19 2019, 1:12 PM
beanz committed rG36228cb63fd5: [CMake] Pass monorepo build settings in cross compile (authored by beanz).
[CMake] Pass monorepo build settings in cross compile
Apr 19 2019, 1:08 PM

Feb 20 2019

beanz accepted D58471: [CMake][runtimes] Set clang-header dependency for builtins.

Makes sense

Feb 20 2019, 2:03 PM · Restricted Project

Feb 14 2019

beanz committed rGe8d95ad9ae3a: [CMake] Fix ability to use LLVM_ENABLE_PROJECTS with LLVM_EXTERNAL_PROJECTS (authored by beanz).
[CMake] Fix ability to use LLVM_ENABLE_PROJECTS with LLVM_EXTERNAL_PROJECTS
Feb 14 2019, 12:57 PM
beanz added inline comments to D57535: [CMake] Use LLVM_ENABLE_PROJECTS as the "single source" of truth when used..
Feb 14 2019, 12:39 PM · Restricted Project

Jan 31 2019

beanz added a reviewer for D57521: [CMake] External compiler-rt-configure requires LLVMTestingSupport when including tests: phosek.

@phosek, does LLVM’s runtime directory do this correctly?

Jan 31 2019, 5:27 PM · Restricted Project, Restricted Project

Jan 11 2019

beanz accepted D56606: [CMake] Export utility targets to the build/install tree depending on LLVM_BUILD/INSTALL_UTILS.

LGTM

Jan 11 2019, 11:28 AM

Jan 7 2019

beanz accepted D49672: [CMake] Honor LLVM_EXTERNAL_<proj>_SOURCE_DIR.

LGTM

Jan 7 2019, 1:43 PM · Restricted Project

Dec 18 2018

beanz added a comment to D54153: Fix compilation issue in VS2017 with Clang-tablegen and LLVM-tablegen.

It is expected that CMake will process all projects put under llvm/tools unless explicitly told not to. So, it is expected that it would process clang there. If you don't want it to process clang you need to set LLVM_TOOL_CLANG_BUILD=Off when configuring.

Dec 18 2018, 3:18 PM
beanz accepted D55816: [CMake] Use XCODE_ATTRIBUTE properties for code signing and entitlements in Xcode.
Dec 18 2018, 9:23 AM
beanz added a comment to D55816: [CMake] Use XCODE_ATTRIBUTE properties for code signing and entitlements in Xcode.

LGTM.

Dec 18 2018, 9:23 AM

Dec 13 2018

beanz added a comment to D55116: [CMake] llvm_codesign workaround for Xcode double-signing errors.

Sorry for being behind on this, but I don't think this is the right solution to the problem.

Dec 13 2018, 4:55 PM

Dec 4 2018

beanz added a comment to D54674: [llvm-objcopy] First bits for MachO .

My $0.02 on the yaml discussion for tests:

Dec 4 2018, 4:50 PM · Restricted Project
beanz added a reviewer for D54674: [llvm-objcopy] First bits for MachO : mtrent.

From an Apple perspective this almost certainly needs @mtrent's eyes, as he owns our binary tools.

Dec 4 2018, 3:06 PM · Restricted Project

Nov 29 2018

beanz added a comment to D55001: [CMake] Fix build with -DLLVM_TOOL_LLVM_{MCA/EXEGESIS}_BUILD=OFF.

How are you instructing lit to not run the tests that rely on these tools? Doesn't seem like you are, so I would expect check-llvm to fail on a clean build configuration due to missing tools.

Nov 29 2018, 9:32 AM

Nov 16 2018

beanz accepted D54443: [CMake] Accept ENTITLEMENTS in add_llvm_executable and llvm_codesign.

LGTM.

Nov 16 2018, 9:57 AM

Nov 14 2018

beanz accepted D54461: [CMake] Support cross-compiling with multi-stage builds.

LGTM!

Nov 14 2018, 12:18 PM

Nov 13 2018

beanz added inline comments to D54443: [CMake] Accept ENTITLEMENTS in add_llvm_executable and llvm_codesign.
Nov 13 2018, 2:19 PM