[PATCH] D61549: Fix use of 'is' operator for comparison

tambre created this revision. tambre added a reviewer: rsmith. Herald added a project: clang. Herald added a subscriber: cfe-commits. The 'is' operator is not meant to be used for comparisons. It currently working is an implementation detail of CPython. CPython 3.8 has added a SyntaxWarning for

[PATCH] D61549: Fix use of 'is' operator for comparison

tambre added a comment. In D61549#1785369 , @Jim wrote: > @j-carl Could you give a Python 3.8 syntax reference for this change. This is not a language change in Python 3.8. AFAICT, the behaviour of `is` has been the same as long as Python has been in

[PATCH] D75811: [CUDA] Choose default architecture based on CUDA installation

tambre added a comment. > Magically changing compiler target based on something external to compiler is > a bad idea IMO. I would expect a compilation with exactly the same compiler > options to do exactly the same thing. If we magically change default target, > that will not be the case.

[PATCH] D75811: [CUDA] Choose default architecture based on CUDA installation

tambre added a comment. Thank you for the long and detailed explanation. It's been of great help! I've gone with the approach of trying the architectures in the most recent non-deprecated order – sm_52, sm_30. A problem with bumping the default architecture would have been that there are

[PATCH] D75806: [CUDA] Add CUDA 10.2 detection

tambre created this revision. tambre added reviewers: tra, jlebar. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D75806 Files: clang/include/clang/Basic/Cuda.h clang/lib/Basic/Cuda.cpp

[PATCH] D75811: [CUDA] Choose default architecture based on CUDA installation

tambre added inline comments. Comment at: clang/lib/Driver/Driver.cpp:642 + *this, CudaTriple, *HostTC, C.getInputArgs(), OFK); + C.getArgs().AddJoinedArg( + nullptr, getOpts().getOption(options::OPT_cuda_version_EQ), This isn't very

[PATCH] D75811: [CUDA] Choose default architecture based on CUDA installation

tambre updated this revision to Diff 248949. tambre added a comment. Add missing error message Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75811/new/ https://reviews.llvm.org/D75811 Files: clang/include/clang/Basic/Cuda.h

[PATCH] D75811: [CUDA] Choose default architecture based on CUDA installation

tambre added a comment. Your help here and over on CMake's side has been very helpful. Thank you! I'll @ you on CMake's side if I need any help while working on CUDA support. Hopefully you won't mind. :) I'm progressing on this and hope to have initial support in a mergeable state within two

[PATCH] D75811: [CUDA] Choose default architecture based on CUDA installation

tambre added a comment. In D75811#1923278 , @csigg wrote: > > I'll be adding a `CUDA_ROOT` option to CMake that will be passed to clang > > as `--cuda-path`. > > I'm not familiar with CMake and whether that option is picked up from an > environment

[PATCH] D77491: [Sema] Fix incompatible builtin redeclarations in non-global scope

tambre added a comment. rsmith: ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77491/new/ https://reviews.llvm.org/D77491 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D77940: [AArch64] Add NVIDIA Carmel support

tambre updated this revision to Diff 256764. tambre added a comment. Fix formatting Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77940/new/ https://reviews.llvm.org/D77940 Files: clang/test/Driver/aarch64-cpus.c

[PATCH] D77940: [AArch64] Add NVIDIA Carmel support

tambre updated this revision to Diff 256763. tambre added a comment. Remove cacheline size Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77940/new/ https://reviews.llvm.org/D77940 Files: clang/test/Driver/aarch64-cpus.c

[PATCH] D77940: [AArch64] Add NVIDIA Carmel support

tambre created this revision. tambre added reviewers: sdesmalen, paquette. Herald added subscribers: cfe-commits, danielkiss, jfb, hiraditya, kristof.beyls. Herald added a project: clang. tambre updated this revision to Diff 256763. tambre added a comment. tambre edited the summary of this

[PATCH] D77491: [Sema] Fix incompatible builtin redeclarations in non-global scope

tambre added a comment. rsmith: ping 2 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77491/new/ https://reviews.llvm.org/D77491 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D77940: [AArch64] Add NVIDIA Carmel support

tambre added inline comments. Comment at: llvm/lib/Target/AArch64/AArch64.td:607 + FeatureSVE + ]>; + sdesmalen wrote: > Not sure how accurate the link you referenced is, but it doesn't mention

[PATCH] D77940: [AArch64] Add NVIDIA Carmel support

