[PATCH] D82677: [Clang] Handle AIX Include management in the driver

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Driver/ToolChains/AIX.cpp:195 + // Return if -nostdlibinc is specified as a driver option. + if (DriverArgs.hasArg(options::OPT_nostdlibinc)) +return; stevewan wrote: > Can we rewrite this

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1884 + if (const BuiltinType *BTy = + Context.getBaseElementType(CTy->getElementType()) + ->getAs()) I don't think there's a reason

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/test/Layout/aix-double-struct-member.cpp:1 +// RUN: %clang_cc1 -emit-llvm-only -triple powerpc-ibm-aix-xcoff \ +// RUN: -fdump-record-layouts -fsyntax-only %s 2>/dev/null | \ Xiangling_L wrote: >

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1877 + CharUnits PreferredAlign = FieldAlign; + if (SupportsAIXPowerAlignment && FieldOffset == CharUnits::Zero() && + (IsUnion || FoundNonOverlappingEmptyField)) {

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1235 + // Do not use AIX special alignment if current base is not the first member or + // the struct is not a union. hubert.reinterpretcast wrote: > hubert.reint

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1908 // The align if the field is not packed. This is to check if the attribute // was unnecessary (-Wpacked). CharUnits UnpackedFieldAlign = FieldAlign; Test

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:2424 + (T->isSpecificBuiltinType(BuiltinType::LongDouble) && + Target->supportsAIXPowerAlignment())) // Don't increase the alignment if an alignment attribute was specified on

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:2409 +const RecordDecl *RD = RT->getDecl(); +return std::max(ABIAlign, static_cast(toBits( + getASTRecordLayout(RD).PreferredAlignment))); -

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1885 + Context.getBaseElementType(CTy->getElementType()) + ->getAs()) +if (BTy->getKind() == BuiltinType::Double || Xiangling_L w

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:666 + /// HasNonEmptyBaseClass - Whether the class has any non-empty class (in the + /// sense of (C++11 [meta.unary.prop])) as base. + bool HasNonEmptyBaseClass; M

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:2414 +assert(PreferredAlign >= ABIAlign && + "PreferredAlign is at least as large as ABIAlign."); +return PreferredAlign; Minor nit: s/is/should be/; ==

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1910 + +auto upgradeAlignmentIfQualified = [&](const BuiltinType *BTy) { + if (BTy->getKind() == BuiltinType::Double || "Qualified" is a term of art in the co

[PATCH] D81583: Update SystemZ ABI to handle C++20 [[no_unique_address]] attribute

hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:523 + if (isa(RT->getDecl())) { +if (!(AllowNoUniqueAddr && FD->hasAttr())) + return false; Minor nit: The nested `if` could be merged with the outer one: ``` i

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1064 setSize(getSize() + PtrWidth); setDataSize(getSize()); } I would suggest setting `HandledFirstNonOverlappingEmptyField` to `true` here with an asser

[PATCH] D81583: Update SystemZ ABI to handle C++20 [[no_unique_address]] attribute

hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:521 + // [[no_unique_address]] attribute (since C++20). Those do count + // as empty according to the Itanium ABI. This property is currently + // only respected if the AllowNoUniqueAd

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1254 + // space or zero-extent array. + if (DefaultsToAIXPowerAlignment && !getDataSize().isZero()) { +PreferredBaseAlign = BaseAlign; This needs to check `Handl

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1796 + bool FoundFirstNonOverlappingEmptyFieldToHandle = + DefaultsToAIXPowerAlignment && FieldOffset == CharUnits::Zero() && + !HandledFirstNonOverlappingEmptyField && !IsO

[PATCH] D82476: [Clang][Driver] Recognize the AIX OBJECT_MODE environment setting

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast marked an inline comment as done. hubert.reinterpretcast added a comment. LGTM with minor nit. Comment at: clang/lib/Driver/Driver.cpp:478 + // On AIX, the env OBJECT_MODE may affect the resulting arch varia

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1263 - // The maximum field alignment overrides base align. + assert(!IsUnion && "Unions cannot have base classes."); + // AIX `power` alignment does not apply the preferred align

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1208 +// "first (inherited) member". +HandledFirstNonOverlappingEmptyField = true; + We need some sort of `IsFirstNonEmptyBase` to record that the current base

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1246 + +// AIX `power` alignment does not apply the preferred alignment for +// non-union classes if the source of the alignment (the current base in Move the

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1244 +if (!Base->Class->isEmpty() && !HandledFirstNonOverlappingEmptyField) { + IsFirstNonEmptyBase = true; + // By handling a base class that is not empty, we're handlin

[PATCH] D83702: [AIX]Generate debug info for static init related functions

hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:248-250 CGF.StartFunction(GlobalDecl(&VD, DynamicInitKind::AtExit), -CGM.getContext().VoidTy, fn, FI, FunctionArgList()); +CGM.getContext().VoidTy,

[PATCH] D81972: [NFC] Cleanup of EmitCXXGlobalInitFunc() and EmitCXXGlobalDtorFunc()

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. LGTM; thanks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81972/new/ https://reviews.llvm.org/D81972 ___ cfe-commits mailing list cfe-commits@lists.llv

[PATCH] D82081: [z/OS] Add binary format goff and operating system zos to the triple

hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:258 return CGOpts.DataSections && !CGOpts.DisableIntegratedAS; + case Triple::GOFF: +llvm::report_fatal_error("ASan not implemented for GOFF."); Minor nit: GOFF

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast marked an inline comment as done. hubert.reinterpretcast added a comment. Thanks; LGTM with minor nit. Comment at: clang/lib/CodeGen/CodeGenModule.h:1060 + + /// Add a sterm finalizer to the C++ global clean

[PATCH] D82081: [z/OS] Add binary format goff and operating system zos to the triple

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. This revision is now accepted and ready to land. LGTM with minor nits. Comment at: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp:4732 + case Triple::GOFF: +llvm_unreachable("GOFF is not yet

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1887 + if (BTy->isFloatingPoint()) { +PreferredAlign = FieldSize; + } I tried `clang -cc1 -triple powerpc-ibm-aix -fsyntax-only` with : ``` struct A

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/test/Layout/aix-double-struct-member.cpp:1 +// RUN: %clang_cc1 -emit-llvm-only -triple powerpc-ibm-aix-xcoff \ +// RUN: -fdump-record-layouts -fsyntax-only %s 2>/dev/null | \ There's no testing f

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:2424 + (T->isSpecificBuiltinType(BuiltinType::LongDouble) && + Target->supportsAIXPowerAlignment())) // Don't increase the alignment if an alignment attribute was specified on

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/include/clang/AST/RecordLayout.h:75 + // can be different than Alignment in cases where it is beneficial for + // performance or backwards compatibility perserving (e.g. AIX-ABI). + CharUnits PreferredAlignment; -

[PATCH] D82677: [Clang] Handle AIX Include management in the driver

hubert.reinterpretcast added a comment. Where are the test cases? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82677/new/ https://reviews.llvm.org/D82677 ___ cfe-commits mailing list cfe-commits@lists

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/test/Layout/aix-double-struct-member.cpp:1 +// RUN: %clang_cc1 -emit-llvm-only -triple powerpc-ibm-aix-xcoff \ +// RUN: -fdump-record-layouts -fsyntax-only %s 2>/dev/null | \ I am concerned that

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

hubert.reinterpretcast added inline comments. Comment at: clang/test/Layout/aix-double-struct-member.cpp:2 +// RUN: %clang_cc1 -emit-llvm-only -triple powerpc-ibm-aix-xcoff \ +// RUN: -fdump-record-layouts -fsyntax-only %s 2>/dev/null | \ +// RUN: FileCheck %s -

[PATCH] D76932: [AIX] emit .extern and .weak directive linkage

hubert.reinterpretcast added inline comments. Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1496 + +MCSymbol *Name = getSymbol(&F); +if (TM.getTargetTriple().isOSBinFormatXCOFF() && !F.isIntrinsic()) { DiggerLin wrote: > jasonliu wrote: > > This

[PATCH] D76932: [AIX] emit .extern and .weak directive linkage

hubert.reinterpretcast added inline comments. Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:2144 report_fatal_error( -"Unhandled linkage when mapping linkage to StorageClass."); +"AvailableExternallyLinkage is for its first instance of linkage

[PATCH] D77168: Add a flag to debug automatic variable initialization

hubert.reinterpretcast added a comment. In D77168#1989973 , @jfb wrote: > I'd also like to see the pragma attribute approach, as well as byte-pattern > variability as I described. I don't think auto-narrowing is the only approach > we should push people

[PATCH] D76932: [AIX] emit .extern and .weak directive linkage

hubert.reinterpretcast added inline comments. Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1509 + +MCSymbol *Name = getSymbol(&F); +if (TM.getTargetTriple().isOSBinFormatXCOFF() && !F.isIntrinsic()) { Add a comment that this gives us the functio

[PATCH] D76932: [AIX] emit .extern and .weak directive linkage

hubert.reinterpretcast added inline comments. Comment at: llvm/test/CodeGen/PowerPC/aix-extern-weak.ll:9 +; RUN: -mattr=-altivec -filetype=obj -o %t.o < %s +; RUN: llvm-readobj --symbols %t.o | FileCheck --check-prefix=CHECKSYM %s + No need for two consecutive s

[PATCH] D76932: [AIX] emit .extern and .weak directive linkage

hubert.reinterpretcast added inline comments. Comment at: llvm/test/CodeGen/PowerPC/aix-extern-weak.ll:174 +; CHECKSYM-NEXT: Index: [[#Index+11]] +; CHECKSYM-NEXT: ContainingCsectSymbolIndex: 8 +; CHECKSYM-NEXT: ParameterHashIndex: 0x0 This shou

[PATCH] D76932: [AIX] emit .extern and .weak directive linkage

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. LGTM with minor comment. Comment at: llvm/test/CodeGen/PowerPC/aix-extern-weak.ll:8 +; RUN: llc -verify-machineinstrs -mtriple powerpc-ibm-aix-xcoff -mcpu=pwr4 \ +; RUN: -mattr=-altivec < %s

[PATCH] D78350: [AST] Build recovery expressions by default for C++.

hubert.reinterpretcast added a comment. In D78350#1988416 , @hokein wrote: > @ebevhan, @hubert.reinterpretcast, the patch is based on > fd7a34186137168064ffe2ca536823559b92d939 > , it

[PATCH] D78938: Fixing all comparisons for C++20 compilation.

hubert.reinterpretcast added a comment. In D78938#2006715 , @BRevzin wrote: > Wtf, where'd my other changes go? I've hit this before, use `arc diff --update D78938 `. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D79033: [NFC][clang] Replace raw new/delete with unique_ptr to store ABIInfo in TargetCodeGenInfo

hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.h:51 - TargetCodeGenInfo(ABIInfo *info = nullptr) : Info(info) {} - virtual ~TargetCodeGenInfo(); I'm not sure removing a virtual destructor is a good idea. The use of `del

[PATCH] D79044: [AIX] Avoid structor alias; die before bad alias codegen

hubert.reinterpretcast created this revision. hubert.reinterpretcast added reviewers: cebowleratibm, jasonliu, sfertile, daltenty, DiggerLin. Herald added subscribers: kbarton, hiraditya, nemanjai. Herald added projects: clang, LLVM. hubert.reinterpretcast edited the summary of this revision. Hera

[PATCH] D79033: [NFC][clang] Replace raw new/delete with unique_ptr to store ABIInfo in TargetCodeGenInfo

hubert.reinterpretcast added a comment. Funny that it's the front-end code that this patch makes more C++11 after so many years. LGTM; thanks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79033/new/ https://reviews.llvm.org/D79033 ___ cfe

[PATCH] D78350: [AST] Build recovery expressions by default for C++.

hubert.reinterpretcast added a comment. In D78350#2007065 , @hokein wrote: > In D78350#2006469 , > @hubert.reinterpretcast wrote: > > > Got it. I'll put together a build. > > > Thank you! Look forward to the result

[PATCH] D76932: [AIX] emit .extern and .weak directive linkage

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. LGTM with minor comment. Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:2143 + case GlobalValue::AvailableExternallyLinkage: +report_fatal_error("Unhandled AvailableExtern

[PATCH] D79290: Update suffix check and cast non-suffix types

hubert.reinterpretcast added a comment. In D79290#2016278 , @reikdas wrote: > In D79290#2016273 , > @hubert.reinterpretcast wrote: > > > @reikdas, please upload the full copy of your combined changes to D77598 > >

[PATCH] D79290: Update suffix check and cast non-suffix types

hubert.reinterpretcast added a comment. In D79290#2016608 , @v.g.vassilev wrote: > @hubert.reinterpretcast, this patch is part of our internal forks which we > would like to put upstream. The author of the previous patch does not have > bandwidth to wor

[PATCH] D85315: [AIX][Clang] Add C++ linker option to the driver toolchain

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Driver/ToolChains/AIX.cpp:152 + if (getToolChain().ShouldLinkCXXStdlib(Args)) +getToolChain().AddCXXStdlibLibArgs(Args, CmdArgs); This is commonly added before `-lc`, etc. To be consistent

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

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Basic/Targets/OSTargets.h:732 +Builder.defineMacro("_OPEN_DEFAULT"); +Builder.defineMacro("_UNIX03_WITHDRAWN"); +Builder.defineMacro("__370__"); This is not defined by z/OS XL C/C++.

[PATCH] D85315: [AIX][Clang][Driver] Generate reference to the C++ library on the link step

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Driver/ToolChains/AIX.cpp:210 + case ToolChain::CST_Libstdcxx: +llvm::report_fatal_error("linking libstdc++ unimplemented on AIX"); + } Should there be a test for the error message? =

[PATCH] D82081: [z/OS] Add binary format goff and operating system zos to the triple

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. I confirm that I have reviewed the Clang part of this change, which is entirely consistent with the status quo for XCOFF. I'm not seeing any outstanding comments, and I do not believe that the changes are cont

[PATCH] D85315: [AIX][Clang][Driver] Generate reference to the C++ library on the link step

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. This revision is now accepted and ready to land. LGTM with minor edits. Comment at: clang/lib/Driver/ToolChains/AIX.cpp:213 + + llvm_unreachable("unexpected c++ library type, only libc++ is

[PATCH] D85722: [SystemZ][z/OS] enable trigraphs by default on z/OS

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2787-2789 // Mimicking gcc's behavior, trigraphs are only enabled if -trigraphs - // is specified, or -std is set to a conforming mode. + // is specified, -std is set to a con

[PATCH] D85722: [SystemZ][z/OS] enable trigraphs by default on z/OS

hubert.reinterpretcast added inline comments. Comment at: clang/test/Lexer/cxx1z-trigraphs.cpp:24 +// notri-warning@6 {{trigraph converted}} tri-warning@6 {{trigraph converted}} +// notri-warning@12 {{backslash and newline separated by space}} tri-warning@12 {{backslash and newl

[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:730 +MacroBuilder &Builder) const override { +Builder.defineMacro("_LONG_LONG"); +Builder.defineMacro("_OPEN_DEFAULT"); The comment from http

[PATCH] D85722: [SystemZ][z/OS] enable trigraphs by default on z/OS

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. LGTM with suggested changes. Comment at: clang/test/Lexer/cxx1z-trigraphs.cpp:19 +// Note, there is intentionally trailing whitespace one lines below. // ??/ +error here; In

[PATCH] D85722: [SystemZ][z/OS] enable trigraphs by default on z/OS

This revision was automatically updated to reflect the committed changes. Closed by commit rGdedaf78fa714: [SystemZ][z/OS] enable trigraphs by default on z/OS (authored by abhina.sreeskantharajan, committed by hubert.reinterpretcast). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D85938: [OpenMP][NFC] Update check lines after D85099

hubert.reinterpretcast added a comment. In D85938#2216905 , @lebedev.ri wrote: > Thanks. Seems to fix the test for me FWIW, but > > 1. why wasn't this caught by anyone and all of the bots? For what it's worth, I have been affected by this and was drafting

[PATCH] D79044: [AIX] Avoid structor alias; die before bad alias codegen

This revision was automatically updated to reflect the committed changes. Closed by commit rGb116ded57da3: [AIX] Avoid structor alias; die before bad alias codegen (authored by hubert.reinterpretcast). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79

[PATCH] D79693: [test][ARM][CMSE] Use -ffreestanding for arm_cmse.h tests

hubert.reinterpretcast created this revision. hubert.reinterpretcast added reviewers: chill, dmgreen. Herald added subscribers: danielkiss, kristof.beyls. Herald added a project: clang. The `arm_cmse.h` header includes standard headers, but some tests that include this header explicitly specify a

[PATCH] D79694: [tests][Driver] Set `--sysroot=""` to allow `DEFAULT_SYSROOT` build

hubert.reinterpretcast created this revision. hubert.reinterpretcast added reviewers: broadwaylamb, sepavloff, jyknight, mstorsjo. Herald added a project: clang. If `DEFAULT_SYSROOT` is configured to some path, some tests would fail. This patch overrides `sysroot` to be the empty string in the s

[PATCH] D79693: [test][ARM][CMSE] Use -ffreestanding for arm_cmse.h tests

hubert.reinterpretcast added a comment. Ping, @chill. It seems you checked these files in under D70817 . The way they were written has issues as indicated by the patch description. Please take a look; thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D79693: [test][ARM][CMSE] Use -ffreestanding for arm_cmse.h tests

hubert.reinterpretcast added a comment. In D79693#2036599 , @chill wrote: > How do you trigger a problem? If your build was configured with `DEFAULT_SYSROOT` to a toolchain path that has a `/include` directory, then the contents of that directory would

[PATCH] D79693: [test][ARM][CMSE] Use clang_cc1 in arm_cmse.h tests

hubert.reinterpretcast added a comment. In D79693#2036710 , @chill wrote: > I can also count (`grep -rn '#include.* standard headers. If they don't create a problem, wouldn't it be better to > rewrite this test to use `%clang_cc1` ? @chill, thanks for t

[PATCH] D79693: [test][ARM][CMSE] Use clang_cc1 in arm_cmse.h tests

hubert.reinterpretcast updated this revision to Diff 264149. hubert.reinterpretcast added a comment. - Use clang_cc1 in arm_cmse.h tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79693/new/ https://reviews.llvm.org/D79693 Files: clang/test/C

[PATCH] D79693: [test][ARM][CMSE] Use clang_cc1 in arm_cmse.h tests

This revision was automatically updated to reflect the committed changes. Closed by commit rG3f5fc73a9d52: [test][ARM][CMSE] Use clang_cc1 in arm_cmse.h tests (authored by hubert.reinterpretcast). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79693/n

[PATCH] D79694: [tests][Driver] Set `--sysroot=""` to allow `DEFAULT_SYSROOT` build

This revision was automatically updated to reflect the committed changes. Closed by commit rG15f0f824b36e: [tests][Driver] Set `--sysroot=""` to allow `DEFAULT_SYSROOT` build (authored by hubert.reinterpretcast). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

hubert.reinterpretcast added inline comments. Comment at: clang/test/CodeGen/static-init.cpp:14 +// CHECK: @llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sterm8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd, i

[PATCH] D40381: Parse concept definition

hubert.reinterpretcast added inline comments. Comment at: test/CXX/concepts-ts/expr/expr.prim/expr.prim.id/p3.cpp:1 +// RUN: %clang_cc1 -std=c++1z -fconcepts-ts -fcxx-exceptions -x c++ -verify %s +// expected-no-diagnostics saar.raz wrote: > Rakete wrote: >

[PATCH] D40381: Parse concept definition

hubert.reinterpretcast added inline comments. Comment at: include/clang/Sema/Sema.h:6194 +SourceLocation TemplateLoc, +const TemplateArgumentListInfo *TemplateArgs); + Indentation issue here too. =

[PATCH] D40381: Parse concept definition

hubert.reinterpretcast added inline comments. Comment at: include/clang/Sema/Sema.h:6194 +SourceLocation TemplateLoc, +const TemplateArgumentListInfo *TemplateArgs); + changyu wrote: > hubert.reinter

[PATCH] D40673: Add _Float128 as alias to __float128 to enable compilations on Fedora27/glibc2-26

hubert.reinterpretcast added inline comments. Comment at: test/Sema/_Float128.cpp:1 +// RUN: %clang_cc1 -verify -std=gnu++11 %s +// RUN: %clang_cc1 -verify -std=c++11 %s GCC documents that it does not support `_Float128` in C++ mode, and I think their decision m

[PATCH] D40673: Add _Float128 as alias to __float128 to enable compilations on Fedora27/glibc2-26

hubert.reinterpretcast added inline comments. Comment at: lib/Frontend/InitPreprocessor.cpp:817 DefineFloatMacros(Builder, "LDBL", &TI.getLongDoubleFormat(), "L"); + DefineFloatMacros(Builder, "FLT128", &TI.getFloat128Format(), "Q"); + GCC //does// define the

[PATCH] D40673: Add _Float128 as alias to __float128 to enable compilations on Fedora27/glibc2-26

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. Looks good to me. Repository: rL LLVM https://reviews.llvm.org/D40673 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mai

[PATCH] D40380: Remove old concepts parsing code

This revision was automatically updated to reflect the committed changes. Closed by commit rC319992: Remove old concepts parsing code (authored by hubert.reinterpretcast). Changed prior to commit: https://reviews.llvm.org/D40380?vs=124032&id=125844#toc Repository: rC Clang https://reviews.l

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

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

[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`). + ve

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

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

[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 > > https://lab.llvm.org/buildb

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

[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, + isa(Dcl),

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

[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 ___ c

[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 cfe-commits@lists.llvm.org

[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 &W) { + return W.getAlias() ? DenseMapInfo::getHashValue(W.getAlias()->getName()) + : DenseMapInfo::getHashV

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

[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&id=418092#toc Repositor

[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: clang/lib/Sema/Sema

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

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

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

[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] 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#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 `// expected-no-diagnostic

<    1   2   3   4   5   6   7   8   >