Page MenuHomePhabricator

beanz (Chris Bieneman)
User

Projects

User does not belong to any projects.

User Details

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

Recent Activity

Thu, Sep 19

beanz committed rG7cb60fb00f54: Make appendCallNB lambda mutable (authored by beanz).
Make appendCallNB lambda mutable
Thu, Sep 19, 8:45 AM

Sat, Sep 14

beanz added a comment to D67585: [clang] [cmake] Make building dylib optional.

But in that case, we should aim for consistency, i.e. remove the matching option from LLVM.

Sat, Sep 14, 11:49 AM
beanz added a reviewer for D67585: [clang] [cmake] Make building dylib optional: compnerd.

This is really much more work than disabling the one component I don't need or care for, especially when I do shared lib build and therefore the additional library is entirely useless.

Sat, Sep 14, 11:37 AM
beanz added a comment to D67585: [clang] [cmake] Make building dylib optional.

Sure but I want to build-test practically everything else. I wouldn't mind if this didn't basically kill my test system by resource exhaustion

Sat, Sep 14, 9:23 AM
beanz requested changes to D67585: [clang] [cmake] Make building dylib optional.

Please no. You don’t need to build the library. ‘check-clang’ doesn’t depend on it, so it should not impact your build and test cycles. We want it included in the ‘all’ target for every possible configuration so that it gets tested and not broken as frequently as the LLVM dylib target has in the past.

Sat, Sep 14, 9:07 AM

Wed, Sep 11

beanz committed rG393b4eac4955: All Errors must be checked (authored by beanz).
All Errors must be checked
Wed, Sep 11, 1:55 PM

Tue, Sep 10

beanz updated the diff for D67407: All Errors must be checked.

Updates based on feedback from Lang.

Tue, Sep 10, 5:09 PM · Restricted Project
beanz created D67407: All Errors must be checked.
Tue, Sep 10, 10:25 AM · Restricted Project

Mon, Sep 9

beanz added a comment to D67321: Respect CLANG_LINK_CLANG_DYLIB=ON in libclang and c-index-test.

@aaronpuchert sounds like a reasonable approach

Mon, Sep 9, 5:45 PM · Restricted Project
beanz accepted D67195: Adding support for overriding LLVM_ENABLE_RUNTIMES for runtimes builds..

I see. Sorry for the noise. This looks fine.

Mon, Sep 9, 4:52 PM · Restricted Project
beanz requested changes to D67321: Respect CLANG_LINK_CLANG_DYLIB=ON in libclang and c-index-test.
Mon, Sep 9, 12:32 PM · Restricted Project

Thu, Sep 5

beanz added a comment to D67195: Adding support for overriding LLVM_ENABLE_RUNTIMES for runtimes builds..

@compnerd, it doesn't look like LLVM_ENABLE_RUNTIMES passed globally is still passed through if you are generating runtimes for multiple targets. It looks like that only gets passed through if you specify RUNTIMES_${name}_LLVM_ENABLE_RUNTIMES.

Thu, Sep 5, 6:41 PM · Restricted Project

Wed, Sep 4

beanz added a comment to D67195: Adding support for overriding LLVM_ENABLE_RUNTIMES for runtimes builds..

I feel like the default behavior should be that if you set LLVM_ENABLE_RUNTIMES it builds them for all targets you request. If you want to disable some runtimes for certain targets that should be an opt-out setting rather than an opt-in to build for targets.

Wed, Sep 4, 5:03 PM · Restricted Project

Wed, Aug 28

beanz accepted D66901: [ObjectYAML] Fix lifetime issue in dumpDebugLines.
Wed, Aug 28, 6:10 PM · Restricted Project
beanz added inline comments to D66901: [ObjectYAML] Fix lifetime issue in dumpDebugLines.
Wed, Aug 28, 1:20 PM · Restricted Project

Aug 20 2019

beanz committed rG8d1838480995: Autogenerate the shebang lines for tools/opt-viewer (authored by beanz).
Autogenerate the shebang lines for tools/opt-viewer
Aug 20 2019, 6:49 PM
beanz added a comment to D65983: Autogenerate the shebang lines for tools/opt-viewer.

Landed in rL369486

Aug 20 2019, 6:48 PM · Restricted Project
beanz added a comment to D65983: Autogenerate the shebang lines for tools/opt-viewer.

I'm happy to commit this for you. Will do so shortly.