tambre updated this revision to Diff 258045. tambre marked 6 inline comments as done. tambre added a comment. Remove SVE, fix formatting Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77940/new/ https://reviews.llvm.org/D77940 Files:

[PATCH] D78392: [CUDA] Define __CUDACC__ before standard library headers

tambre created this revision. tambre added a reviewer: tra. Herald added subscribers: cfe-commits, yaxunl. Herald added a project: clang. tambre edited the summary of this revision. tambre edited the summary of this revision. tra added inline comments. Comment at:

[PATCH] D77940: [AArch64] Add NVIDIA Carmel support

tambre updated this revision to Diff 258513. tambre added a comment. Add cacheline size per the technical reference manual Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77940/new/ https://reviews.llvm.org/D77940 Files:

[PATCH] D77502: [clang][CodeGen] Handle throw expression in conditional operator constant folding

tambre updated this revision to Diff 255607. tambre marked an inline comment as done. tambre added a comment. Update comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77502/new/ https://reviews.llvm.org/D77502 Files:

[PATCH] D77502: [clang][CodeGen] Handle throw expression in conditional operator constant folding

tambre marked an inline comment as done. tambre added a comment. Updated the comment. Please commit this for me, as I lack commit privileges. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77502/new/ https://reviews.llvm.org/D77502

[PATCH] D77502: [clang][CodeGen] Handle throw expression in conditional operator constant folding

tambre updated this revision to Diff 255175. tambre added a comment. Fix formatting Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77502/new/ https://reviews.llvm.org/D77502 Files: clang/lib/CodeGen/CGExpr.cpp

[PATCH] D77502: [clang][CodeGen] Handle throw expression in conditional operator constant folding

tambre created this revision. tambre added a reviewer: rsmith. Herald added a project: clang. Herald added a subscriber: cfe-commits. tambre edited the summary of this revision. We're smart and do constant folding when emitting conditional operators. Thus we emit the live value as a lvalue. This

[PATCH] D77491: [Sema] Fix incompatible builtin redeclarations in non-global scope

tambre created this revision. tambre added a reviewer: rsmith. Herald added a project: clang. Herald added a subscriber: cfe-commits. We didn't mark them previously as non-builtin in non-global scope if we determined they were incompatible redeclarations. This caused problems down the line with

[PATCH] D77502: [clang][CodeGen] Handle throw expression in conditional operator constant folding

tambre added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:4337 +EmitCXXThrowExpr(ThrowExpr); +return EmitLValue(dead); + } rsmith wrote: > The IR we emit for the dead operand is unreachable; it's a bit wasteful to > generate it

[PATCH] D77502: [clang][CodeGen] Handle throw expression in conditional operator constant folding

tambre updated this revision to Diff 255275. tambre marked 4 inline comments as done. tambre added a comment. Emit undef lvalue, add throw CHECK to test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77502/new/ https://reviews.llvm.org/D77502

[PATCH] D77491: [Sema] Fix incompatible builtin redeclarations in non-global scope

tambre added a reviewer: MaskRay. tambre added a comment. Could someone please review this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77491/new/ https://reviews.llvm.org/D77491 ___ cfe-commits

[PATCH] D77940: [AArch64] Add NVIDIA Carmel support

tambre added a comment. sdesmalen: Could you please take another look? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77940/new/ https://reviews.llvm.org/D77940 ___ cfe-commits mailing list

[PATCH] D78726: [CUDA] Define __CUDACC_DEBUG__ when compiling device code in debug mode

tambre created this revision. tambre added a reviewer: tra. Herald added subscribers: cfe-commits, yaxunl. Herald added a project: clang. NVCC defines __CUDACC_DEBUG__ when compiling device code in debug mode. Let's do the same to help with compatibility between the two and possibly enable users

[PATCH] D77940: [AArch64] Add NVIDIA Carmel support

tambre added a comment. In D77940#2012092 , @sdesmalen wrote: > LGTM! Please land this for me as I lack commit privileges. :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77940/new/

[PATCH] D77940: [AArch64] Add NVIDIA Carmel support

tambre added a comment. Herald added a project: LLVM. Herald added a subscriber: llvm-commits. sdesmalen: ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77940/new/ https://reviews.llvm.org/D77940

[PATCH] D77491: [Sema] Fix incompatible builtin redeclarations in non-global scope

tambre added a comment. ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77491/new/ https://reviews.llvm.org/D77491 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D78726: [CUDA] Define __CUDACC_DEBUG__ when compiling device code in debug mode

