[PATCH] D139586: [Clang][C++23] Lifetime extension in range-based for loops

hubert.reinterpretcast requested changes to this revision. hubert.reinterpretcast added a comment. This revision now requires changes to proceed. Waiting on changes to fix the visitation to locate the appropriate temporaries. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D142358: Opting out of Clang 16 ABI Changes for AIX and z/OS

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0f5099cd9422: Opting out of Clang 16 ABI Changes for AIX and z/OS (authored by nicolerabjohn, committed by hubert.reinterpretcast). Repository:

[PATCH] D142358: Opting out of Clang 15 ABI Changes for AIX and z/OS

hubert.reinterpretcast added a comment. Since this is a change only for AIX and z/OS, and considering that this is a patch to prevent ABI changes compared to the current ABI baselines for those platforms, I will land this ahead of the LLVM 16 branching. CHANGES SINCE LAST ACTION

[PATCH] D142358: Opting out of Clang 15 ABI Changes for AIX and z/OS

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. This revision is now accepted and ready to land. Thanks @nicolerabjohn; this LGTM! Comment at: clang/test/SemaCXX/class-layout.cpp:685 +#if (defined(CLANG_ABI_COMPAT) && CLANG_ABI_COMPAT <=

[PATCH] D142358: Opting out of Clang 15 ABI Changes for AIX and z/OS

hubert.reinterpretcast added inline comments. Comment at: clang/test/SemaCXX/class-layout.cpp:688 }; #if defined(CLANG_ABI_COMPAT) && CLANG_ABI_COMPAT <= 15 _Static_assert(sizeof(t3) == 8, ""); hubert.reinterpretcast wrote: > hubert.reinterpretcast wrote: > >

[PATCH] D142358: Opting out of Clang 15 ABI Changes for AIX and z/OS

hubert.reinterpretcast added inline comments. Comment at: clang/test/SemaCXX/class-layout.cpp:18 // expected-no-diagnostics +#if !defined(__MVS__) && !defined(_AIX) dblaikie wrote: > What's the reason this part is #ifdef'd out? Does it contain code that's >

[PATCH] D142358: Opting out of Clang 15 ABI Changes for AIX and z/OS

hubert.reinterpretcast added inline comments. Comment at: clang/test/SemaCXX/class-layout.cpp:688 }; #if defined(CLANG_ABI_COMPAT) && CLANG_ABI_COMPAT <= 15 _Static_assert(sizeof(t3) == 8, ""); hubert.reinterpretcast wrote: > Because z/OS has some of the

[PATCH] D142358: Opting out of Clang 15 ABI Changes for AIX and z/OS

hubert.reinterpretcast added inline comments. Comment at: clang/test/SemaCXX/class-layout.cpp:14 +// RUN: %clang_cc1 -triple powerpc64-ibm-aix7.3.0.0 %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -fclang-abi-compat=15 -DCLANG_ABI_COMPAT=15 +//RUN: %clang_cc1

[PATCH] D142358: Opting out of Clang 15 ABI Changes for AIX and z/OS

hubert.reinterpretcast added inline comments. Comment at: clang/test/SemaCXX/class-layout.cpp:14 +// RUN: %clang_cc1 -triple powerpc64-ibm-aix7.3.0.0 %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -fclang-abi-compat=15 -DCLANG_ABI_COMPAT=15 +//RUN: %clang_cc1

[PATCH] D142358: Opting out of Clang 15 ABI Changes for AIX and z/OS

hubert.reinterpretcast added inline comments. Comment at: clang/docs/ReleaseNotes.rst:817 attribute is also specified on the member. Clang historically did perform such packing. Clang now matches the gcc behavior (except on Darwin and PS4). You can switch back to the

[PATCH] D142358: Opting out of Clang 15 ABI Changes for AIX and z/OS

hubert.reinterpretcast added inline comments. Comment at: clang/test/SemaCXX/class-layout.cpp:10 // RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base -fclang-abi-compat=16 -DCLANG_ABI_COMPAT=16 // expected-no-diagnostics

