[PATCH] D150221: Add option -fkeep-static-variables to emit all static variables

cebowleratibm added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2207 +addUsedOrCompilerUsedGlobal(GV); + else if (CodeGenOpts.KeepStaticConsts && VD->getType().isConstQualified()) +addUsedOrCompilerUsedGlobal(GV); why

[PATCH] D136554: Implement CWG2631

cebowleratibm added a comment. In D136554#4056862 , @cor3ntin wrote: > Yes please! However the warning looks correct to me in that case. A > constructs x which constructs A etc. https://github.com/llvm/llvm-project/issues/60082 Please set the

[PATCH] D136554: Implement CWG2631

cebowleratibm added a comment. I've reduced a regression on: commit ca619613801233ef2def8c3cc7d311d5ed0033cb (HEAD, refs/bisect/bad) Author: Corentin Jabot Date: Sun Oct 23 17:32:58 2022 +0200 template int f(T) {

[PATCH] D135384: [AIX] Fix mcount name and call arguments

cebowleratibm closed this revision. cebowleratibm added a comment. I had a typo in the commit so this didn't auto close. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135384/new/ https://reviews.llvm.org/D135384

[PATCH] D137375: [AIX][pg] Add Correct Search Paths for Profiled Libraries

cebowleratibm accepted this revision. cebowleratibm added a comment. This revision is now accepted and ready to land. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137375/new/ https://reviews.llvm.org/D137375

[PATCH] D137375: [AIX][pg] Add Correct Search Paths for Profiled Libraries

cebowleratibm added a comment. Looks good but I just want to confirm that the profiled -L paths are appended in the correct order with respect to the non-profiled paths. Comment at: clang/test/Driver/aix-ld.c:192 // CHECK-LD32-GPROF: "-lc" +// CHECK-LD32-GPROF:

[PATCH] D128223: [clang] Cached linkage assertion for static locals of static function

cebowleratibm added a comment. I believe the fix for a52d151f9dde7 inadvertently exposed a code path where by the linkage of a static local of a static function, which would otherwise return LinkageInfo::none() may now return VisibleNoLinkage depending on the incoming computation argument. I

[PATCH] D128223: [clang] Cached linkage assertion for static locals of static function

cebowleratibm updated this revision to Diff 473756. cebowleratibm retitled this revision from "[clang] Linkage computation of static locals may require forcing visibility computation" to "[clang] Cached linkage assertion for static locals of static function". cebowleratibm edited the summary of

