Page MenuHomePhabricator

beanz (Chris Bieneman)
User

Projects

User does not belong to any projects.

User Details

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

Recent Activity

Tue, Jan 14

beanz accepted D71169: [Debug] Convert DebugOnly from cl::opt with a custom type to cl::list [NFC].

Looks totally reasonable to me, and like a good simplification of the logic.

Tue, Jan 14, 2:17 PM · Restricted Project

Sat, Jan 11

beanz added a comment to D72278: [test] On Mac, don't try to use result of sysctl command if calling it failed..

I found this running lit tests on a Mac that doesn't have sysctl available. Not very familiar with it so I'm not sure why that is the case for our particular machine.

So this change is me assuming the intention was to just print and carry on if "sysctl" fails.

Sat, Jan 11, 4:42 PM · Restricted Project

Dec 17 2019

beanz accepted D70764: build: reduce CMake handling for zlib.

I think this is good, and it has been sitting a while.

Dec 17 2019, 9:47 AM · Restricted Project, Restricted Project, Restricted Project, Restricted Project

Dec 5 2019

beanz accepted D70620: [CommandLine] Add callbacks to Options.

LGTM.

Dec 5 2019, 2:45 PM · Restricted Project

Dec 4 2019

beanz added a comment to D70764: build: reduce CMake handling for zlib.

I conferred off-list with @compnerd. He and I talked through my concerns, and I believe this patch is fine as-is, so LGTM.

Dec 4 2019, 2:48 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project
beanz added a comment to D70764: build: reduce CMake handling for zlib.

@labath sorry for not replying on-list. I've actually been discussing offline with @compnerd. If llvm-config is printing an absolute path, I'm concerned about how that will interact with binary package distributions for the llvm-dev packages. Not sure if @tstellar has any thoughts on that.

Dec 4 2019, 1:32 PM · Restricted Project, Restricted Project, Restricted Project, Restricted Project

Dec 3 2019

beanz added inline comments to D70620: [CommandLine] Add callbacks to Options.
Dec 3 2019, 11:10 AM · Restricted Project

Dec 2 2019

beanz added a comment to D70620: [CommandLine] Add callbacks to Options.

I like the idea behind this. I provided more detailed feedback to Don off-list, but I really don't like the const void* parameter to the callback functions. I think we can make that match the parser data type, and have compile-time errors for type mismatches. That would be vastly preferable to casting void pointers.

Dec 2 2019, 2:44 PM · Restricted Project

Nov 19 2019

beanz accepted D69824: Extends the tblgen macro to allow mlir-tblgen to be installed.

Given that, this change LGTM. Using an extra argument would be nice, but this is a macro, not a function, and IIRC CMake argument parsing in macros doesn't really work, so that would be a big change.

Nov 19 2019, 12:52 PM · Restricted Project
beanz added a comment to D69824: Extends the tblgen macro to allow mlir-tblgen to be installed.

When cross compiling any LLVM-based project we do not rely on installed copies of table gen, instead we configure and build host tablegen tools. That functionality in the LLVM build system is flexible enough to support any sub-project that provides its own tablegen tool.

Nov 19 2019, 9:29 AM · Restricted Project

Nov 14 2019

beanz accepted D70280: Remove Support/Options.h, it is unused.

Yea. Unfortunately the migration stalled out because there was no way to make the registration method work with the new pass manager.

Nov 14 2019, 9:35 PM · Restricted Project, Restricted Project

Nov 13 2019

beanz accepted D70179: [cmake] Explicitly mark libraries defined in lib/ as "Component Libraries".

This looks like a great cleanup to me.

Nov 13 2019, 11:23 AM · Restricted Project
beanz accepted D70161: [cmake] Emit an error for -DBUILD_SHARED_LIBS=ON on Windows.

LGTM.

Nov 13 2019, 11:23 AM · Restricted Project
beanz accepted D70160: [cmake] Prevent building with BUILD_SHARED_LIBS and LLVM_LINK_LLVM_DYLIB.

LGTM.

Nov 13 2019, 11:23 AM · Restricted Project
beanz accepted D70159: [cmake] Remove confusing condition argument from else() NFC.

LGTM

Nov 13 2019, 11:23 AM · Restricted Project
beanz added inline comments to D70026: [cmake] Always build the libLLVM shared library.
Nov 13 2019, 11:20 AM · Restricted Project

Nov 11 2019

beanz added a comment to D70026: [cmake] Always build the libLLVM shared library.

I very much dislike using EXCLUDE_FROM_ALL. I really believe all should be all, not most things I think are important. In general iteration cycles the check-* targets won't build or link libLLVM unless you specify LLVM_LINK_LLVM_DYLIB, and if you actually want to build everything as a means of testing your changes the all target should do that.