[PATCH] D137756: [z/OS][pg] Throw error when using -pg on z/OS

hubert.reinterpretcast added a comment. In D137756#4018666 , @MaskRay wrote: > Most targets reject `-p` now. It's unnecessary to have another z/OS specific > diagnostic. So this patch can be abandoned. > > % fclang -p a.cc > clang-16: error:

[PATCH] D139586: [Clang][C++23] Lifetime extension in range-based for loops

hubert.reinterpretcast added a comment. At least on the surface, it looks like there will be a lot of trouble to deal with default arguments: struct A { A(); ~A(); int x[3]; }; int ((const A & = A()))[3]; void bar(int); void foo() { for (auto e : f()) { bar(e); } } We get a

[PATCH] D139586: [Clang][C++23] Lifetime extension in range-based for loops

hubert.reinterpretcast added a comment. In D139586#3984118 , @cor3ntin wrote: > I will try to do a more detailed review later, but at least I think we might > want more tests. Maybe codegen tests that do not rely on > [[clang::lifetimebound]], and

[PATCH] D139586: [Clang][C++23] Lifetime extension in range-based for loops

hubert.reinterpretcast added a comment. In D139586#3987252 , @royjacobson wrote: > The (non-wording) paper makes a pretty convincing case to just apply this > retroactively to any C++11 code >

[PATCH] D137986: [Clang][CodeGen][AIX] Map __builtin_frexpl, __builtin_ldexpl, and __builtin_modfl to 'double' version lib calls in 64-bit 'long double' mode

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. This revision is now accepted and ready to land. LGTM; thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137986/new/ https://reviews.llvm.org/D137986

[PATCH] D137986: [Clang][CodeGen][AIX] Map __builtin_frexpl, __builtin_ldexpl, and __builtin_modfl to 'double' version lib calls in 64-bit 'long double' mode

hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:112 + // double' mode. + static SmallDenseMap AIXLongDoubleBuiltins{ + {Builtin::BI__builtin_frexpl, "frexp"}, Please rename to "AIXLongDouble64Builtins".

[PATCH] D137437: [lit][AIX] Convert clang tests to use 'target={{.*}}aix{{.*}}'

hubert.reinterpretcast added inline comments. Comment at: clang/test/ClangScanDeps/modules-no-undeclared-includes.c:3 // section in XCOFF yet. -// UNSUPPORTED: aix +// UNSUPPORTED: target={{.*}}aix{{.*}} Perhaps we should normalize on `-aix` while we are

[Diffusion] rGfdab9f1203ee: [Clang] Check for response file existence prior to check for recursion

hubert.reinterpretcast added a subscriber: cfe-commits. BRANCHES main Users: sepavloff (Author) https://reviews.llvm.org/rGfdab9f1203ee ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D136568: [Clang] Support constexpr builtin ilogb

hubert.reinterpretcast added inline comments. Comment at: clang/test/Sema/constant-builtins-ilogb.cpp:1 +// RUN: %clang_cc1 -fsyntax-only -verify %s +// expected-no-diagnostics aaron.ballman wrote: > hubert.reinterpretcast wrote: > > There seems to be no C

[PATCH] D136568: [Clang] Support constexpr builtin ilogb