Aug 20 2019, 6:48 PM · Restricted Project
beanz added a comment to D65983: Autogenerate the shebang lines for tools/opt-viewer.

Seems reasonable to me. @anemet any concerns?

Aug 20 2019, 3:53 PM · Restricted Project
beanz added a comment to D66068: cmake: Make building clang-shlib optional.

sorry for the delay. I fully understand the need to reduce the number of options. Having always used SHARED_LIBS build I remember weekly shared build breakages.

Aug 20 2019, 3:51 PM · Restricted Project

Aug 16 2019

beanz added a comment to D65983: Autogenerate the shebang lines for tools/opt-viewer.

It actually worked with the env for me.

Aug 16 2019, 3:23 PM · Restricted Project
beanz added a comment to D65983: Autogenerate the shebang lines for tools/opt-viewer.

Sadly I'm not sure that there is a better way than generating the shebang line, but I can imagine some problems with the way this patch does it. PYTHON_EXECUTABLE could be a full path, which means that this can't be used to generate binary distributions because you'd be relying on python being in the same place on disk, which defeats the purpose of env being on the shebang.

Aug 16 2019, 3:05 PM · Restricted Project

Aug 15 2019

beanz accepted D66326: Fix llvm-config support for CMake build-mode-style builds.

This fix looks good, although I would suggest that when building libcxx, using the LLVM CMake exports should be more reliable than llvm-confit.

Aug 15 2019, 10:22 PM · Restricted Project
beanz committed rG450f5f3986ce: Correcting clang-cpp release not to spcify supported targets. (authored by beanz).
Correcting clang-cpp release not to spcify supported targets.
Aug 15 2019, 9:56 AM

Aug 14 2019

beanz added a reviewer for D66068: cmake: Make building clang-shlib optional: compnerd.

It duplicates functionality provided by separate/component libraries.

Aug 14 2019, 11:36 AM · Restricted Project
beanz committed rGa8e070366a4d: [CMake] Fix cache invalidation of LLVM_CXX_STD (authored by beanz).
[CMake] Fix cache invalidation of LLVM_CXX_STD
Aug 14 2019, 11:27 AM
beanz committed rG201b879fd7fe: Merging release note update in r368874 (authored by beanz).
Merging release note update in r368874
Aug 14 2019, 11:12 AM
beanz committed rGa80a3a2b2395: Document clang-cpp in the release notes for clang (authored by beanz).
Document clang-cpp in the release notes for clang
Aug 14 2019, 9:50 AM

Aug 13 2019

beanz added a comment to D66068: cmake: Make building clang-shlib optional.

I want to dissect this a bit.

Aug 13 2019, 12:41 PM · Restricted Project
beanz added a comment to D66068: cmake: Make building clang-shlib optional.

That's your workflow. I need to run 'make install' because the modifications are used by an external project. If there's an option to skip shlib during 'make install' I'd be happy to use it.

Aug 13 2019, 10:13 AM · Restricted Project

Aug 12 2019

beanz added a comment to D66068: cmake: Make building clang-shlib optional.

I think you and I disagree here. General developer workflows don't need to include building all for every minor change. In my normal workflow I just re-run check-llvm or check-clang over and over again, only building the all target before I post a patch. With that workflow I only build the library once per-patch to ensure that it builds. Which is exactly the goal of not having it be included.

Aug 12 2019, 3:01 PM · Restricted Project
beanz added a comment to D66068: cmake: Make building clang-shlib optional.

I generally am not a fan of adding more and more options. As long as you're not linking the library it won't be generated by any of the check targets. With the llvm dylib we've had many issues over the years where changes to LLVM break building the dylib and many developers don't build it, so we have to wait for a bot to catch it. Making the clang dylib build as part of all in every possible build configuration is a recognition that this is something we ship and we should ensure works.

Aug 12 2019, 10:30 AM · Restricted Project

Aug 6 2019

beanz committed rG96655b32d8ed: Add order-dependencies to object libraries (authored by beanz).
Add order-dependencies to object libraries
Aug 6 2019, 12:43 PM
beanz created D65818: Add order-dependencies to object libraries.
Aug 6 2019, 10:18 AM · Restricted Project

Aug 5 2019

beanz added a comment to D64032: [cmake] With utils disabled, don't build tblgen in cross mode.

I don't know of any way to emulate that because it isn't something CMake exports or even knows about. It is something the underlying build system detects from the compiler's depfiles. One thought that might work would be to generate an object library target containing the tool source files, and instead depend on the object library and all the target static archives. I think that would get the ordering right and would not require the link step to run.

