[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: > hubert.reinter

[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 wrote:

[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 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 mod

[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] 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.

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 https://revie

[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 `0

[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); } Rep

[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 witho

[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] D104420: thread_local support for AIX

hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:265 +/// Create a stub function, suitable for being passed to atexit, +/// which passes the given address to the given destructor function. Since the function has some spe

[PATCH] D104420: thread_local support for AIX

hubert.reinterpretcast added a comment. I think we're good after some last updates. Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:340-342 + // Ignore all attributes except ConstInit when IgnoreAttrs is true. + bool isEmittedWithConstantInitializer(const VarDecl *VD, +

[PATCH] D104420: thread_local support for AIX

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. This revision is now accepted and ready to land. LGTM; thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104420/new/ https://reviews.llvm.org/D104420 _

[PATCH] D106074: [AIX] Emit unsupported 128-bit long double option for AIX

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4845 + if (Arg *A = Args.getLastArg(options::OPT_mlong_double_128)) { +// AIX doesn't support 128-bit long double yet. +if (Triple.isOSAIX()) cebowleratibm wr

[PATCH] D106371: [AIX] Generate large code model relocations when mcmodel=medium on AIX

hubert.reinterpretcast added a comment. LGTM with minor nit; thanks. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5140 +A->render(Args, CmdArgs); +} else D.Diag(diag::err_drv_invalid_argument_to_option) Add braces to the `else` block whe

[PATCH] D106393: [PowerPC][AIX] Add support for varargs for complex types on AIX

hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:4577-4583 + // If we have a complex type and the base type is smaller than 8 bytes, + // the ABI calls for the real and imaginary parts to be right-adjusted + // in separate doublewords.

[PATCH] D106371: [AIX] Generate large code model relocations when mcmodel=medium on AIX

hubert.reinterpretcast added a comment. In D106371#2894736 , @anjankgk wrote: > Anjan Kumar Guttahalli Krishna > > Thank you! I running a build with this now -- I'll check its status in the morning. CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D106371: [AIX] Generate large code model relocations when mcmodel=medium on AIX

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7d669ec1: [AIX] Generate large code model relocations when mcmodel=medium on AIX (authored by anjankgk, committed by hubert.reinterpretcast). R

[PATCH] D106577: [clang] Define __STDC_ISO_10646__

hubert.reinterpretcast added a comment. In D106577#2897588 , @aaron.ballman wrote: > In D106577#2897522 , @jyknight > wrote: > >> I'm not sure we should be populating this. >> >> The _value_ is determined by wha

[PATCH] D106914: [libclang] Check LLVM_HAVE_LINK_VERSION_SCRIPT

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/D106914/new/ https://reviews.llvm.org/D106914 __

[PATCH] D106920: [AIX] Update fetch_and_add type

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. This revision is now accepted and ready to land. Thanks. LGTM with minor nits. Comment at: clang/test/CodeGen/builtins-ppc-xlcompat-fetch.c:19 // -void test_builtin_ppc_fetch_and_add(unsigne

[PATCH] D107063: Set TargetCPUName for AIX to default to pwr7.

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. This revision is now accepted and ready to land. LGTM with comment. Comment at: clang/test/Driver/aix-mcpu-default.c:9 -// Check that the target cpu defaults to power4 on AIX7.1 and below.

[PATCH] D111400: [Clang] Implement P2242R3

hubert.reinterpretcast added a comment. @aaron.ballman, I haven't had a chance to look at the code in detail yet, but I would like to get on the same page about the error/warning behaviour. In the older modes, we stay conforming either - by producing the warning (when we know the user said they

[PATCH] D111400: [Clang] Implement P2242R3

hubert.reinterpretcast added a comment. In D111400#3376070 , @hubert.reinterpretcast wrote: > In C++2b, the `-Wc++20-compat` warning is produced for the places where we > warn above and is not produced for the places where we don't warn above. I'm not

[PATCH] D111400: [Clang] Implement P2242R3

hubert.reinterpretcast added a comment. Noting for myself: Clang's status quo already has behaviours that are similar to P2242R3 in its C++20 mode despite those behaviours being non-conforming and contributing to binary compat breakage with GCC. This patch is not responsible for those behaviours

[PATCH] D111400: [Clang] Implement P2242R3

hubert.reinterpretcast added inline comments. Comment at: clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3-2b.cpp:16 +} + +constexpr int f(int x) { Add a `NonLiteral` case and a case with a labelled statement and no `goto`? Comment at: clang/te

[PATCH] D111400: [Clang] Implement P2242R3

hubert.reinterpretcast added a comment. @aaron.ballman @cor3ntin, are we confident that testing the non-lambda cases is sufficient to cover the lambda cases as well? I suggest using a pattern such as: int (*test_cxx2b_constexpr_label_in_body())() { auto qq = []() { label: return 42;

[PATCH] D121441: [PowerPC][NFC] Add atomic alignments and ops tests for powerpc

hubert.reinterpretcast added inline comments. Comment at: clang/test/CodeGen/PowerPC/atomic-alignment.c:2-6 +// RUN: %clang_cc1 -verify -triple powerpc-unkonwn-unknown -emit-llvm -o - %s | \ +// RUN: FileCheck %s --check-prefixes=PPC,PPC32 +// RUN: %clang_cc1 -verify -triple p

[PATCH] D121441: [PowerPC][NFC] Add atomic alignments and ops tests for powerpc

hubert.reinterpretcast added inline comments. Comment at: clang/test/CodeGen/PowerPC/atomic-alignment.c:1 +// REQUIRES: powerpc-registered-target +// RUN: %clang_cc1 -verify -triple powerpc-unkonwn-unknown -emit-llvm -o - %s | \ I am not sure about having a `Cod

[PATCH] D111400: [Clang] Implement P2242R3

hubert.reinterpretcast added inline comments. Comment at: clang/test/SemaCXX/constant-expression-cxx2b.cpp:101 + + auto b = [](int n) constexpr { +if (!n) I think a lambda marked `constexpr` and one that isn't has fundamental differences in behaviour in rel

[PATCH] D121441: [PowerPC][NFC] Add atomic alignments and ops tests for powerpc

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. LGTM with comment (not blocking). Comment at: clang/test/CodeGen/PowerPC/atomic-alignment.c:1 +// REQUIRES: powerpc-registered-target +// RUN: %clang_cc1 -verify -triple powerpc-unkonwn-unkno

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

hubert.reinterpretcast created this revision. hubert.reinterpretcast added reviewers: jamieschmeiser, aaron.ballman, chandlerc, cebowleratibm, w2yehia. Herald added a project: All. hubert.reinterpretcast requested review of this revision. Herald added a project: clang. Update `WeakUndeclaredIdent

[PATCH] D121441: [PowerPC][NFC] Add atomic alignments and ops tests for powerpc

hubert.reinterpretcast added inline comments. Comment at: clang/test/CodeGen/PowerPC/atomic-alignment.c:1 +// REQUIRES: powerpc-registered-target +// RUN: %clang_cc1 -verify -triple powerpc-unkonwn-unknown -emit-llvm -o - %s | \ lkail wrote: > hubert.reinterpret

[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:35 inline SourceLocation getLocation() const { return loc; } - void setUsed(bool Used=true) { used = Used; } - inline bool getUs

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

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:1904-1906 +if (!SemaRef.LangOpts.CPlusPlus2b && +CheckLiteralType(SemaRef, Kind, VD->getLocation(), VD->getType(), diag::err_constexpr_local_

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

hubert.reinterpretcast added a comment. > I think labels can be left as is. Yes, the `static`, `thread_local`, label, and `goto` cases would all categorically contribute to a IFNDR program in the older modes (and we currently reliably diagnose them in the definition context). ===

[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 va

[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/dtor.cpp:26 }; - struct Nonlit { Nonlit(); }; // expected-note {{not literal}} + struct Nonlit { // cxx2a-note {{'Nonlit' is not literal becaus}} +Nonlit(); --

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

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:1904-1906 +if (!SemaRef.LangOpts.CPlusPlus2b && +CheckLiteralType(SemaRef, Kind, VD->getLocation(), VD->getType(), diag::err_constexpr_local_

[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/dtor.cpp:26 }; - struct Nonlit { Nonlit(); }; // expected-note {{not literal}} + struct Nonlit { // cxx2a-note {{'Nonlit' is not literal becaus}} +Nonlit(); --

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

hubert.reinterpretcast added inline comments. Comment at: clang/test/SemaCXX/constant-expression-cxx2b.cpp:163 + +// Test whether lambda are correctly treated as implicitely constexpr under the relaxed c++23 rules. +int test_lambdas_implicitly_constexpr() { Fix

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

hubert.reinterpretcast added inline comments. Comment at: clang/include/clang/Driver/Driver.h:152 + /// Alternative toolchain path in prior to sysroot. + std::string OverlayToolChainPath; I don't understand the use of "in" in the comment. Perhaps "used" was m

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

hubert.reinterpretcast updated this revision to Diff 417042. hubert.reinterpretcast marked an inline comment as done. hubert.reinterpretcast added a comment. - Address review comments: Remove deleted non-operator member functions; add extra line to test Repository: rG LLVM Github Monorepo CH

[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 {{'NonLiter

[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 va

[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 note_constexpr_subobject_declared_h

[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 ad

[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(); +} aaron.ballman

[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 getLocati

[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 https://

[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 `const

[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; } hubert.reinte

[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/ https://reviews.llvm.org/D1

[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 tr

[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 v

[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 https://r

[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 `expected-no-diagnosti

[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 powerpc-unknown-u

[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] 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 con

[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/ https://reviews.llvm.org/D

[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`

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 https://

[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] 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] 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] 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: llvm/lib/LTO/LT

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

hubert.reinterpretcast added a comment. Is building `check-all` immediately after configuration expected to be clean? I have a build configured to build LLVM with just `clang` and just with the `PowerPC` target, and it seems `check-all` (under such a configuration) does not trigger the building

[PATCH] D68115: Zero initialize padding in unions

hubert.reinterpretcast added a comment. In D68115#1810891 , @lebedev.ri wrote: > Does this have to be an unilateral change, > likely penalizing non-`-ftrivial-auto-var-init=` cases, > i.e. [why] can't it be **only** done for when `-ftrivial-auto-var-init

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

hubert.reinterpretcast added a comment. In D65761#1804302 , @hubert.reinterpretcast wrote: > I have confirmed that the case I mentioned fails with rGd157a9b > . @ajpaverd, is a fix fo

[PATCH] D68115: Zero initialize padding in unions

hubert.reinterpretcast added a comment. In D68115#1820462 , @vitalybuka wrote: > So if I understand this the proposal is to have something like > -fzero-union-padding which is off by default. > When it's OFF compiler will continue to do whatever it does

[PATCH] D72736: [AIX] Add improved interface for retrieving load module paths

hubert.reinterpretcast added inline comments. Comment at: llvm/include/llvm/Support/FileUtilities.h:44 +if (!FuncAddr) { + return ""; +} I'm not sure that this function should handle failure by returning an empty string. The error condition can be m

[PATCH] D72736: [AIX] Add improved interface for retrieving load module paths

hubert.reinterpretcast added inline comments. Comment at: llvm/include/llvm/Support/FileUtilities.h:38 +#if defined(_AIX) + SmallString<128> getLoadModuleFilenameForFunctionAIX(void (*Func)(void)); +#endif This should be an internal function in the `.cpp` file i

[PATCH] D87611: [SystemZ][z/OS] Set aligned allocation unavailable by default for z/OS

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Driver/ToolChains/ZOS.cpp:25-27 +void ZOS::addClangTargetOptions(const llvm::opt::ArgList &DriverArgs, +llvm::opt::ArgStringList &CC1Args, +Action::

[PATCH] D87624: [SystemZ][z/OS] Set default wchar_t type for zOS

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87624/new/ https://reviews.llvm.org/D87624 ___ cfe-commits maili

[PATCH] D87611: [SystemZ][z/OS] Set aligned allocation unavailable by default for z/OS

hubert.reinterpretcast added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7216 + "aligned %select{allocation|deallocation}0 function of type '%1' is " + "%select{only|not}4 available on %2 %select{%3 or newer|}4">; def note_silence_aligned_allo

[PATCH] D87611: [SystemZ][z/OS] Set aligned allocation unavailable by default for z/OS

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. LGTM; thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87611/new/ https://reviews.llvm.org/D87611 ___ cfe-commi

[PATCH] D87927: [AIX][clang][driver] Make sure ctri.o is linked in C++ mode

hubert.reinterpretcast added inline comments. Comment at: clang/test/Driver/aix-ld.c:16 // CHECK-LD32: "-bpT:0x1000" "-bpD:0x2000" // CHECK-LD32: "[[SYSROOT]]/usr/lib{{/|}}crt0.o" // CHECK-LD32: "-L[[SYSROOT]]/usr/lib" There should be c

[PATCH] D87927: [AIX][clang][driver] Make sure crti[_64].o is linked in C++ 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/D87927/new/ https://reviews.llvm.org/D87927

[PATCH] D88182: [clang][driver][AIX] Set compiler-rt as default rtlib

hubert.reinterpretcast added inline comments. Comment at: clang/test/Driver/aix-ld.c:215 // CHECK-LD64-NO-DEFAULT-LIBS: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "powerpc64-ibm-aix7.1.0.0" +// CHECK-LD64-NO-STD-LIB: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]" // CHECK-LD64

[PATCH] D88182: [clang][driver][AIX] Set compiler-rt as default rtlib

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/D88182/new/ https://reviews.llvm.org/D88182

[PATCH] D88105: [NFC] [PPC] Add PowerPC expected IR tests for C99 complex

hubert.reinterpretcast added a comment. Looks to me that this (since it is approved) completely replaces D88130 . The patch description needs to be changed of course. Not sure if the committer wants to try splitting out the non-AIX test as an NFC patch. CHANGE

[PATCH] D88105: [NFC] [PPC] Add PowerPC expected IR tests for C99 complex

hubert.reinterpretcast added a comment. Off-list discussion seems to indicate that only the NFC portion of the patch was intended to be approved. That is, the scope of this patch is supposed to be https://reviews.llvm.org/D88105?id=293625 plus formatting changes. CHANGES SINCE LAST ACTION ht

[PATCH] D85324: [SystemZ][z/OS] Add z/OS Target and define macros

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Basic/Targets/OSTargets.h:736 +MacroBuilder &Builder) const override { +// FIXME: LONG_LONG should not be defined under -std=c89 +Builder.defineMacro("_LONG_LONG"); Mi

[PATCH] D85324: [SystemZ][z/OS] Add z/OS Target and define macros

hubert.reinterpretcast added inline comments. Comment at: clang/test/Preprocessor/init-zos.c:5 +// +// S390X-ZOS-CXX:#define _EXT 1 +// S390X-ZOS-C99:#define _ISOC99_SOURCE 1 Should this be `GNUXX`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D85324: [SystemZ][z/OS] Add z/OS Target and define macros

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Basic/Targets/OSTargets.h:743 +Builder.defineMacro("__BFP__"); +// FIXME: __BOOL__ should be defined under strict -std=c89. +Builder.defineMacro("__BOOL__"); MaskRay wrote: > What is

[PATCH] D85324: [SystemZ][z/OS] Add z/OS Target and define macros

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. This revision is now accepted and ready to land. LGTM from my end; although @MaskRay might want another look. Comment at: clang/lib/Basic/Targets/OSTargets.h:758 + Builder.defineMacro("_

[PATCH] D85324: [SystemZ][z/OS] Add z/OS Target and define macros

hubert.reinterpretcast added a comment. In D85324#2233290 , @abhina.sreeskantharajan wrote: > Thanks Hubert, I fixed the comment. Got it; I'll look into committing this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

[PATCH] D85324: [SystemZ][z/OS] Add z/OS Target and define macros

This revision was automatically updated to reflect the committed changes. Closed by commit rG97ccf93b3615: [SystemZ][z/OS] Add z/OS Target and define macros (authored by abhina.sreeskantharajan, committed by hubert.reinterpretcast). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[PATCH] D86707: [SystemZ][z/OS] Adding initial toolchain for z/OS

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Driver/ToolChains/ZOS.h:25 + + bool isPICDefault() const override { return false; } + bool isPIEDefault() const override { return false; } According to the RFC re: LLVM on z/OS, the initial sup

[PATCH] D86707: [SystemZ][z/OS] Adding initial toolchain for z/OS

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/lib/Driver/ToolChains/ZOS.cpp:15 + +using namespace clang::driver; +using namespace clang::driver:

[PATCH] D86707: [SystemZ][z/OS] Adding initial toolchain for z/OS

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. Confirming LGTM. @abhina.sreeskantharajan, it seems that you have a good number of commits to the project (I see at least three). If you do not yet have commit access, it may be appropriate to request it now s

[PATCH] D82727: [PowerPC] Implement Vector Expand Mask builtins in LLVM/Clang

hubert.reinterpretcast added inline comments. Comment at: llvm/lib/Target/PowerPC/PPCInstrPrefix.td:1006 "vexpandbm $vD, $vB", IIC_VecGeneral, - []>; + [(set v16i8:$vD,

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:6230 /// LHS < RHS, return -1. int ASTContext::getFloatingTypeOrder(QualType LHS, QualType RHS) const { FloatingRank LHSR = getFloatingRank(LHS); I think this function sho

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

hubert.reinterpretcast added inline comments. Comment at: clang/test/CodeGenCXX/ibm128-declarations.cpp:25 + +__ibm128 func4(__ibm128 a, __ibm128 b) { + return a + b; hubert.reinterpretcast wrote: > hubert.reinterpretcast wrote: > > There's a lot of missing test

[PATCH] D93377: [Clang] Add __ibm128 type to represent ppc_fp128

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:6230 /// LHS < RHS, return -1. int ASTContext::getFloatingTypeOrder(QualType LHS, QualType RHS) const { FloatingRank LHSR = getFloatingRank(LHS); nemanjai wrote: > hubert.

[PATCH] D88500: [AIX][Clang][Driver] Link libm along with libc++

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Driver/ToolChains/AIX.cpp:165 +// Since libc++ has dependencies on libm, if we have one then add the other. +if (getToolChain().ShouldLinkCXXStdlib(Args)) Is that right? My check of t

[PATCH] D88500: [AIX][Clang][Driver] Link libm in c++ mode

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/Driver/aix-ld.c:406 // CHECK-LD32-NOSTDLIBXX-LCXX: "[[RESOURCE_DIR]]{{/|}}lib{{/|\\

[PATCH] D88526: [clang][Sema] Fix PR47676: Handle dependent AltiVec C-style cast

hubert.reinterpretcast created this revision. hubert.reinterpretcast added reviewers: pkubaj, nemanjai, jasonliu, aaron.ballman. Herald added a project: clang. hubert.reinterpretcast requested review of this revision. Fix premature decision in the presence of type-dependent expression operands on

<    1   2   3   4   5   6   7   8   >