[PATCH] D159247: [HLSL] Cleanup support for `this` as an l-value

beanz added a comment. Full disclosure here, I need to write some more tests for this. It should be NFC for all languages other than HLSL, but I want to spend some time making sure that the tests adequately exercise the HLSL paths (which should also be mostly NFC). Repository: rG LLVM Githu

[PATCH] D159247: [HLSL] Cleanup support for `this` as an l-value

beanz added a comment. Yea, we could do that approach. It will mean filtering about a few `getNonReferenceType()` calls, which is what I was trying to avoid. That said, it might be the better solution since it can be unconditional and will work for both deducing `this` and HLSL. I'll test it o

[PATCH] D159247: [HLSL] Cleanup support for `this` as an l-value

beanz updated this revision to Diff 555118. beanz added a comment. Updating based on review feedback from @core3ntin. Thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159247/new/ https://reviews.llvm.org/D159247 Files: clang/include/clan

[PATCH] D159247: [HLSL] Cleanup support for `this` as an l-value

beanz updated this revision to Diff 555380. beanz added a comment. Updating based on PR feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159247/new/ https://reviews.llvm.org/D159247 Files: clang/include/clang/AST/ExprCXX.h clang/lib/AST

[PATCH] D159247: [HLSL] Cleanup support for `this` as an l-value

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG400d3261a0da: [HLSL] Cleanup support for `this` as an l-value (authored by beanz). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D69356: [NFC] Rename LLVM_NO_DEAD_STRIP