tambre abandoned this revision. tambre marked an inline comment as done. tambre added a comment. Abandoning as there are good reasons to not add this. Comment at: clang/lib/Driver/ToolChains/Cuda.cpp:632 + if (mustEmitDebugInfo(DriverArgs) == EmitSameDebugInfoAsHost) +

[PATCH] D77940: [AArch64] Add NVIDIA Carmel support

tambre updated this revision to Diff 259653. tambre marked 4 inline comments as done. tambre added a comment. Remove SVE, add another check to parser test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77940/new/ https://reviews.llvm.org/D77940

[PATCH] D77940: [AArch64] Add NVIDIA Carmel support

tambre added a comment. In D77940#1998856 , @sdesmalen wrote: > It seems a NVIDIA Developer Program Membership is needed in order to download > the TRM. I'm not sure if that is just a matter of creating an account, but > without it I can't really verify

[PATCH] D79800: [Sema] Remove default values for arguments prior to a parameter pack if the pack is used

tambre updated this revision to Diff 263759. tambre added a comment. Simplify code, improve comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79800/new/ https://reviews.llvm.org/D79800 Files: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

tambre updated this revision to Diff 264447. tambre marked 3 inline comments as done. tambre added a comment. Fix adding BuiltinAttr in C++ mode. Update one test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77491/new/

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

tambre updated this revision to Diff 264497. tambre marked an inline comment as not done. tambre added a comment. Semantic compatibility checking for C++ builtin redeclarations. Remove some now unnecessary logic from getBuiltinID(). Update more tests. 4 tests still failing. Repository: rG

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

tambre marked 4 inline comments as done. tambre added a comment. Thanks for the reviews and design pointers, John! There are still a few tests failing, but I'd rather not dive in before the general approach is considered acceptable. Comment at:

[PATCH] D77491: [Sema] Fix incompatible builtin redeclarations in non-global scope

tambre added a comment. ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77491/new/ https://reviews.llvm.org/D77491 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

tambre updated this revision to Diff 264441. tambre added a comment. Herald added a reviewer: aaron.ballman. Rework builtin declaration handling. Introduce BuiltinAttr. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77491/new/

[PATCH] D79800: [Sema] Remove default values for arguments prior to a parameter pack if the pack is used

tambre created this revision. tambre added reviewers: rsmith, rjmccall. Herald added a project: clang. Herald added a subscriber: cfe-commits. When a parameter pack is expanded the expanded parameters don't have default values. Thus if a parameter pack is expanded after any arguments with a

[PATCH] D75806: [CUDA] Add CUDA 10.2 detection

tambre added a comment. Thanks for the context! To avoid other people stepping into this trap, maybe it'd make sense to reword this warning to say "unsupported" instead of "unknown"? Or maybe add detection for newer versions, but introduce a separate warning for unsupported versions?

[PATCH] D75811: [CUDA] Choose default architecture based on CUDA installation

tambre created this revision. tambre added reviewers: tra, jlebar. Herald added a project: clang. Herald added a subscriber: cfe-commits. tambre added inline comments. Comment at: clang/lib/Driver/Driver.cpp:642 + *this, CudaTriple, *HostTC, C.getInputArgs(), OFK);

[PATCH] D75811: [CUDA] Choose default architecture based on CUDA installation

tambre added inline comments. Comment at: clang/lib/Driver/Driver.cpp:2579 + if (Version == CudaVersion::UNKNOWN) { +C.getDriver().Diag(clang::diag::err_drv_cuda_unknown_version) +<< VersionString; This error is hit when simply running

[PATCH] D79800: [Sema] Remove default values for arguments prior to a parameter pack if the pack is used

tambre added a comment. ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79800/new/ https://reviews.llvm.org/D79800 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D87189: [CMake] Remove dead FindPythonInterp code

tambre created this revision. tambre added reviewers: compnerd, phosek. Herald added subscribers: llvm-commits, libcxx-commits, Sanitizers, cfe-commits, mgorny. Herald added projects: clang, Sanitizers, libc++, LLVM. Herald added a reviewer: libc++. tambre requested review of this revision. LLVM

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

tambre added a comment. In D77491#2254065 , @rjmccall wrote: > The builtins with custom type-checking are all true intrinsics like > `__builtin_operator_new` and so on. They really can't be validly declared by > the user program. The thing that seems

[PATCH] D86877: [Clang][Driver] Support per-target runtime directories in the bare-metal toolchain

tambre added a comment. In D86877#2248064 , @phosek wrote: > This was discussed when `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR` was introduced > and there was a pushback against changing the driver behavior depending on > the value of that option, so if we're

[PATCH] D86877: [Clang][Driver] Support per-target runtime directories in the bare-metal toolchain

tambre updated this revision to Diff 289085. tambre added a comment. Remove define to change driver behaviour, use ToolChain::getCompilerRTArgString() instead. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86877/new/

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

tambre added a comment. FYI: PR45410, which this fixes, has been marked as a release blocker for 11.0.0. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77491/new/ https://reviews.llvm.org/D77491 ___

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

tambre added inline comments. Comment at: clang/include/clang/Basic/IdentifierTable.h:231 return ObjCOrBuiltinID == tok::NUM_OBJC_KEYWORDS; } rjmccall wrote: > Do we need to support reverting builtins anymore? We don't. It'd be possible to set the

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