[PATCH] D137373: [Test][[AIX][p] Add 64-bit linker invocation tests

cebowleratibm accepted this revision. cebowleratibm added a comment. This revision is now accepted and ready to land. LGTM. Test only patch is safe to commit. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137373/new/

[PATCH] D137372: [Test][[AIX][pg] Add 32-bit linker invocation tests

cebowleratibm accepted this revision. cebowleratibm added a comment. This revision is now accepted and ready to land. LGTM. Test only patch is safe to commit. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137372/new/

[PATCH] D137373: [AIX][p] Add 64-bit Test Case to Linker Tests

cebowleratibm added a comment. Suggest: [Test][AIX][p] Add 64-bit driver expected output No summary required. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137373/new/ https://reviews.llvm.org/D137373

[PATCH] D137372: [AIX][pg] Add 32-bit Test Case to Linker Tests

cebowleratibm added a comment. Use the [Test] tag in the abstract. Suggest: "[Test][AIX][pg] Add 32-bit expected driver output" No need for a summary. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137372/new/ https://reviews.llvm.org/D137372

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

cebowleratibm added inline comments. Comment at: llvm/lib/Transforms/Utils/EntryExitInstrumenter.cpp:38 +const std::string = M.getTargetTriple(); +if (targetTriple.find("powerpc-ibm-aix") == std::string::npos || +targetTriple.find("powerpc64-ibm-aix") ==

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

cebowleratibm added inline comments. Comment at: clang/lib/Basic/Targets/OSTargets.h:754 : OSTargetInfo(Triple, Opts) { +this->MCountName = "__mcount.aix"; this->TheCXXABI.set(TargetCXXABI::XL); Suggest sticking with the normal mcount name and

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

cebowleratibm added inline comments. Comment at: clang/test/CodeGen/mcount-aix.c:2 +// RUN: %clang_cc1 -pg -no-opaque-pointers -triple powerpc-ibm-aix7.2.0.0 -S -emit-llvm %s -o - | FileCheck %s +// RUN: %clang_cc1 -pg -no-opaque-pointers -triple powerpc64-ibm-aix7.2.0.0 -S

[PATCH] D128223: [clang] Linkage of static locals may require inspecting visibility

cebowleratibm created this revision. cebowleratibm added a reviewer: daltenty. Herald added a project: All. cebowleratibm requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. An assertion failure was encountered after

[PATCH] D122519: [AIX] XFAIL test for lack of visibility support

cebowleratibm accepted this revision. cebowleratibm added a comment. This revision is now accepted and ready to land. The AIX default of ignoring source-level visibility is problematic. I agree the test should be xfailed for AIX. You can put [NFC] and [tests] to the commit. Should be safe to

[PATCH] D122497: [clang][NFC] Add filescope array initialization test

cebowleratibm created this revision. cebowleratibm added a reviewer: AaronLiu. cebowleratibm added a project: clang. Herald added a project: All. cebowleratibm requested review of this revision. Herald added a subscriber: cfe-commits. This test is based on a customer-reported failure in an XL

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

cebowleratibm accepted this revision. cebowleratibm added a comment. This revision is now accepted and ready to land. Makes sense to me. Comment at: clang/include/clang/Sema/Weak.h:35 inline SourceLocation getLocation() const { return loc; } - void setUsed(bool Used=true)

[PATCH] D108917: [AIX][PowerPC] Define __powerpc and __PPC macros

cebowleratibm accepted this revision. cebowleratibm added a comment. This revision is now accepted and ready to land. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108917/new/ https://reviews.llvm.org/D108917

[PATCH] D108917: [AIX][PowerPC] Define __powerpc and __PPC macros

cebowleratibm requested changes to this revision. cebowleratibm added inline comments. This revision now requires changes to proceed. Comment at: clang/test/Preprocessor/init-ppc.c:520 // PPC-AIX:#define __POWERPC__ 1 +// PPC-AIX:#define __PPC 1 // PPC-AIX:#define __PPC__ 1

[PATCH] D108917: [PowerPC] Define __powerpc and __PPC macros

cebowleratibm added a comment. The XL compiler only defined this weird form of the macros on AIX, but not on the XL Linux on Power compiler. I think it's preferable that we define these macros only on AIX and in the source/commit messages indicate that we're only doing so for AIX XL C/C++

[PATCH] D108917: [AIX] Define __powerpc and __PPC macros

cebowleratibm requested changes to this revision. cebowleratibm added a comment. This revision now requires changes to proceed. I'd like to see the rationale for adding these forms of the macros in the review and also in the extended commit message. These forms are being added primarily

[PATCH] D107242: [AIX] Define __HOS_AIX__ macro

cebowleratibm added a comment. > Given that we have a legacy XL macro with no legacy cross compiler I think > it's fine if we set this according to the target only. It's fully redundant > to `_AIX ` but we'll define it for any working use-case with the current xlC > compiler. If IBM has a

[PATCH] D107242: [AIX] Define __HOS_AIX__ macro

cebowleratibm added a comment. In D107242#2934611 , @joerg wrote: > clang is fundamentally a cross-compiler only. I don't see any point for > having host-specific branches in this case at all. Either the macro should be > specified for the target all

[PATCH] D107242: [AIX] Define __HOS_AIX__ macro

cebowleratibm added a comment. In D107242#2932289 , @joerg wrote: > I'm puzzled by this change. I don't think we have any case so far where the > compiler behavior changes with the host OS and I don't think it should. > What's the point / use case of

[PATCH] D107506: [PowerPC][AIX] Warn when using pragma align(packed) on AIX.

cebowleratibm accepted this revision. cebowleratibm added a comment. The new version is better than the previous. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107506/new/ https://reviews.llvm.org/D107506

[PATCH] D107242: [AIX] Define __HOS_AIX__ macro

cebowleratibm accepted this revision. cebowleratibm added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107242/new/ https://reviews.llvm.org/D107242

[PATCH] D107244: [AIX] Define _ARCH_PPC64 macro for 32-bit

cebowleratibm accepted this revision. cebowleratibm added a comment. This revision is now accepted and ready to land. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107244/new/ https://reviews.llvm.org/D107244

[PATCH] D107244: [AIX] Define _ARCH_PPC64 macro for 32-bit

cebowleratibm requested changes to this revision. cebowleratibm added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Basic/Targets/PPC.cpp:260 + } else if (getTriple().isOSAIX()) { +// Also define _ARCH_PPC64 for 32-bit on AIX. +

[PATCH] D107242: [AIX] Define __HOS_AIX__ macro

cebowleratibm added inline comments. Comment at: clang/lib/Basic/Targets/PPC.cpp:306 + llvm::Triple HostTriple(llvm::sys::getProcessTriple()); + if (HostTriple.isOSAIX() && getTriple().isOSAIX()) { +Builder.defineMacro("__HOS_AIX__"); suggest using a temp

[PATCH] D107242: [AIX] Define __HOS_AIX__ macro

cebowleratibm added inline comments. Comment at: clang/test/Preprocessor/not-host-aix.c:2 +// UNSUPPORTED: system-aix +// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-unknown-linux-gnu -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPC-LINUX

[PATCH] D107506: [PowerPC][AIX] Warn when using pragma align(packed) on AIX.

cebowleratibm added inline comments. Comment at: clang/test/Sema/aix-pragma-align-packed-warn.c:13 + short a : 8; // expected-warning {{#pragma align(packed) may not be compatible with objects generated with AIX XL C/C++}} + short b : 8; // expected-warning {{#pragma

[PATCH] D107506: [PowerPC][AIX] Warn when using pragma align(packed) on AIX.

cebowleratibm accepted this revision. cebowleratibm added a comment. This revision is now accepted and ready to land. It would be nice if the diagnostic could be deferred to layout, in case the struct is defined but not used in a header, but I understand that #pragma pack(1) and #pragma

[PATCH] D107506: [PowerPC][AIX] Warn when using pragma align(packed) on AIX.

cebowleratibm added inline comments. Comment at: clang/lib/Sema/SemaAttr.cpp:238 +if (this->Context.getTargetInfo().getTriple().isOSAIX()) + Diag(PragmaLoc, diag::warn_pragma_align_not_xl_compatible); Action = Sema::PSK_Push_Set; Does this

[PATCH] D107242: [AIX] Define __HOS_AIX__ macro

cebowleratibm requested changes to this revision. cebowleratibm added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Basic/Targets/OSTargets.h:679 Builder.defineMacro("__TOS_AIX__"); +Builder.defineMacro("__HOS_AIX__");

[PATCH] D107241: [AIX] Define __THW_BIG_ENDIAN__ macro

cebowleratibm accepted this revision. cebowleratibm added a comment. This revision is now accepted and ready to land. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107241/new/ https://reviews.llvm.org/D107241

[PATCH] D107243: [AIX] Define __THW_PPC__ macro

cebowleratibm accepted this revision. cebowleratibm added a comment. This revision is now accepted and ready to land. LGTM. __THW_PPC__ is a macro historically defined by xlc on AIX and defining it may help users port to clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D107244: [AIX] Define _ARCH_PPC64 macro for 32-bit

cebowleratibm requested changes to this revision. cebowleratibm added a comment. This revision now requires changes to proceed. The divergence with GCC is regrettable. Unfortunately defining _ARCH_PPC64 in 32-bit has been a long-standing discrepancy between xlc and gcc and one I don't expect

[PATCH] D105477: [AIX] Define __LONGDOUBLE64 macro

cebowleratibm added a comment. The decision was made to make -mlong-double-128 an error on AIX: https://reviews.llvm.org/D106074 [AIX] Clang's library integration support for 128-bit long double is incomplete on AIX. So I think your patch is ready to land. LGTM. Repository: rG LLVM Github

[PATCH] D106074: [AIX] Clang's library integration support for 128-bit long double is incomplete on AIX.

cebowleratibm accepted this revision. cebowleratibm added a comment. This revision is now accepted and ready to land. LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106074/new/ https://reviews.llvm.org/D106074 ___ cfe-commits mailing

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

cebowleratibm requested changes to this revision. cebowleratibm added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4845 + if (Arg *A = Args.getLastArg(options::OPT_mlong_double_128)) { +// AIX doesn't

[PATCH] D105477: [AIX] Define __LONGDOUBLE64 macro

cebowleratibm accepted this revision. cebowleratibm added a comment. This revision is now accepted and ready to land. The change made me think we should add the negative test with -mlong-double-128, but then I thought we should probably make that an error until we've sorted through the AIX

[PATCH] D105477: [AIX] Define __LONGDOUBLE64 macro

cebowleratibm requested changes to this revision. cebowleratibm added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Basic/Targets/PPC.cpp:175 + if (getTriple().isOSAIX() && LongDoubleWidth == 64) { +

[PATCH] D103587: [AIX] Define __TOS_AIX__ predefined macro

cebowleratibm accepted this revision. cebowleratibm added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103587/new/ https://reviews.llvm.org/D103587

[PATCH] D103707: [AIX] Define __STDC_NO_ATOMICS__ and __STDC_NO_THREADS__ predefined macros

cebowleratibm accepted this revision. cebowleratibm added a comment. This revision is now accepted and ready to land. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103707/new/ https://reviews.llvm.org/D103707

[PATCH] D103707: [AIX] Define __STDC_NO_ATOMICS__ and __STDC_NO_THREADS__ predefined macros

cebowleratibm requested changes to this revision. cebowleratibm added a comment. This revision now requires changes to proceed. Would like you to prune the new tests, then I think this will be ready to land. Comment at: clang/test/Preprocessor/init-ppc.c:726 +// RUN:

[PATCH] D103587: [AIX] Transfer predefined macros

cebowleratibm requested changes to this revision. cebowleratibm added a comment. This revision now requires changes to proceed. I think it makes sense to split the __STDC macro changes and __TOS_AIX__ into different patches. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D103587: [AIX] Transfer predefined macros

cebowleratibm added inline comments. Comment at: clang/lib/Basic/Targets/OSTargets.h:680 + +if (Opts.LangStd == LangStandard::lang_c11 || +Opts.LangStd == LangStandard::lang_gnu11 || I think LangStandard::isC11() does what you want. You wouldn't

[PATCH] D103587: [AIX] Transfer predefined macros

cebowleratibm requested changes to this revision. cebowleratibm added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Basic/Targets/OSTargets.h:680 + +if (Opts.LangStd == LangStandard::lang_c11 || +Opts.LangStd ==

[PATCH] D103587: [AIX] Transfer predefined macros

cebowleratibm requested changes to this revision. cebowleratibm added a comment. This revision now requires changes to proceed. Please rework the test update into init-ppc.c Comment at: clang/lib/Basic/Targets/PPC.cpp:95 Builder.defineMacro("_ARCH_PPC"); +

[PATCH] D101130: [PowerPC] Provide XL-compatible builtins in altivec.h

cebowleratibm accepted this revision. cebowleratibm added inline comments. This revision is now accepted and ready to land. Comment at: clang/test/CodeGen/builtins-ppc-xlcompat.c:2 +// REQUIRES: powerpc-registered-target +// RUN: %clang_cc1 -target-feature +altivec

[PATCH] D94986: Remove requirement for -maltivec to be used when using -mabi=vec-extabi or -mabi=vec-default when not using vector code

cebowleratibm added inline comments. Comment at: clang/test/CodeGen/altivec.c:1 // RUN: %clang_cc1 -target-feature +altivec -triple powerpc-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,CHECK-BE // RUN: %clang_cc1 -target-feature +altivec -triple

[PATCH] D94986: Remove requirement for -maltivec to be used when using -mabi=vec-extabi or -mabi=vec-default when not using vector code

cebowleratibm accepted this revision. cebowleratibm added a comment. This revision is now accepted and ready to land. Minor suggestion to keep and remap some of the diagnostic cases. Comment at: clang/test/CodeGen/altivec.c:13 // RUN: not %clang -S -emit-llvm -maltivec

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

cebowleratibm updated this revision to Diff 293917. cebowleratibm marked 2 inline comments as done. cebowleratibm added a comment. Reworked the prefixes and indentation. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88105/new/ https://reviews.llvm.org/D88105 Files:

[PATCH] D88130: [PPC] [AIX] Implement calling convention IR for C99 complex types on AIX

cebowleratibm created this revision. cebowleratibm added reviewers: ZarkoCA, daltenty, sfertile. Herald added subscribers: cfe-commits, nemanjai. Herald added a project: clang. cebowleratibm requested review of this revision. Builds on D88105 to add AIX calling

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

cebowleratibm updated this revision to Diff 293625. cebowleratibm marked an inline comment as done. cebowleratibm added a comment. Added ppc64le target. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88105/new/ https://reviews.llvm.org/D88105 Files:

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

cebowleratibm created this revision. cebowleratibm added reviewers: daltenty, ZarkoCA, sfertile. Herald added subscribers: cfe-commits, shchenz, nemanjai. Herald added a project: clang. cebowleratibm requested review of this revision. Adding this test so that I can extend it in a follow on patch

[PATCH] D74015: [AIX][Frontend] C++ ABI customizations for AIX boilerplate

cebowleratibm added a comment. In D74015#1882933 , @sfertile wrote: > Patch LGTM as far a formatting/naming/testing etc. C++ specifics is outside > my wheelhouse though, so I can't confirm things like the tail padding rules > are correct for AIX.

[PATCH] D74015: [AIX][Frontend] C++ ABI customizations for AIX boilerplate

cebowleratibm added a comment. From my perspective, the only issue holding this up is settling on the name. I'd like to hammer that out and get this committed. Comment at: clang/include/clang/Basic/TargetCXXABI.h:116 +/// - static initialization is adjusted to use

[PATCH] D74015: [AIX][Frontend] C++ ABI customizations for AIX boilerplate

cebowleratibm added a comment. Looks fine, but we need to settle on the name for the ABI. My preference would be "XLC++11", or perhaps "XLCXX11" (I propose the latter because of the common reference CXXABI.) Comment at: clang/include/clang/Basic/TargetCXXABI.h:116 +///