beanz added a comment. `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). It seems to me that may

[PATCH] D69356: [NFC] Rename LLVM_NO_DEAD_STRIP

beanz added a comment. In D69356#1726336 , @hubert.reinterpretcast wrote: > Is there some documentation indicating these other use cases? If you have JIT'd code that needs to link against functions exposed in the process running the JIT you can't dead

[PATCH] D69356: [NFC] Rename LLVM_NO_DEAD_STRIP

beanz added a comment. In D69356#1726527 , @hubert.reinterpretcast wrote: > 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

[PATCH] D69356: [NFC] Rename LLVM_NO_DEAD_STRIP

beanz added a comment. In D69356#1727451 , @daltenty wrote: > 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 en

[PATCH] D69638: [NFC] Add SUPPORT_PLUGINS to add_llvm_executable()

beanz added a comment. A few comments inline. Comment at: clang/tools/driver/CMakeLists.txt:34 ${tablegen_deps} + SUPPORT_PLUGINS ) This is now a behavior change because you're always passing this. You'll want to bring back the conditional statement wit

[PATCH] D69638: [NFC] Add SUPPORT_PLUGINS to add_llvm_executable()

beanz accepted this revision. beanz added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69638/new/ https://reviews.llvm.org/D69638 ___ c

[PATCH] D70280: Remove Support/Options.h, it is unused

beanz accepted this revision. beanz added a comment. This revision is now accepted and ready to land. Yea. Unfortunately the migration stalled out because there was no way to make the registration method work with the new pass manager. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST A

[PATCH] D70764: build: reduce CMake handling for zlib

beanz added a comment. @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 a

[PATCH] D70764: build: reduce CMake handling for zlib

beanz added a comment. I conferred off-list with @compnerd. He and I talked through my concerns, and I believe this patch is fine as-is, so LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70764/new/ https://reviews.llvm.org/D70764 _

[PATCH] D70764: build: reduce CMake handling for zlib

beanz accepted this revision. beanz added a comment. This revision is now accepted and ready to land. I think this is good, and it has been sitting a while. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70764/new/ https://reviews.llvm.org/D70764

[PATCH] D68520: [cmake] Fix clang builds with BUILD_SHARED=ON and CLANG_LINK_CLANG_DYLIB=ON

beanz accepted this revision. beanz added a comment. Seems reasonable. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68520/new/ https://reviews.llvm.org/D68520 ___ cfe-commits mailing list cfe-commits@

[PATCH] D40194: [libcxxabi][CMake] Provide option to disable installing of the library

beanz accepted this revision. beanz added a comment. This revision is now accepted and ready to land. LGTM. Repository: rL LLVM https://reviews.llvm.org/D40194 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/

[PATCH] D40195: [libunwind][CMake] Provide option to disable instalation of the library

beanz accepted this revision. beanz added a comment. This revision is now accepted and ready to land. LGTM. Repository: rL LLVM https://reviews.llvm.org/D40195 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/

[PATCH] D145270: Add codegen for llvm exp/exp2 elementwise builtins

beanz added inline comments. Comment at: clang/docs/LanguageExtensions.rst:642 T __builtin_elementwise_log10(T x) return the base 10 logarithm of x floating point types + T __builtin_elementwise_exp(T x)returns the base-e exp

[PATCH] D149119: [CMake] Use LLVM own tools in extract_symbols.py

beanz added a comment. One potential area of concern here: If `llvm-driver` is ever extended to work as a plugin loader (thus exporting its symbols), removing support for the pre-installed host tools could cause a cyclic dependency. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149119/

[PATCH] D143553: [Clang][CMake] Use perf-training for Clang-BOLT

beanz accepted this revision. beanz added a comment. This revision is now accepted and ready to land. LGTM. Sorry for the delays reviewing! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143553/new/ https://reviews.llvm.org/D143553 ___

[PATCH] D151683: [clang] Enable C++11-style attributes in all language modes

beanz added inline comments. Comment at: clang/test/ParserHLSL/group_shared.hlsl:14 -// expected-error@+1 {{expected expression}} float groupshared [[]] i = 12; aaron.ballman wrote: > philnik wrote: > > Should this also get an extension warning/should attribut

[PATCH] D151683: [clang] Enable C++11-style attributes in all language modes

beanz added inline comments. Comment at: clang/test/ParserHLSL/group_shared.hlsl:14 -// expected-error@+1 {{expected expression}} float groupshared [[]] i = 12; philnik wrote: > beanz wrote: > > aaron.ballman wrote: > > > philnik wrote: > > > > Should this als

[PATCH] D139137: add floor library function

beanz accepted this revision. beanz added a comment. This revision is now accepted and ready to land. LGTM. @bob80905 if you haven't already, you should follow the steps here (https://www.llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access) to get commit access. Repository: rG LLVM Gi

[PATCH] D27812: [test] Extend llvm_shlib_dir fix to unittests

beanz accepted this revision. beanz added a comment. This revision is now accepted and ready to land. Looks reasonable to me. https://reviews.llvm.org/D27812 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailm

[PATCH] D28269: [clang] Update lit config in utils/perf-training

beanz added a comment. I don't think this works either. Pretty sure not setting the driver mode defaults it to gcc, when we should be setting it to g++. Can you try setting driver mode to g++ instead of removing the flag? Repository: rL LLVM https://reviews.llvm.org/D28269 __

[PATCH] D28269: [clang] Update lit config in utils/perf-training

beanz added a comment. LGTM! Repository: rL LLVM https://reviews.llvm.org/D28269 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29851: [clang-tools-extra] [test] Fix test dependencies when using installed tools

beanz accepted this revision. beanz added a comment. This revision is now accepted and ready to land. Ugh... I really don't like how `LLVM_UTILS_PROVIDED` is implemented, but I guess it is fine. This patch is good. Repository: rL LLVM https://reviews.llvm.org/D29851 __

[PATCH] D31363: [libc++] Remove cmake glob for source files

beanz added a comment. LLVM has a CMake module "LLVMProcessSources.cmake" which verifies that all source files are referenced in the CMakeLists file. Since it is part of the LLVM distributed modules, you can re-use it in libcxx. https://reviews.llvm.org/D31363 __

[PATCH] D31447: [Driver] Add option to print the resource directory

beanz accepted this revision. beanz added a comment. This revision is now accepted and ready to land. One minor nitpick on the test case, but otherwise this looks fine to me. Comment at: test/Driver/immediate-options.c:19 +// RUN: %clang -print-resource-dir | FileCheck %s -che

[PATCH] D31696: Automatically add include-what-you-use for when building in tree

beanz added inline comments. Comment at: clang/tools/.gitignore:12 +#==# +# The include-what-you-use project, for when building in-tree. +include-what-you-use rnk wrote: > LLVM doesn't cu

[PATCH] D26649: [CMake] Support lld with LTO bootstrap

beanz added inline comments. Comment at: cfe/trunk/CMakeLists.txt:531 + if(BOOTSTRAP_LLVM_ENABLE_LLD) +add_dependencies(clang-bootstrap-deps lld) + elseif(LLVM_BINUTILS_INCDIR) mehdi_amini wrote: > I come back to this a bit late, sorry, but I'm

[PATCH] D28821: [CMake] Separate LLVM_ENABLE_LTO and LLVM_ENABLE_LLD

beanz accepted this revision. beanz added a comment. This revision is now accepted and ready to land. Many thanks for the quick cleanup! LGTM! Repository: rL LLVM https://reviews.llvm.org/D28821 ___ cfe-commits mailing list cfe-commits@lists.llvm

[PATCH] D42490: [cmake] Set cmake policy CMP0068 to suppress warnings on OSX

beanz accepted this revision. beanz added a comment. This revision is now accepted and ready to land. Historically we've duplicated `cmake_policy` calls on a per-project basis which we needed to support standalone builds. That said, it would be nice if we had LLVM vend some CMake modules that en

[PATCH] D26654: [CMake] Add Fuchsia toolchain CMake cache files

beanz added a comment. Thanks for all your work on these! Repository: rL LLVM https://reviews.llvm.org/D26654 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26764: [compiler-rt] [cmake] Remove i686 target that is duplicate to i386

beanz added a reviewer: eugenis. beanz added a subscriber: eugenis. beanz added a comment. Looping in @eugenis, who added i686 support in r218761. eugenis, since i686 is identical to i386 generating it as a separate target is undesirable. Can you help advise as to how we can better meet your nee

[PATCH] D27683: Prepare PrettyStackTrace for LLDB adoption

beanz accepted this revision. beanz added a comment. LGTM other than a few style comments. Please clang-format before committing. Some of the formatting looks off (although that might just be my phone). Comment at: lib/Support/PrettyStackTrace.cpp:152 + va_start(ap, format);

[PATCH] D32603: Build the Apple-style stage2 with modules and full debug info

beanz accepted this revision. beanz added a comment. This revision is now accepted and ready to land. SWEET! this is great to see! One small comment inline, otherwise LGTM. Comment at: cmake/caches/Apple-stage2.cmake:33 +set(CMAKE_C_FLAGS_RELWITHDEBINFO "-O2 -g -DNDEBUG" CACHE

[PATCH] D32815: [clang-tidy][CMake] Make clang-tidy usable as distribution component

beanz accepted this revision. beanz added a comment. Yep, this seems straight forward. Repository: rL LLVM https://reviews.llvm.org/D32815 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-

[PATCH] D117419: [clang][cmake] Use `GNUInstallDirs` to support custom installation dirs

beanz added inline comments. Comment at: clang/CMakeLists.txt:3 +include(GNUInstallDirs) + If GNUInstallDirs is included before the `project` call enables languages it results in a CMake warning being logged. Repository: rG LLVM Github Monorepo CHANGES SI

[PATCH] D117626: [ADT] [NFC] Add StringRef::detectEOL

beanz created this revision. beanz added reviewers: rnk, jyknight, dexonsmith. beanz requested review of this revision. Herald added projects: clang, LLVM. This change moves EOL detection out of the clang::InclusionRewriter into llvm::StringRef so that it can be easily reused elsewhere. It also ad

[PATCH] D117626: [ADT] [NFC] Add StringRef::detectEOL

beanz updated this revision to Diff 401071. beanz added a comment. Minor fix to properly handle classic mac... because it is _very_ important. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117626/new/ https://reviews.llvm.org/D117626 Files: clan

[PATCH] D117626: [ADT] [NFC] Add StringRef::detectEOL

beanz updated this revision to Diff 401074. beanz added a comment. Fixing the CR test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117626/new/ https://reviews.llvm.org/D117626 Files: clang/lib/Frontend/Rewrite/InclusionRewriter.cpp llvm/

[PATCH] D115848: tidy-llvm

beanz added a comment. Sorry I’m a bit late here. If you want to use clang-tidy on a CMake built project (like LLVM) you can use CMake’s built-in support for clang-tidy: https://cmake.org/cmake/help/latest/variable/CMAKE_LANG_CLANG_TIDY.html This will run clang-tidy during the project build wit

[PATCH] D117626: [ADT] [NFC] Add StringRef::detectEOL

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG9d3437fbf341: [ADT] [NFC] Add StringRef::detectEOL (authored by beanz). Changed prior to commit: https://reviews.llvm.org/D117626?vs=401074&id=401

[PATCH] D116966: [analyzer] Don't specify PLUGIN_TOOL for analyzer plugins

beanz added a comment. Have you looked at the impact on binary size? PLUGIN_TOOL _should_ cause the plugins to link against the copy of LLVM & Clang in the tool target binary which reduces the binary size of the plugin and avoids duplicate global initializations. Repository: rG LLVM Github

[PATCH] D109977: LLVM Driver Multicall tool

beanz added a comment. Sorry for disappearing on this. I'm working right now on adding support for multi-entry tools (like objcopy). I'll get an update in today. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109977/new/ https://reviews.llvm.org/D1

[PATCH] D109977: LLVM Driver Multicall tool

beanz updated this revision to Diff 404114. beanz added a comment. Adding support for multiple subcommands mapping to the same entry. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109977/new/ https://reviews.llvm.org/D109977 Files: clang/cmake/m

[PATCH] D116492: [lld] Deprecate using llvm-config to detect llvm installation

beanz added inline comments. Comment at: clang/CMakeLists.txt:26 "--src-root" - "--cmakedir") + "--cmakedir" + "--bindir" I assume these are re-arranged to match what you're doing in lld. Is that correct? Generally it is preferred to do th

[PATCH] D116492: [lld] Deprecate using llvm-config to detect llvm installation

beanz added inline comments. Comment at: clang/CMakeLists.txt:26 "--src-root" - "--cmakedir") + "--cmakedir" + "--bindir" Ericson2314 wrote: > beanz wrote: > > I assume these are re-arranged to match what you're doing in lld. Is that > > co

[PATCH] D116548: [clang][cmake] Rearrange top-level CMakeLists.txt for D116492

beanz accepted this revision. beanz added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116548/new/ https://reviews.llvm.org/D116548 ___ c

[PATCH] D116521: [CMake] Factor out config prefix finding logic

beanz accepted this revision. beanz added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116521/new/ https://reviews.llvm.org/D116521 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D116492: [lld] Deprecate using llvm-config to detect llvm installation

beanz accepted this revision. beanz added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116492/new/ https://reviews.llvm.org/D116492 ___ c

[PATCH] D89942: Disable LTO and LLD for bootstrap builds on systems unsupported by LLD

beanz added a comment. I'm super sorry to be popping in so late on this. Is this really the direction we should take? If we know SystemZ + LLD + LTO is a bad configuration, "fixing" the configuration and logging a message seems like the wrong move when the only way to get into that state is to

[PATCH] D89942: Disable LTO and LLD for bootstrap builds on systems unsupported by LLD

beanz added a comment. In D89942#2629184 , @tbaeder wrote: > But that's not true. The `3-stage-base.cmake` sets > `BOOTSTRAP_LLVM_ENABLE_LTO` to `ON`, which doesn't work on SystemZ. This is > the initial problem I had. I was not interested in having LTO

[PATCH] D89942: Disable LTO and LLD for bootstrap builds on systems unsupported by LLD

beanz added a comment. If you add `include(CMakeDetermineSystem)` to the top of the cache file, it should initialize `CMAKE_HOST_SYSTEM_PROCESSOR`, which also probably gives you a reasonable value to key off. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89942/new/ https://reviews.llv

[PATCH] D89942: Disable LTO and LLD for bootstrap builds on systems unsupported by LLD

beanz accepted this revision. beanz added a comment. LGTM. The `*_LTO` and `*_LLD` variables are usable independently of each other even on non-Apple platforms, so it makes sense to need to set them both. LLVM’s LTO is supported in Gold, and I believe our build system supports MSVC’s LTCG (whi

[PATCH] D99322: [clang] Pass option directly to command. NFC

beanz accepted this revision. beanz added a comment. This revision is now accepted and ready to land. Woo! Thanks for cleaning up one of my old messes :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99322/new/ https://reviews.llvm.org/D99322

[PATCH] D111109: AddGlobalAnnotations for function with or without function body.

This revision was automatically updated to reflect the committed changes. Closed by commit rG121b2252de0e: AddGlobalAnnotations for function with or without function body. (authored by beanz). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D09/new/

[PATCH] D107095: Implement #pragma clang header_unsafe

beanz updated this revision to Diff 367851. beanz added a comment. Renaming to restrict_expansion Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107095/new/ https://reviews.llvm.org/D107095 Files: clang/docs/LanguageExtensions.rst clang/include

[PATCH] D107095: Implement #pragma clang header_unsafe

beanz updated this revision to Diff 367856. beanz added a comment. Fixing documentation line wrapping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107095/new/ https://reviews.llvm.org/D107095 Files: clang/docs/LanguageExtensions.rst clang/in

[PATCH] D107095: Implement #pragma clang header_unsafe

beanz updated this revision to Diff 367861. beanz added a comment. Cleaning up documentation wording to make more sense with the rename Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107095/new/ https://reviews.llvm.org/D107095 Files: clang/docs/

[PATCH] D107095: Implement #pragma clang header_unsafe

beanz updated this revision to Diff 367863. beanz added a comment. Missed updating the flag name. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107095/new/ https://reviews.llvm.org/D107095 Files: clang/docs/LanguageExtensions.rst clang/include

[PATCH] D107095: Implement #pragma clang restrict_expansion

This revision was automatically updated to reflect the committed changes. Closed by commit rG43de869d77f7: Implement #pragma clang restrict_expansion (authored by beanz). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107095/new/ https://reviews.llvm

[PATCH] D108567: Implement #pragma clang final extension

beanz created this revision. beanz added reviewers: aaron.ballman, rsmith, lgerbarg, pete, lebedev.ri, dexonsmith. beanz requested review of this revision. Herald added a project: clang. This patch adds a new preprocessor extension ``#pragma clang final`` which enables warning on undefinition and

<    5   6   7   8   9   10