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

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

[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

[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

[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 , +llvm::opt::ArgStringList , +Action::OffloadKind

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

[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

[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

[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

[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

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

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

[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 ) const override { +// FIXME: LONG_LONG should not be defined under -std=c89 +Builder.defineMacro("_LONG_LONG"); Minor

[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

[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

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

[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

[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

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

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

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

hubert.reinterpretcast added a comment. I've done another pass over the code (but did not get through the tests). I have no comments about the code at this time. My understanding is that @jasonliu will be doing another pass over this patch, so he can approve while I'm away on vacation.

[PATCH] D83974: [AIX] report_fatal_error on `-fregister_global_dtors_with_atexit` for static init

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

[PATCH] D83974: [AIX] report_fatal_error on `-fregister_global_dtors_with_atexit` for static init

hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1212 if (CodeGenOpts.RegisterGlobalDtorsWithAtExit) { +if (getContext().getTargetInfo().getTriple().isOSAIX()) + llvm::report_fatal_error( This should query

[PATCH] D83915: [PowerPC] Remove QPX/A2Q BGQ/BGP support

hubert.reinterpretcast added a comment. Should we indicate planned removal in the Release Notes for version 11 and actual removal in those for version 12? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83915/new/ https://reviews.llvm.org/D83915

[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(, DynamicInitKind::AtExit), -CGM.getContext().VoidTy, fn, FI, FunctionArgList()); +CGM.getContext().VoidTy,

[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

[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:1208 +// "first (inherited) member". +HandledFirstNonOverlappingEmptyField = true; + We need some sort of `IsFirstNonEmptyBase` to record that the current

[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

[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

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

[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

[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

[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

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

[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

[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:666 + /// HasNonEmptyBaseClass - Whether the class has any non-empty class (in the + /// sense of (C++11 [meta.unary.prop])) as base. + bool HasNonEmptyBaseClass;

[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

[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/RecordLayoutBuilder.cpp:1908 // The align if the field is not packed. This is to check if the attribute // was unnecessary (-Wpacked). CharUnits UnpackedFieldAlign = FieldAlign;

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

[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/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:1884 + if (const BuiltinType *BTy = + Context.getBaseElementType(CTy->getElementType()) + ->getAs()) I don't think there's a reason

[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:1778 +if (FoundNonOverlappingEmptyField) + HandledFirstNonOverlappingEmptyField = true; + See my other comment for rationale on why

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

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. Aside from one comment; LGTM. Thanks. Comment at: clang/test/Driver/aix-toolchain-include.cpp:4 +// Check powerpc-ibm-aix, 32-bit/64-bit. +// RUN: %clangxx -### -no-canonical-prefixes %s

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

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1871 + // AIX ABI has this special rule that in aggregates, the first member of + // floating point data type(or aggregate type contains floating point data

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

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

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Driver/ToolChains/AIX.cpp:179 +ArgStringList ) const { + // return if -nostdinc/-nostdlibinc is specified as a driver option. + if (DriverArgs.hasArg(options::OPT_nostdinc)

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

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Driver/ToolChains/AIX.cpp:172 +return; + + const Driver = getDriver(); daltenty wrote: > hubert.reinterpretcast wrote: > > Please investigate the handling of `OPT_nostdlibinc`. > If we are

[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] D82677: [Clang] Handle AIX Include management in the driver

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Driver/ToolChains/AIX.cpp:179 + } + addSystemInclude(DriverArgs, CC1Args, "/usr/include"); +} This does not observe `--sysroot`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

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

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Driver/ToolChains/AIX.cpp:172 +return; + + const Driver = getDriver(); Please investigate the handling of `OPT_nostdlibinc`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

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

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:2416 - // Double and long long should be naturally aligned if possible. + // Double, long double (only when the target supports AIX power alignment) and + // long long should be

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

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

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

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

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

hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:600 + // with priority emitted above. + SmallString<128> FileName; llvm::Function *Fn = CreateGlobalInitOrDestructFunction( In the less complex context of this patch,

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

hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4489 + // DestructCallBlock, otherwise jump to EndBlock directly. + CGF.EmitCXXGuardedInitBranch(NeedsDestruct, DestructCallBlock, EndBlock, +

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

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:660 + /// When there are OverlappingEmptyFields existing in the aggregate, the + /// flag shows if the following first non-overlappingEmptyField has been + /// handled, if any.

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

hubert.reinterpretcast added inline comments. Comment at: clang/include/clang/Basic/TargetInfo.h:1389 + /// Whether target supports the special power alignment rules of AIX. + virtual bool supportsAIXPowerAlignment() const { return false; } Minor nit: Use

[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:31 +// CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sinit8000_clang_510e898aa8d263cac999dd03eeed5b51,

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

hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4482 + llvm::Value *NeedsDestruct = + CGF.Builder.CreateIsNull(V, "needsDestruct"); + There are uses of `CreateIsNull` with `snake_case`; this is the only

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

hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:277 + llvm::FunctionType::get(CGM.VoidTy, false) && + "atexit has wrong parameter type."); + s/atexit has wrong parameter type/Argument to atexit has a

[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:8 +// RUN: FileCheck %s struct test { Xiangling_L wrote: > jasonliu wrote: > > Looks like the non-inline comments are easier to get ignored and missed, so > > I

[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:31 +// CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sinit8000_clang_510e898aa8d263cac999dd03eeed5b51,

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

hubert.reinterpretcast marked an inline comment as done. hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:700 + +Fn = CreateGlobalInitOrDestructFunction( +FTy, llvm::Twine("__sterm8000_clang_") + GlobalUniqueModuleId, FI,

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

hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:807 void CodeGenFunction::GenerateCXXGlobalDtorsFunc( llvm::Function *Fn, This function is to be renamed. Comment at:

[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:12 ~test(); -} t; +} t1, t2; I suggest adding also one each of the following: - a dynamic initialization of a non-local variable of type `int` - a `constinit`

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

hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:682 void CodeGenModule::EmitCXXGlobalDtorFunc() { if (CXXGlobalDtors.empty()) return; hubert.reinterpretcast wrote: > Following from my previous comments on

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

hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:824 std::tie(CalleeTy, Callee, Arg) = DtorsAndObjects[e - i - 1]; - llvm::CallInst *CI = Builder.CreateCall(CalleeTy, Callee, Arg); + llvm::CallInst *CI = (Arg == nullptr)

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

hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/ItaniumMangle.cpp:5231 + CXXNameMangler Mangler(*this, Out); + Mangler.getStream() << "__cxx_global_var_destruct_"; + if (shouldMangleDeclName(D)) I believe these are actually paired with

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

hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:293 +CodeGenFunction::unregisterGlobalDtorWithUnAtExit(llvm::Function *dtorStub) { + // extern "C" int unatexit(void (*f)(void)); + assert(dtorStub->getFunctionType() ==

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

hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:305 + if (llvm::Function *unatexitFn = + dyn_cast(unatexit.getCallee())) +unatexitFn->setDoesNotThrow(); Xiangling_L wrote: > jasonliu wrote: > > Is there a

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

hubert.reinterpretcast added inline comments. Comment at: clang/include/clang/AST/Mangle.h:178 + virtual void mangleDynamicDestructor(const VarDecl *D, raw_ostream ) = 0; + I am not sure "destructor" is the right term here. This seems to be an analogue to

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

hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:562 +static std::string getTransformedFileName(llvm::Module ) { + SmallString<128> FileName = llvm::sys::path::filename(M.getName()); Consider having the

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

hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4447 + + // Create __dtor function for the var decl. + llvm::Function *dtorStub = CGF.createAtExitStub(D, dtor, addr); We should probably report_fatal_error if

[PATCH] D81115: [PowerPC] Do not special case Darwin on PowerPC in target cpu handling

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

[PATCH] D80947: Add to the Coding Standard our that single-line bodies omit braces

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. This revision is now accepted and ready to land. This LGTM. I believe we have not heard back from @arsenm on the response to some of their comments though. Comment at:

[PATCH] D80947: Add to the Coding Standard our that single-line bodies omit braces

hubert.reinterpretcast added inline comments. Comment at: llvm/docs/CodingStandards.rst:1576 + +When writing the body of an `if`, `else`, or loop statement, omit the braces to avoid +unnecessary and otherwise meaningless code. However, braces should be used

[PATCH] D81115: [PowerPC] Do not check for non-Darwin in PowerPC target cpu handling

hubert.reinterpretcast added a comment. Calling it the "Darwin factor" gives it a certain je ne sais quoi. :) Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:298 +// each architecture. (except on AIX) if (TargetCPUName.empty()) { if (T.isOSAIX())

[PATCH] D80835: [AIX] Change the default target CPU to power4 for AIX on Power

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. This revision is now accepted and ready to land. LGTM; thanks. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:301 +TargetCPUName = "pwr4"; + else if (!T.isOSDarwin()) {

[PATCH] D80300: [Driver] Add DEFAULT_DYLD_PREFIX and DEFAULT_RPATH to complement DEFAULT_SYSROOT

hubert.reinterpretcast marked an inline comment as done. hubert.reinterpretcast added a comment. I believe I have responded to @joerg's comments with rationale to support the current direction. I intend to commit this patch within a week or two. Repository: rG LLVM Github Monorepo CHANGES

<    1   2   3   4   5   6   7   8   >