tambre updated this revision to Diff 288911. tambre marked 2 inline comments as done. tambre added a comment. Remove builtin reverting. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77491/new/ https://reviews.llvm.org/D77491 Files:

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

tambre added a comment. ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77491/new/ https://reviews.llvm.org/D77491 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D86877: [Clang][Driver] Use full path to builtins in bare-metal toolchain

tambre abandoned this revision. tambre added a comment. In D86877#2262448 , @phosek wrote: > It's not clear why couldn't we support per-target runtime directory? It uses > the standard multiarch layout, so in you'd end up using a directory like >

[PATCH] D87189: [CMake] Remove dead FindPythonInterp code

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG86bd8f82cc74: [CMake] Remove dead FindPythonInterp code (authored by tambre). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

tambre updated this revision to Diff 290229. tambre marked 3 inline comments as done. tambre added a comment. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77491/new/ https://reviews.llvm.org/D77491 Files:

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

tambre updated this revision to Diff 290232. tambre marked an inline comment as done. tambre added a comment. Remove now obsolete FIXME. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77491/new/ https://reviews.llvm.org/D77491 Files:

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

tambre added a comment. Thanks for the review. All tests still pass, should be good for another round. Comment at: clang/lib/Sema/SemaDecl.cpp:9672-9673 + if (unsigned BuiltinID = II->getBuiltinID()) { +const auto *LinkageDecl = +

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

tambre updated this revision to Diff 291778. tambre marked 2 inline comments as done. tambre added a comment. Address comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77491/new/ https://reviews.llvm.org/D77491 Files:

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

tambre added a comment. If there are no further comments I'll commit this in a few days. Comment at: clang/lib/Sema/SemaDecl.cpp:2088 + + if (Error || R.isNull()) +return nullptr; aaron.ballman wrote: > Should we do this check *before* we create the C

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

tambre added a comment. Thanks for the review! I believe I've managed to address your comments. In D77491#2248454 , @rsmith wrote: > What happens for builtins with the "t" (custom typechecking) flag, for which > the signature is intended to have no

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

tambre updated this revision to Diff 289631. tambre marked 6 inline comments as done. tambre added a comment. Remove __inline__ and static from all builtins in intrin.h. Remove ASTReader/ASTWriter placeholders. During builtin recognition: - Use ASTContext::GetBuiltinType() instead of creating a

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

tambre updated this revision to Diff 289986. tambre added a comment. Improve comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77491/new/ https://reviews.llvm.org/D77491 Files: clang/include/clang/Basic/Attr.td

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

tambre added a comment. In D77491#2254967 , @rjmccall wrote: > I didn't see the specific example, sorry. I agree that my description is > more applicable to builtins in the `__builtin` namespace, which describes > most of the builtins with custom

[PATCH] D86877: [Clang][Driver] Use full path to builtins in bare-metal toolchain

tambre added a comment. @phosek Please review again. I've overhauled the patch as I realized that per-target runtime directories don't make sense for the bare-metal target, since the runtime is only distinguished by the //specific// architecture and nothing else. As a result I've simply changed

[PATCH] D86877: [Clang][Driver] Support per-target runtime directories in the bare-metal toolchain

tambre updated this revision to Diff 289161. tambre added a comment. Rework patch to simply cleanup runtime handling in the bare-metal toolchain. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86877/new/ https://reviews.llvm.org/D86877 Files:

[PATCH] D86877: [Clang][Driver] Support per-target runtime directories in the bare-metal toolchain

tambre updated this revision to Diff 288956. tambre added a comment. Add missing dash. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86877/new/ https://reviews.llvm.org/D86877 Files: clang/lib/Driver/CMakeLists.txt

[PATCH] D86877: [Clang][Driver] Support per-target runtime directories in the bare-metal toolchain

tambre created this revision. tambre added a reviewer: phosek. Herald added subscribers: cfe-commits, mgorny. Herald added a project: clang. tambre requested review of this revision. When LLVM_ENABLE_PER_TARGET_RUNTIME_DIR is set to ON the clang_rt.builtins will not have an architecture prefix

[PATCH] D86877: [Clang][Driver] Use full path to builtins in bare-metal toolchain

tambre added a comment. phosek: ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86877/new/ https://reviews.llvm.org/D86877 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D76323: [AST] Fix handling of long double and bool in __builtin_bit_cast

tambre added a comment. For reference, I've been using this + D75960 in my employer's Clang build for almost two months. We have been using `std::bit_cast` in our codebase quite a bit and haven't encountered any issues with these two changes. It would be nice

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

tambre updated this revision to Diff 288792. tambre added a comment. Herald added subscribers: danielkiss, jfb. Rebase, fix new MS builtins and other tests, XFAIL pthread_create() test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77491/new/

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

tambre added a comment. Please review. I would like to get this in a mergeable state. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77491/new/ https://reviews.llvm.org/D77491 ___ cfe-commits mailing

[PATCH] D86308: Reland [compiler-rt] Compile assembly files as ASM not C

tambre created this revision. tambre added reviewers: phosek, teemperor. Herald added subscribers: Sanitizers, cfe-commits, dexonsmith, kristof.beyls, mgorny, dberris. Herald added projects: clang, Sanitizers. tambre requested review of this revision. It isn't very wise to pass an assembly file

[PATCH] D86308: Reland [compiler-rt] Compile assembly files as ASM not C

tambre added a comment. I'm pretty sure `add_asm_sources()` has nothing to do. The ASM language is enabled by compiler-rt anyway and CMake can recognize the files as assembly anyway. @teemperor Could you give this a try? If this doesn't work, could you upload the Ninja file for me to

[PATCH] D85706: [compiler-rt] Compile assembly files as ASM not C

tambre added a comment. ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85706/new/ https://reviews.llvm.org/D85706 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D86308: Reland [compiler-rt] Compile assembly files as ASM not C

tambre updated this revision to Diff 286957. tambre added a comment. Herald added a project: libunwind. Herald added a subscriber: libcxx-commits. Herald added a reviewer: libunwind. Add workaround for Apple, and for MinGW prior to CMake 3.17. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D86308: [CMake][compiler-rt][libunwind] Compile assembly files as ASM not C, unify workarounds

tambre added a comment. In D86308#2229199 , @mstorsjo wrote: > Just FWIW, a similar change was made in libunwind earlier > (c48974ffd7d1676f79d39d3b1e70f07d3a5e2e44 > ), which > then

[PATCH] D86308: [CMake][compiler-rt][libunwind] Compile assembly files as ASM not C, unify workarounds

tambre added a comment. In D86308#2229901 , @teemperor wrote: > Sorry, just got around to check this out. With the new workaround this seems > to work on macOS (the initial patch did produce the same error). Many thanks! I've submitted an upstream CMake

[PATCH] D86308: [CMake][compiler-rt][libunwind] Compile assembly files as ASM not C, unify workarounds

tambre updated this revision to Diff 288292. tambre added a comment. Rebase, just in case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86308/new/ https://reviews.llvm.org/D86308 Files: clang/runtime/CMakeLists.txt

[PATCH] D86308: [CMake][compiler-rt][libunwind] Compile assembly files as ASM not C, unify workarounds

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG45344cf7ac5b: [CMake][compiler-rt][libunwind] Compile assembly

[PATCH] D86308: [CMake][compiler-rt][libunwind] Compile assembly files as ASM not C, unify workarounds

tambre added a comment. In D86308#2233934 , @teemperor wrote: > In D86308#2229936 , @tambre wrote: > >> In D86308#2229901 , @teemperor >> wrote: >> >>> Sorry, just got

[PATCH] D86308: [CMake][compiler-rt][libunwind] Compile assembly files as ASM not C, unify workarounds

tambre updated this revision to Diff 287421. tambre added a comment. Gate Apple workaround behind CMake version 3.19 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86308/new/ https://reviews.llvm.org/D86308 Files: clang/runtime/CMakeLists.txt

[PATCH] D85706: [compiler-rt] Compile assembly files as ASM not C

tambre updated this revision to Diff 285692. tambre added a comment. Herald added a project: clang. Herald added a subscriber: cfe-commits. Pass CMAKE_ASM_COMPILER to compiler-rt in runtime build Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D85706: [compiler-rt] Compile assembly files as ASM not C

tambre added a comment. In D85706#2214009 , @phosek wrote: > One concern I have with this change is that we may not always consistently > update CMAKE_ASM_FLAGS or set CMAKE_ASM_COMPILER. This wouldn't make a > difference before, but it'll with this

[PATCH] D88518: Recognize setjmp and friends as builtins even if jmp_buf is not declared yet.

tambre added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88518/new/ https://reviews.llvm.org/D88518 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D87983: [Sema] Split special builtin type lookups into a separate function

This revision was automatically updated to reflect the committed changes. tambre marked an inline comment as done. Closed by commit rGf91f28c350df: [Sema] Split special builtin type lookups into a separate function (authored by tambre). Changed prior to commit:

[PATCH] D87983: [Sema] Split special builtin type lookups into a separate function

tambre added inline comments. Comment at: clang/lib/Sema/SemaLookup.cpp:924 +void Sema::LookupNecessaryTypesForBuiltin(Scope *S, unsigned ID) { + if (getLangOpts().ObjC && ID == Builtin::BIobjc_msgSendSuper) +LookupPredefedObjCSuperType(*this, S); rjmccall

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

tambre added a comment. In D77491#2282219 , @tambre wrote: > In D77491#2282166 , @dmajor wrote: > >> This commit broke Firefox builds on Mac with an error in the SDK headers. >> Could you please revert if a fix is

[PATCH] D87917: [Sema] Handle objc_super special lookup when checking builtin compatibility

tambre created this revision. tambre added a reviewer: rsmith. Herald added a project: clang. Herald added a subscriber: cfe-commits. tambre requested review of this revision. objc_super is special and needs LookupPredefedObjCSuperType() called before performing builtin type comparisons. This

[PATCH] D87917: [Sema] Handle objc_super special lookup when checking builtin compatibility

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rGa1aa330b202f: [Sema] Handle objc_super special lookup when checking builtin compatibility (authored by tambre).

[PATCH] D87917: [Sema] Handle objc_super special lookup when checking builtin compatibility

tambre added a comment. I'll go ahead and commit this, as it's affecting an user and the fix is simple and obvious. Hopefully post-commit review is acceptable per my understanding of the rules. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D87917: [Sema] Handle objc_super special lookup when checking builtin compatibility

tambre added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:9674 ASTContext::GetBuiltinTypeError Error; +LookupPredefedObjCSuperType(*this, S, NewFD->getIdentifier()); QualType BuiltinType = Context.GetBuiltinType(BuiltinID,

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

tambre added a comment. In D77491#2299938 , @rsmith wrote: > We've hit a fairly subtle miscompile caused by this patch. > > glibc's setjmp.h looks like this (irrelevant parts removed): > > struct __jmp_buf_tag { /*...*/ }; > extern int

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe09107ab80dc: [Sema] Introduce BuiltinAttr, per-declaration

[PATCH] D79800: [Sema] Implement DR2233

tambre updated this revision to Diff 265855. tambre marked an inline comment as done. tambre added a comment. Improve comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79800/new/ https://reviews.llvm.org/D79800 Files:

[PATCH] D79800: [Sema] Implement DR2233

tambre added a comment. Thanks for the reviews! I believe this now handles all cases and with this we're standards-conforming in regard to DR777 and DR2233. Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:1987 + +if (Function->getNumParams() >=

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

tambre updated this revision to Diff 265872. tambre marked 4 inline comments as done. tambre added a comment. Remove memcpy overload tests from warn-fortify-source.c, which relied on identifier-based builtin identification. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

tambre updated this revision to Diff 265871. tambre marked 6 inline comments as done. tambre added a comment. Weakened noexcept checking. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77491/new/ https://reviews.llvm.org/D77491 Files:

[PATCH] D77491: [Sema] Introduce BuiltinAttr, per-declaration builtin-ness

tambre added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:8880 + } +} + rjmccall wrote: > tambre wrote: > > rjmccall wrote: > > > Hmm. I'm concerned about not doing any sort of semantic compatibility > > > check here before we assign the

[PATCH] D79800: [Sema] Remove default values for arguments prior to a parameter pack if the pack is used

tambre updated this revision to Diff 265850. tambre marked 5 inline comments as done. tambre added a comment. Handle multiple parameter packs interleaved with default values. Mark DR777 as superseded by DR2233. Mark DR2233 as resolved. Moved tests from dr7xx.cpp to dr22xx.cpp. Added note in

  1   2   >