hubert.reinterpretcast requested changes to this revision. hubert.reinterpretcast added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/AST/ExprConstant.cpp:12452 +int Ilogb; +if (APFloat::opStatus St = ilogb(F, Ilogb);

[Diffusion] rGe4ec6ce8a75c: Clang: Add release note for defaulted-special-members-POD GCC ABI fix

hubert.reinterpretcast added inline comments. BRANCHES main /clang/docs/ReleaseNotes.rst:612-613 Thanks! Users: dblaikie (Author) https://reviews.llvm.org/rGe4ec6ce8a75c ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[Diffusion] rGe4ec6ce8a75c: Clang: Add release note for defaulted-special-members-POD GCC ABI fix

hubert.reinterpretcast added inline comments. BRANCHES main /clang/docs/ReleaseNotes.rst:612-613 Shouldn't the usage of POD here be clarified to say that we mean "POD for the purposes of Itanium ABI layout"? Users: dblaikie (Author) https://reviews.llvm.org/rGe4ec6ce8a75c

[Diffusion] rGe4ec6ce8a75c: Clang: Add release note for defaulted-special-members-POD GCC ABI fix

hubert.reinterpretcast added a subscriber: cfe-commits. BRANCHES main Users: dblaikie (Author) https://reviews.llvm.org/rGe4ec6ce8a75c ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D136568: [Clang] Support constexpr builtin ilogb

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:12452 +int Ilogb; +if (APFloat::opStatus St = ilogb(F, Ilogb); !isConstantOpStatus(St)) + return false; hubert.reinterpretcast wrote: > hubert.reinterpretcast

[PATCH] D136568: [Clang] Support constexpr builtin ilogb

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:12452 +int Ilogb; +if (APFloat::opStatus St = ilogb(F, Ilogb); !isConstantOpStatus(St)) + return false; hubert.reinterpretcast wrote: > Izaron wrote: > >

[PATCH] D136568: [Clang] Support constexpr builtin ilogb

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:12452 +int Ilogb; +if (APFloat::opStatus St = ilogb(F, Ilogb); !isConstantOpStatus(St)) + return false; Izaron wrote: > majnemer wrote: > > Izaron wrote: > > >

[PATCH] D135919: [Clang] Set thread_local Itanium ABI guard variables before calling constructors.

hubert.reinterpretcast added a comment. In D135919#3859491 , @tahonermann wrote: >> the case from https://github.com/llvm/llvm-project/issues/57828 is not for a >> block-scope variable, but the case in the patch description is... > > Thanks, Hubert.

[PATCH] D135919: [Clang] Set thread_local Itanium ABI guard variables before calling constructors.

hubert.reinterpretcast added a comment. In D135919#3858808 , @rjmccall wrote: > We can't set the flag if initialization is aborted by an exception, which is > not ruled out by the use of thread-unsafe statics. So this is not a correct > change.

[PATCH] D135848: [clang][Module][AIX] Mark test unsupported since objc doesn't have xcoff support

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. Confirming LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135848/new/ https://reviews.llvm.org/D135848 ___

[PATCH] D135384: [AIX] Enable the use of the -pg flag

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. This revision is now accepted and ready to land. LGTM with minor comment; thanks! Comment at: llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp:39 +Triple

[PATCH] D135848: [clang][Module][AIX] Mark test unsupported since objc doesn't have xcoff support

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. This revision is now accepted and ready to land. LGTM with comment (matches similar files already present in the directory). Comment at: clang/test/Modules/module-file-home-is-cwd.m:1 +//

[PATCH] D128750: [C++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

hubert.reinterpretcast added a comment. > I reached out to the GCC author to confirm that the committee acknowledges > that there should be a change to temp.func.order p6.2.1, but no consensus on > what changes to make [3]. A more accurate description is that some changes elsewhere to overload

[PATCH] D135384: [AIX] Enable the use of the -pg flag

hubert.reinterpretcast added inline comments. Comment at: llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp:38-40 +const Twine targetTwine(M.getTargetTriple()); +Triple targetTriple(targetTwine); +if (targetTriple.isOSAIX()) { A `Twine` can be

[PATCH] D128745: [c++] implements DR692, DR1395 and tentatively DR1432, about partial ordering of variadic template partial specialization or function template

hubert.reinterpretcast added a comment. In D128745#3834435 , @ychen wrote: > New flag or not, as long as we allow users to use the old behavior, it is > already fracturing the ecosystem. Do you mean (1) we shouldn't give the user > the choices or (2)

[PATCH] D128745: [c++] implements DR692, DR1395 and tentatively DR1432, about partial ordering of variadic template partial specialization or function template

hubert.reinterpretcast added a comment. In D128745#3834313 , @ychen wrote: > I think the consensus is some flag is needed to put it back to legacy > behavior just in case. I am not sure there is strong consensus to add a flag "just in case".

[PATCH] D128745: [c++] implements DR692, DR1395 and tentatively DR1432, about partial ordering of variadic template partial specialization or function template

hubert.reinterpretcast added a comment. In D128745#3833291 , @aaron.ballman wrote: > @rjmccall is correct about us not being required to apply DRs when they're > disruptive, but at the same time, WG21 DRs are intended to be handled as if > the

[PATCH] D134284: [AIX] change the clang tests with llvm-nm -Xany

hubert.reinterpretcast added a comment. Instead of applying `OBJECT_MODE` to everything, maybe we can apply it only to the tool invocations via a LIT expansion? Something like `%llvm-nm` that expands (for AIX) to `OBJECT_MODE=any llvm-nm`? Would need to check that the LIT internal shell accepts

[PATCH] D131541: [Sema] Fix friend destructor declarations after D130936

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. This revision is now accepted and ready to land. This LGTM; thanks! In D131541#3721383 , @royjacobson wrote: > I still don't diagnose the dependent friend case, but

[PATCH] D131541: [Sema] Fix friend destructor declarations after D130936

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:11522 + if (NewFD->getFriendObjectKind() == Decl::FriendObjectKind::FOK_None || + !NewFD->isDependentContext()) { +QualType ClassType = Destructor->getThisObjectType();

[PATCH] D131541: [Sema] Fix friend destructor declarations after D130936

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:11518-11520 + // FIXME: We still don't diagnose on this case + // template + // struct A { friend T::S::~V(); }; There's nothing we can diagnose about this

[PATCH] D130936: [SemaCXX] Validate destructor is valid for dependent classes

hubert.reinterpretcast added a comment. This does not work for friend declarations. template struct A { friend T::S::~S(); private: static constexpr int secret = 42; }; struct Q { struct S { ~S(); }; }; Q::S::~S() { void foo(int); foo(A::secret); }

[PATCH] D49863: [istream] Fix error flags and exceptions propagated from input stream operations

hubert.reinterpretcast added inline comments. Comment at: libcxx/trunk/include/istream:1223 +__state |= ios_base::badbit; return -1; } @ldionne, another dubious aspect of this patch is that it initializes `__r` to

[PATCH] D129401: [libLTO] Set data-sections by default in libLTO.

hubert.reinterpretcast added a comment. With respect to the compiler changes: they look good. I checked, and the injected options being passed to the linker are added before user-specified ones (and that is good). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D129401: [libLTO] Set data-sections by default in libLTO.

hubert.reinterpretcast added a comment. Tests require fixing. Comment at: llvm/test/LTO/PowerPC/data-sections-aix.ll:18-19 + +; CHECK-NOT: (csect: .data) +; CHECK: g O .data {{.*}} var + Unfortunately (without the change)

[PATCH] D129401: [libLTO] Set data-sections by default in libLTO for ELF and XCOFF.

hubert.reinterpretcast added a comment. In D129401#3662857 , @quinnp wrote: > @hubert.reinterpretcast please correct me if I am wrong about why this change > is needed. I think your description is correct. Comment at:

[PATCH] D129912: [Tooling/DependencyScanning] Enable passing a `vfs::FileSystem` object to `DependencyScanningTool`

hubert.reinterpretcast added a comment. In D129912#3661282 , @akyrtzi wrote: > Sorry about that, it should be fixed via > https://github.com/llvm/llvm-project/commit/d1b58cada61aa8bc44d8e8ef9c23ed12ef7b549b Thanks for the fix; it does look promising.

[PATCH] D129912: [Tooling/DependencyScanning] Enable passing a `vfs::FileSystem` object to `DependencyScanningTool`

hubert.reinterpretcast added a comment. The added test is not passing on the AIX builder: https://lab.llvm.org/buildbot/#/builders/214/builds/2388/steps/6/logs/FAIL__Clang-Unit__83 Note that Clang on that platform generates assembly by default (then invokes the system assembler). Repository:

[PATCH] D128059: [Clang] Add a warning on invalid UTF-8 in comments.

hubert.reinterpretcast added a comment. In D128059#3640564 , @cor3ntin wrote: > Good point. The error was a bit misleading but i guess what's happening is a > segfault when running `clang-ast-dump`. > I'm reverting for now and I don't really know how

[PATCH] D128059: [Clang] Add a warning on invalid UTF-8 in comments.

hubert.reinterpretcast added a comment. In D128059#3640424 , @cor3ntin wrote: > @aaron.ballman Thanks for the review. I landed the changes and got a bunch of > bots screaming at me for changes that are completely unrelated > >

[PATCH] D129165: [AIX][clang/test] Set/propagate AIXTHREAD_STK for AIX

This revision was automatically updated to reflect the committed changes. Closed by commit rGbd8b55e609c8: [AIX][clang/test] Set/propagate AIXTHREAD_STK for AIX (authored by hubert.reinterpretcast). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D129165: [AIX][clang/test] Set/propagate AIXTHREAD_STK for AIX

hubert.reinterpretcast created this revision. hubert.reinterpretcast added reviewers: xingxue, daltenty, Jake-Egan, cebowleratibm. Herald added subscribers: ormris, arphaman, steven_wu, hiraditya, kristof.beyls. Herald added a project: All. hubert.reinterpretcast requested review of this

[PATCH] D128645: Update developer policy.

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. LGTM! Comment at: llvm/docs/DeveloperPolicy.rst:88 +#. Patches should be unified diffs with "infinite context" (i.e. using something + like `git diff -U99 main`). +

[PATCH] D128645: Update developer policy.

hubert.reinterpretcast added inline comments. Comment at: llvm/docs/DeveloperPolicy.rst:403-405 +Note, these mailing lists are moderated and it is not unusual for a large +commit to require a moderator to approve the email, so do not be concerned if a +commit does not

[PATCH] D128645: Update developer policy.

hubert.reinterpretcast added inline comments. Comment at: llvm/docs/DeveloperPolicy.rst:88 +#. Patches should be unified diffs with "infinite context" (i.e. using something + like `git diff -U99 main`). + Using `git diff` like this, there are risks that

[PATCH] D127544: Add no_sanitize('hwaddress') (and 'memtag', but that's a no-op).

hubert.reinterpretcast added a comment. In D127544#3609393 , @hctim wrote: > I see that the clang-ppc64le-linux bot is green with the second attempt > (https://lab.llvm.org/buildbot/#/builders/105/builds/27200). Please let me > know if you have further

[PATCH] D127544: Add no_sanitize('hwaddress') (and 'memtag', but that's a no-op).

hubert.reinterpretcast added a comment. In D127544#3609335 , @hctim wrote: > Sent fadc98b06befb674fa47da4f3d8606bf61bed681 > to > fix-forward. I thought `*-registered-target` is

[PATCH] D127544: Add no_sanitize('hwaddress') (and 'memtag', but that's a no-op).

hubert.reinterpretcast added a comment. From the content of this patch, it probably is `llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp`: switch (TargetTriple.getArch()) { case Triple::x86_64: // The signal handler will find the data address in rdi. Asm = InlineAsm::get(

[PATCH] D127544: Add no_sanitize('hwaddress') (and 'memtag', but that's a no-op).

hubert.reinterpretcast added a comment. This is causing "unsupported architecture" errors on bots. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127544/new/ https://reviews.llvm.org/D127544 ___

[PATCH] D124556: [NFC] Prevent shadowing a variable declared in `if`

hubert.reinterpretcast added a comment. In D124556#3482224 , @ken-matsui wrote: > Thank you! Thanks in turn. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124556/new/ https://reviews.llvm.org/D124556

[PATCH] D124556: [NFC] Prevent shadowing a variable declared in `if`

This revision was automatically updated to reflect the committed changes. Closed by commit rG52ce95a1a554: [NFC] Prevent shadowing a variable declared in `if` (authored by ken-matsui, committed by hubert.reinterpretcast). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D124556: [NFC] Prevent shadowing a variable declared in `if`

hubert.reinterpretcast added a comment. In D124556#3480228 , @ken-matsui wrote: > @hubert.reinterpretcast > > Sorry, I'm a newbie here, but is there anything I should do after getting > approved? I'm not sure if the instructions are a bit out-of-date:

[PATCH] D124556: [NFC] Prevent shadowing a variable declared in `if`

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. This revision is now accepted and ready to land. LGTM; thanks for the patch! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124556/new/

[PATCH] D124556: [NFC] Prevent shadowing a variable declared in `if`

hubert.reinterpretcast added a comment. In D124556#3479833 , @ken-matsui wrote: > @hubert.reinterpretcast, > > Sorry to have missed providing a summary. You can still provide one by using the "Edit Revision" link. I suggest something like: Prevents

[PATCH] D124556: [NFC] Prevent shadowing a variable declared in `if`

hubert.reinterpretcast added a comment. @ken-matsui, can you provide some rationale for the change (got compiler warning/error)? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124556/new/ https://reviews.llvm.org/D124556

[PATCH] D122377: [PowerPC] Support 16-byte lock free atomics on pwr8 and up

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. This revision is now accepted and ready to land. LGTM with minor comments. Comment at: clang/test/CodeGen/PowerPC/atomic-alignment.c:1-12 // RUN: %clang_cc1 -verify -triple

[PATCH] D122377: [PowerPC] Support 16-byte lock free atomics on pwr8 and up

hubert.reinterpretcast added inline comments. Comment at: clang/test/CodeGen/PowerPC/atomic-alignment.c:14 // PPC: @c = global i8 0, align 1{{$}} _Atomic(char) c; // expected-no-diagnostics I was recently informed that, instead of using

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

hubert.reinterpretcast added a comment. In D122983#3429060 , @aaron.ballman wrote: > Morally, yes, that's reasonable in CodeGen because you're ensuring you get no > diagnostics. Practically, that's a convoluted, more expensive, less > maintainable way

[PATCH] D122377: [PowerPC] Support 16-byte lock free atomics on pwr8 and up

hubert.reinterpretcast added a comment. I am not sure that the choice of `isOSBinFormatELF` to (afaik) primarily scope this change from affecting AIX (where we know the library calls are not implemented to be lock-free yet) is better than alternative where the condition is for little-endian

[PATCH] D122377: [PowerPC] Support 16-byte lock free atomics on pwr8 and up

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Basic/Targets/PPC.h:443 -// PPC64 supports atomics up to 8 bytes. -MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 64; +// PPC64 supports atomics up to 16 bytes. +MaxAtomicPromoteWidth = 128;

[PATCH] D122377: [PowerPC] Support 16-byte lock free atomics on pwr8 and up

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Basic/Targets/PPC.h:448 + void setMaxAtomicWidth() override { +// For layout on ELF targets, we support up to 16 bytes. +if (getTriple().isOSBinFormatELF()) hubert.reinterpretcast

[PATCH] D122377: [PowerPC] Support 16-byte lock free atomics on pwr8 and up

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Basic/Targets/PPC.h:448 + void setMaxAtomicWidth() override { +// For layout on ELF targets, we support up to 16 bytes. +if (getTriple().isOSBinFormatELF()) lkail wrote: >

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

hubert.reinterpretcast added a comment. In D122983#3426716 , @erichkeane wrote: > We typically avoid doing -verify in CodeGen (unless the diagnostic is > ACTUALLY in CodeGen) as a matter of business. I hope that `-verify` and `//

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

hubert.reinterpretcast added a comment. In D122983#3426541 , @paulwalker-arm wrote: > The tests verify a set of builtins do not exist when the associated feature > flag is not present. They sit within CodeGen because the tests were > plentiful and it

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

hubert.reinterpretcast added a comment. In D122983#3426450 , @aaron.ballman wrote: > Yeah, those tests seem to be overly-constraining. There's no reason to be > validating whether there's an implicit function declaration warning in a > *codegen* test.

[PATCH] D122377: [PowerPC][Linux] Support 16-byte lock free atomics on pwr8 and up

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Basic/Targets/PPC.h:448 + void setMaxAtomicWidth() override { +// For layout on ELF targets, we support up to 16 bytes. +if (getTriple().isOSBinFormatELF()) I believe this should be

[PATCH] D122377: [PowerPC][Linux] Support 16-byte lock free atomics on pwr8 and up

hubert.reinterpretcast added inline comments. Comment at: clang/test/CodeGen/PowerPC/atomic-alignment.c:34 // PPC32: @o = global %struct.O zeroinitializer, align 1{{$}} // PPC64: @o = global %struct.O zeroinitializer, align 8{{$}} Just noting that GCC

[PATCH] D121992: [Clang] [Driver] Add option to set alternative toolchain path

hubert.reinterpretcast added a comment. In D121992#3418443 , @MaskRay wrote: > If you intend to overlay ld.so, you'll necessarily overlay libc, then > --sysroot seems just unneeded at all. Why? The header and library search paths are not restricted to

[PATCH] D122377: [PowerPC][Linux] Support 16-byte lock free atomics on pwr8 and up

hubert.reinterpretcast added inline comments. Comment at: clang/test/CodeGen/PowerPC/atomic-alignment.c:45 +// PPC64: @q = global %struct.Q zeroinitializer, align 16{{$}} +// AIX64: @q = global %struct.Q zeroinitializer, align 1{{$}} _Atomic(Q) q; // expected-no-diagnostics

[PATCH] D122249: [Clang] Add a compatibiliy warning for non-literals in constexpr.

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. This revision is now accepted and ready to land. This LGTM (with minor comment). Please wait for Aaron to respond re: the handling of template instantiations. Comment at:

[PATCH] D121927: [Clang] Work with multiple pragmas weak before definition

This revision was automatically updated to reflect the committed changes. Closed by commit rGce21c926f8ef: [Clang] Work with multiple pragmas weak before definition (authored by hubert.reinterpretcast). Changed prior to commit: https://reviews.llvm.org/D121927?vs=417755=418092#toc Repository:

[PATCH] D121927: [Clang] Work with multiple pragmas weak before definition

hubert.reinterpretcast updated this revision to Diff 417755. hubert.reinterpretcast added a comment. - Adjust per observation: Use DenseMapInfo for the alias pointer value Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121927/new/

[PATCH] D121927: [Clang] Work with multiple pragmas weak before definition

hubert.reinterpretcast added inline comments. Comment at: clang/include/clang/Sema/Weak.h:47 +static unsigned getHashValue(const WeakInfo ) { + return W.getAlias() ? DenseMapInfo::getHashValue(W.getAlias()->getName()) + :

[PATCH] D121992: [Clang] [Driver] Add option to set alternative toolchain path

hubert.reinterpretcast added a comment. LGTM; thank you. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121992/new/ https://reviews.llvm.org/D121992 ___ cfe-commits mailing list

[PATCH] D121927: [Clang] Work with multiple pragmas weak before definition

hubert.reinterpretcast added a comment. @aaron.ballman, I believe I have responded to all of the comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121927/new/ https://reviews.llvm.org/D121927 ___

[PATCH] D121927: [Clang] Work with multiple pragmas weak before definition

hubert.reinterpretcast updated this revision to Diff 417666. hubert.reinterpretcast marked an inline comment as done. hubert.reinterpretcast added a comment. - Address review comments: Return `const` from getAlias() Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D122249: [Clang] Add a compatibiliy warning for non-literals in constexpr.

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:1905-1907 + CheckLiteralType(SemaRef, Kind, VD->getLocation(), VD->getType(), + diag::warn_cxx20_compat_constexpr_var, +

[PATCH] D122261: [Clang][NFC] Some `const` for `IdentifierInfo *`s feeding `DeclarationName`

This revision was automatically updated to reflect the committed changes. Closed by commit rGda167a53c87f: [Clang][NFC] Some `const` for `IdentifierInfo *`s feeding `DeclarationName` (authored by hubert.reinterpretcast). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D122249: [Clang] Add a compatibiliy warning for non-literals in constexpr.

hubert.reinterpretcast added inline comments. Comment at: clang/test/SemaCXX/constant-expression-cxx2b.cpp:219 + NonLiteral n; // cxx2b-note {{non-literal type 'NonLiteral' cannot be used in a constant expression}} \ +// cxx2b-warning {{definition of a

[PATCH] D122249: [Clang] Add a compatibiliy warning for non-literals in constexpr.

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:1905 +if (SemaRef.LangOpts.CPlusPlus2b) { + if (!VD->getType()->isLiteralType(SemaRef.Context)) +SemaRef.Diag(VD->getLocation(), This seems to

[PATCH] D121927: [Clang] Work with multiple pragmas weak before definition

hubert.reinterpretcast updated this revision to Diff 417443. hubert.reinterpretcast added a comment. - Address review comments: Add release notes, expand comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121927/new/

[PATCH] D121927: [Clang] Work with multiple pragmas weak before definition

hubert.reinterpretcast added inline comments. Comment at: clang/include/clang/Sema/Weak.h:33 + : alias(Alias), loc(Loc) {} + inline IdentifierInfo *getAlias() const { return alias; } inline SourceLocation getLocation() const { return loc; }

[PATCH] D122261: [Clang][NFC] Some `const` for `IdentifierInfo *`s feeding `DeclarationName`

hubert.reinterpretcast created this revision. hubert.reinterpretcast added a reviewer: aaron.ballman. Herald added a project: All. hubert.reinterpretcast requested review of this revision. Herald added a project: clang. `DeclarationName` already takes `const IdentifierInfo *`. Propagate the

[PATCH] D121927: [Clang] Work with multiple pragmas weak before definition

hubert.reinterpretcast updated this revision to Diff 417386. hubert.reinterpretcast marked 2 inline comments as done. hubert.reinterpretcast added a comment. - Address review comments: Use default member init in WeakInfo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D121927: [Clang] Work with multiple pragmas weak before definition

hubert.reinterpretcast marked an inline comment as done. hubert.reinterpretcast added inline comments. Comment at: clang/include/clang/Sema/Weak.h:33 + : alias(Alias), loc(Loc) {} + inline IdentifierInfo *getAlias() const { return alias; } inline SourceLocation

[PATCH] D121927: [Clang] Work with multiple pragmas weak before definition

hubert.reinterpretcast marked an inline comment as done. hubert.reinterpretcast added inline comments. Comment at: clang/include/clang/Sema/Weak.h:62 +return false; + return LHS.getAlias()->getName() == RHS.getAlias()->getName(); +}

[PATCH] D111400: [Clang][C++2b] P2242R3: Non-literal variables [...] in constexpr

hubert.reinterpretcast added a comment. Confirming LGTM with minor comments. In D111400#3399929 , @cor3ntin wrote: > - We may still miss a compatibility warning for non-literal in c++23 mode? My understanding is that Aaron and I both want the warning

[PATCH] D111400: [Clang][C++2b] P2242R3: Non-literal variables [...] in constexpr

hubert.reinterpretcast added inline comments. Comment at: clang/include/clang/Basic/DiagnosticASTKinds.td:69 +def note_constexpr_static_local : Note< + "control flows through the declaration of a %select{static|thread_local}0 variable">; def

[PATCH] D111400: [Clang][C++2b] P2242R3: Non-literal variables [...] in constexpr

hubert.reinterpretcast added inline comments. Comment at: clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3-2b.cpp:17-22 +constexpr int g() { // expected-error {{constexpr function never produces a constant expression}} + goto test;// expected-note {{subexpression not

[PATCH] D111400: [Clang][C++2b] P2242R3: Non-literal variables [...] in constexpr

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. This revision is now accepted and ready to land. LGTM with minor nit. Thank you. Comment at: clang/test/SemaCXX/constant-expression-cxx2b.cpp:4 + +struct NonLiteral { //cxx2a-note

<    1   2   3   4   5   6   7   8   >