Nov 11 2019, 10:40 AM · Restricted Project

Nov 8 2019

beanz added a comment to D69973: [libcxxabi] Prevent cmake from removing our explicit system C++ include paths.

This should only be broken for CMake 3.14 and later. We should include that in the comments around this change.

Nov 8 2019, 4:05 PM · Restricted Project
beanz added a comment to D69973: [libcxxabi] Prevent cmake from removing our explicit system C++ include paths.

https://gitlab.kitware.com/cmake/cmake/issues/19227

Nov 8 2019, 10:53 AM · Restricted Project
beanz added a comment to D69973: [libcxxabi] Prevent cmake from removing our explicit system C++ include paths.

Reviewing the CMake change more closely, that is a bug. They shouldn't have changed the default behavior in that way. At the very least that change should have been a policy change.

Nov 8 2019, 10:35 AM · Restricted Project
beanz added a comment to D69973: [libcxxabi] Prevent cmake from removing our explicit system C++ include paths.

I suspect this behavior change was caused by: https://gitlab.kitware.com/cmake/cmake/commit/b0b820ea39fe4edaf0672ba899a7042ef93a20d2

Nov 8 2019, 10:27 AM · Restricted Project
beanz added a comment to D69931: Add rpath to liblldb so vendors can ship their own python framework (or others).

It is a bit gross that Python does an @rpath install name, that is generally against convention. I can see adding an option to add rpaths to liblldb making sense. I certainly agree LLDB shouldn't be in the business of packaging transitive dependencies.

Nov 8 2019, 10:17 AM · Restricted Project

Nov 6 2019

beanz accepted D69856: [Object][MachO] Rewrite macho-invalid-fat-arch-size into YAML.

LGTM

Nov 6 2019, 11:23 AM · Restricted Project

Nov 5 2019

beanz committed rG34688fafea81: Implement `sys::getHostCPUName()` for Darwin ARM (authored by beanz).
Implement `sys::getHostCPUName()` for Darwin ARM
Nov 5 2019, 5:53 PM
beanz closed D69597: Implement `sys::getHostCPUName()` for Darwin ARM.
Nov 5 2019, 5:53 PM · Restricted Project
beanz added a comment to D69817: Fix OrcError build with modules enabled..

I don't think that's the right fix as it moves a module map from representing a project structure towards having a random set of headers. But even after excluding 3 headers there are errors

Nov 5 2019, 5:43 PM · Restricted Project
beanz accepted D69003: [dsymutil] Explicitly link against libatomic when necessary.

Yea, that makes sense. Sorry again for the run around.

Nov 5 2019, 2:29 PM · Restricted Project
beanz added a comment to D69817: Fix OrcError build with modules enabled..

RPCUtils.h and RPCSerialization.h can both also be excluded.

Nov 5 2019, 1:24 PM · Restricted Project
beanz added a comment to D69817: Fix OrcError build with modules enabled..

Adding exclude header "ExecutionEngine/Orc/OrcError.h" to the module specification for LLVM_ExecutionEngine seems like a more appropriate fix.

Nov 5 2019, 11:06 AM · Restricted Project
beanz accepted D69003: [dsymutil] Explicitly link against libatomic when necessary.

Sorry, I had missed the atomic use directly in the tool. Sorry for my incorrect statements. This seems like a reasonable fix.

Nov 5 2019, 10:56 AM · Restricted Project
beanz added a comment to D69817: Fix OrcError build with modules enabled..

This confuses me. LLVMOrcError only directly depends on LLVMSupport. It doesn't include or need anything from ExecutionEngine.h.

Nov 5 2019, 10:56 AM · Restricted Project
beanz updated the diff for D69597: Implement `sys::getHostCPUName()` for Darwin ARM.

Rebasing and updating to separate arm32 and arm64.

Nov 5 2019, 9:14 AM · Restricted Project

Nov 1 2019

beanz accepted D69638: [NFC] Add SUPPORT_PLUGINS to add_llvm_executable().

LGTM

Nov 1 2019, 3:02 PM · Restricted Project, Restricted Project

Oct 31 2019

beanz added a comment to D69638: [NFC] Add SUPPORT_PLUGINS to add_llvm_executable().

A few comments inline.

Oct 31 2019, 11:07 AM · Restricted Project, Restricted Project

Oct 30 2019

beanz added a comment to D69356: [NFC] Rename LLVM_NO_DEAD_STRIP.