Aug 5 2019, 2:44 PM · Restricted Project
beanz added a comment to D64032: [cmake] With utils disabled, don't build tblgen in cross mode.

You'd need to make the NATIVE target depend on all the source files and headers that the the target depends on, which is theoretically possible, although I'm not sure of a clean way to do it in CMake.

Aug 5 2019, 12:27 PM · Restricted Project
beanz added a comment to D64032: [cmake] With utils disabled, don't build tblgen in cross mode.

This patch is effectively un-done as of rL367895. I'm happy to discuss alternative approaches to the problem you're trying to solve, but we need correct builds, which means we need the dependency between the native and target tool.

Aug 5 2019, 11:10 AM · Restricted Project
beanz committed rGcd26b1ae2c91: NFC. Documenting Native tablegen dependency (authored by beanz).
NFC. Documenting Native tablegen dependency
Aug 5 2019, 11:04 AM
beanz added a comment to D64032: [cmake] With utils disabled, don't build tblgen in cross mode.

This patch is actually really problematic. We have the native tablegen depend on the target tablegen so that the native one rebuilds whenever the sources for the target one rebuild. Without that dependency the native tablegen may not rebuild at all, or (if it does) it will likely rebuild at the wrong time.

Aug 5 2019, 10:56 AM · Restricted Project
beanz committed rG3c0c6e5c50d6: NATIVE tablegen needs to depend on target tablegen (authored by beanz).
NATIVE tablegen needs to depend on target tablegen
Aug 5 2019, 10:55 AM

Aug 1 2019

beanz accepted D65559: Change /build to /build* in top-level .gitignore.

Yes please! I actually have this locally.

Aug 1 2019, 4:50 PM · Restricted Project

Jul 26 2019

beanz added a comment to D65295: [opt] Ensure the IR-Linker is available to plugins..

If this is the proper way, why do we even support add_llvm_library(${NAME} MODULE without PLUGIN_TOOL?

Jul 26 2019, 2:15 PM · Restricted Project
beanz accepted D65323: [compiler-rt] Fix running tests on macOS when XCode is not installed.

LGTM.

Jul 26 2019, 2:10 PM · Restricted Project, Restricted Project
beanz requested changes to D65270: [CMake] Fix source path generation for install in multi-config (MSBuild).
Jul 26 2019, 2:10 PM · Restricted Project, Restricted Project, Restricted Project
beanz added inline comments to D28855: [CMake] Copy per-component `required_libraries` into `LINK_COMPONENTS`. NFC..
Jul 26 2019, 10:56 AM · Restricted Project
beanz accepted D65045: [CMake] Allow LLVM_EXTERNAL_<proj>_SOURCE_DIR to be overridden if it is empty..

LGTM. At some point (after the mono-repo becomes the source of truth) we need to clean all this up.

Jul 26 2019, 9:50 AM · Restricted Project
beanz requested changes to D65295: [opt] Ensure the IR-Linker is available to plugins..

The CMake build system option PLUGIN_TOOL on add_llvm_loadable_module is for this specific purpose, it instructs the link step for the plugin dylib to resolve symbols against the tool.

Jul 26 2019, 9:46 AM · Restricted Project
beanz added inline comments to D65323: [compiler-rt] Fix running tests on macOS when XCode is not installed.
Jul 26 2019, 9:22 AM · Restricted Project, Restricted Project

Jul 20 2019

beanz requested changes to D65045: [CMake] Allow LLVM_EXTERNAL_<proj>_SOURCE_DIR to be overridden if it is empty..

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

Jul 20 2019, 8:28 PM · Restricted Project

Jul 17 2019

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

Jul 11 2019

beanz accepted D64278: Rename libclang_shared to libclang-cpp.

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

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

LGTM.

Jul 11 2019, 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.

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

LGTM.

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

Jul 9 2019

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.

Jul 9 2019, 9:51 AM · Restricted Project

Jul 8 2019

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?

Jul 8 2019, 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
Jul 8 2019, 11:34 AM

Jul 7 2019

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

Jul 5 2019

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.

Jul 5 2019, 5:20 AM · Restricted Project
beanz requested changes to D63881: [cmake] Fix build with BUILD_SHARED_LIBS=ON .
Jul 5 2019, 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.

Jul 5 2019, 4:40 AM · Restricted Project

Jul 2 2019

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

One comment inline. Otherwise LGTM.

Jul 2 2019, 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