The intention of LLVM_SUPPORT_PLUGINS was as an internal option set by tools which may have plugins and need to be built in a specific way (such as avoiding deadstriping) if plugins are enabled.

Oct 30 2019, 11:12 AM · Restricted Project, Restricted Project
beanz added a comment to D69356: [NFC] Rename LLVM_NO_DEAD_STRIP.

For your second question: As the patch author indicated, the change to adjust the behaviour on the affected platform is forthcoming. Your question seems predicated upon that not happening.

Oct 30 2019, 10:22 AM · Restricted Project, Restricted Project

Oct 29 2019

beanz updated subscribers of D69597: Implement `sys::getHostCPUName()` for Darwin ARM.
Oct 29 2019, 6:48 PM · Restricted Project
beanz updated the summary of D69597: Implement `sys::getHostCPUName()` for Darwin ARM.
Oct 29 2019, 6:16 PM · Restricted Project
beanz created D69597: Implement `sys::getHostCPUName()` for Darwin ARM.
Oct 29 2019, 6:14 PM · Restricted Project
beanz committed rGa34680a33eb1: Break out OrcError and RPC (authored by beanz).
Break out OrcError and RPC
Oct 29 2019, 5:46 PM
beanz closed D68732: Break out OrcError and RPC.
Oct 29 2019, 5:46 PM · Restricted Project
beanz added a comment to D69356: [NFC] Rename LLVM_NO_DEAD_STRIP.

Is there some documentation indicating these other use cases?

Oct 29 2019, 5:27 PM · Restricted Project, Restricted Project
beanz committed rG187d43b8fc89: NFC. clang-format (authored by beanz).
NFC. clang-format
Oct 29 2019, 4:17 PM
beanz committed rG51c9c1d23a79: Fixing clang build as a result of r332021 (authored by beanz).
Fixing clang build as a result of r332021
Oct 29 2019, 4:17 PM
beanz added a comment to D69356: [NFC] Rename LLVM_NO_DEAD_STRIP.

LLVM_NO_DEAD_STRIP isn't actually useful only for when plugins are enabled. It is also useful in a lot of contexts around building JIT host processes. The advantage of the old name was it specified exactly what it did (i.e. not dead strip code).

Oct 29 2019, 2:31 PM · Restricted Project, Restricted Project
beanz committed rGd9d83b6c126d: Merge remote-tracking branch 'origin/master' into upstream-with-swift (authored by beanz).
Merge remote-tracking branch 'origin/master' into upstream-with-swift
Oct 29 2019, 12:59 PM
beanz committed rGf72d762d1e8c: Merge remote-tracking branch 'origin/master' into upstream-with-swift (authored by beanz).
Merge remote-tracking branch 'origin/master' into upstream-with-swift
Oct 29 2019, 12:36 PM
beanz accepted D69444: [Support] Check for atomics64 when deciding if '-latomic' is needed.
Oct 29 2019, 12:31 PM · Restricted Project

Oct 25 2019

beanz accepted D69412: build: avoid hardcoding the libxml2 library name.

LGTM

Oct 25 2019, 3:06 PM · Restricted Project
beanz requested changes 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.

Oct 25 2019, 2:56 PM · Restricted Project
beanz added a comment to D69407: build: remove `LLVM_CXX_STD` extension point.

Everything about this change seems good to me, the only thing I'd suggest is that we add a documentation note about CMAKE_CXX_STANDARD to docs/CMake.rst.

Oct 25 2019, 10:34 AM · Restricted Project
beanz accepted D69407: build: remove `LLVM_CXX_STD` extension point.
Oct 25 2019, 10:34 AM · Restricted Project

Oct 22 2019

beanz accepted D69325: Fix broken sphinx link in CMake.rst..

LGTM

Oct 22 2019, 2:50 PM · Restricted Project

Oct 16 2019

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

I'm not sure this is the correct fix.

Oct 16 2019, 12:31 PM · Restricted Project

Oct 11 2019

beanz added a comment to D68833: [CMake] Re-order runtimes in the order of dependencies.

Yes, precisely. That's also the currently preferred way of building libc++: https://libcxx.llvm.org/docs/BuildingLibcxx.html

Oct 11 2019, 2:04 PM · Restricted Project
beanz added a comment to D68833: [CMake] Re-order runtimes in the order of dependencies.

CMake tracks dependencies between targets, but not between directories. If the CMakeLists.txt in some directory (e.g. <root>/libcxx/) needs a target defined in another directory (e.g. <root>/libcxxabi/), one has to make sure that libcxxabi's CMakeLists.txt is included before libcxx's CMakeLists.txt. This isn't new or vexing, IMO. If that is what this patch ensures (I don't know the runtimes build very well), I think this is good.

Oct 11 2019, 11:19 AM · Restricted Project

Oct 10 2019

beanz added a comment to D68833: [CMake] Re-order runtimes in the order of dependencies.

Two thoughts.

Oct 10 2019, 8:12 PM · Restricted Project
beanz requested changes to D68833: [CMake] Re-order runtimes in the order of dependencies.
Oct 10 2019, 8:12 PM · Restricted Project

Oct 9 2019

beanz created D68732: Break out OrcError and RPC.
Oct 9 2019, 2:31 PM · Restricted Project
beanz added a comment to D63614: [System Model] [TTI] Update cache and prefetch TTI interfaces.

This patch causes lots of warning spews because it adds virtual methods to a class that doesn't have a virtual destructor.

Oct 9 2019, 1:22 PM · Restricted Project

Oct 6 2019

beanz created D68556: Document `LLVM_USE_SPLIT_DWARF` option.
Oct 6 2019, 2:20 PM · Restricted Project
beanz abandoned D68555: Make appendCallNB lambda mutable.

Accidentally uploaded this

Oct 6 2019, 12:24 PM · Restricted Project
beanz created D68555: Make appendCallNB lambda mutable.
Oct 6 2019, 12:24 PM · Restricted Project

Oct 4 2019

beanz added a comment to D68448: [clang-tools-extra] [cmake] Link against libclang-cpp whenever possible.

More dynamic, less static usage :)

Oct 4 2019, 1:20 PM · Restricted Project
beanz accepted D68448: [clang-tools-extra] [cmake] Link against libclang-cpp whenever possible.

LGTM.

Oct 4 2019, 1:17 PM · Restricted Project
beanz accepted D68429: [clang] [cmake] Use add_clang_tool() to install all tools.

fair enough

Oct 4 2019, 1:13 PM · Restricted Project
beanz accepted D68430: Don't use object libraries with Xcode.

That's fine. That said, there are things that just can't be done, or don't work well, in the Xcode and Visual Studio generators, so we have precedent for disabling functionality based on those generators.

Oct 4 2019, 11:13 AM · Restricted Project

Oct 3 2019

beanz added a comment to D68430: Don't use object libraries with Xcode.

I'm not quite sure what it's doing. The executable targets end up trying to link against the static libraries anyway, which of course haven't been built. It's possible that this is because the LIBTYPE is both STATIC and OBJECT and if it were just OBJECT we might be better off, but I'm not sure if Xcode's IDE features will be happy with a target that doesn't actually produce a library. I can try it if you want, though.

Oct 3 2019, 6:25 PM · Restricted Project
beanz added a comment to D68429: [clang] [cmake] Use add_clang_tool() to install all tools.

Are these tools intended to be installed? I thought they were developer focused tools. I don't think we should make install targets for things that the project doesn't want to support publicly.

Oct 3 2019, 4:54 PM · Restricted Project
beanz accepted D68413: [clang] [cmake] Add distribution install targets for remaining components.

LGTM

Oct 3 2019, 4:50 PM · Restricted Project
beanz added a comment to D68430: Don't use object libraries with Xcode.

This has the side-effect of making libclang_cpp effectively empty when you build with Xcode.

Oct 3 2019, 4:50 PM · Restricted Project

Oct 1 2019

beanz requested changes to D68259: [llvm] [cmake] Store a list of all possible LLVM_DISTRIBUTION_COMPONENTS.

This isn't really possible to do because CMake doesn't have a mechanism for listing all targets. A more correct approach for this would require generating a ninja build, then you could use ninja -t list to dump all the targets, then you can post-process that list to find a full list of components.

Oct 1 2019, 11:17 AM · Restricted Project

Sep 19 2019

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

Sep 14 2019

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.

Sep 14 2019, 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.

Sep 14 2019, 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

Sep 14 2019, 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.

Sep 14 2019, 9:07 AM

Sep 11 2019

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

Sep 10 2019

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

Updates based on feedback from Lang.

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

Sep 9 2019

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

@aaronpuchert sounds like a reasonable approach

Sep 9 2019, 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.

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

Sep 5 2019

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.

Sep 5 2019, 6:41 PM · Restricted Project

Sep 4 2019

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.

Sep 4 2019, 5:03 PM · Restricted Project

Aug 28 2019

beanz accepted D66901: [ObjectYAML] Fix lifetime issue in dumpDebugLines.
Aug 28 2019, 6:10 PM · Restricted Project
beanz added inline comments to D66901: [ObjectYAML] Fix lifetime issue in dumpDebugLines.
Aug 28 2019, 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