[PATCH] D28620: Guard __gnuc_va_list typedef

sfertile added a comment. I think the fix/test looks good, but someone with more experience than me should review it before accepting it https://reviews.llvm.org/D28620 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D30415: Fix -mno-altivec cannot overwrite -maltivec option

sfertile added inline comments. Comment at: lib/Driver/Tools.cpp:5436 + Args.AddLastArg(CmdArgs, options::OPT_fzvector); } If -fno-zvector has the same problem we should fix it as well. https://reviews.llvm.org/D30415

[PATCH] D25263: [Driver] Allow setting the default linker during build

sfertile added inline comments. Comment at: lib/Driver/ToolChain.cpp:721-724 + +const char *ToolChain::getDefaultLinker() const { + return CLANG_DEFAULT_LINKER; +} Hahnfeld wrote: > I think this could go into the header The CLANG_DEFAULT_LINKER macro is getting

[PATCH] D26546: [PPC] Add vec_insert4b/vec_extract4b to altivec.h

sfertile updated this revision to Diff 79860. sfertile marked an inline comment as done. sfertile added a comment. Changed all variable names to start with capitals, added description strings to the assertions, changed uses of '12' to MaxIndex and initialized the arrays in their definition.

[PATCH] D26546: [PPC] Add vec_insert4b/vec_extract4b to altivec.h

sfertile updated this revision to Diff 82031. sfertile marked 6 inline comments as done. sfertile added a comment. Updated to swap the arguments when generating the intrinsic. Updated a number of the comments, and added some tests with the index out of range. Repository: rL LLVM

[PATCH] D30415: Fix -mno-altivec cannot overwrite -maltivec option

sfertile added a comment. Thanks Eric. Kuang, commit r298449 contained this fix so you can close the review now. https://reviews.llvm.org/D30415 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D30415: Fix -mno-altivec cannot overwrite -maltivec option

sfertile added a comment. > I have a patch to do this now. I'll plan on committing it in a bit. Is there a way to mark the -f form of the option as deprecated? We should warn and suggest users switch to the -maltivec option for a release to give people a chance to update their builds.

[PATCH] D38517: Enabling new pass manager in LTO (and thinLTO) link step via -fexperimental-new-pass-manager option

This revision was automatically updated to reflect the committed changes. Closed by commit rL314964: Enabling new pass manager in LTO (and thinLTO) link step. (authored by sfertile). Changed prior to commit: https://reviews.llvm.org/D38517?vs=117677=117771#toc Repository: rL LLVM

[PATCH] D39376: [PowerPC] Add implementation for -msave-toc-indirect option - clang portion

sfertile added inline comments. Comment at: include/clang/Driver/Options.td:1907 def mvsx : Flag<["-"], "mvsx">, Group; +def msave_toc_indirect : Flag<["-"], "msave-toc-indirect">, Group; def mno_vsx : Flag<["-"], "mno-vsx">, Group; hfinkel wrote: > You also

[PATCH] D41318: Start setting dso_local in clang

sfertile added a comment. Sorry, I missed that you wanted this reviewed again, I'll make sure to review it today. https://reviews.llvm.org/D41318 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D41318: Start setting dso_local in clang

sfertile accepted this revision. sfertile added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D41318 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D41318: Start setting dso_local in clang

sfertile added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:750 + // If we can use a plt entry as the symbol address we can assume it + // is local. + if (isa(D) && !CGOpts.NoPLT) I don't think this is the case. I think this would break

[PATCH] D49424: [PowerPC] Handle __builtin_xxpermdi the same way as GCC does

sfertile added inline comments. Comment at: lib/CodeGen/CGBuiltin.cpp:10781 // endian platforms (i.e. index is complemented and source vector reversed). -unsigned ElemIdx0; -unsigned ElemIdx1; -if (getTarget().isLittleEndian()) { - ElemIdx0 = (~Index & 1)

[PATCH] D58930: Add XCOFF triple object format type for AIX

sfertile added inline comments. Comment at: llvm/lib/MC/MCContext.cpp:165 +case MCObjectFileInfo::IsXCOFF: + // TODO: Need to implement class MCSymbolXCOFF. + break; jasonliu wrote: > JDevlieghere wrote: > > See previous comment. > It is certain

[PATCH] D59048: Add AIX Target Info

sfertile added inline comments. Comment at: clang/test/Preprocessor/init.c:6420 +// PPC64-AIX:#define _LONG_LONG 1 +// PPC64-AIX:#define _POWER 1 +// PPC64-AIX:#define __64BIT__ 1 hubert.reinterpretcast wrote: > hubert.reinterpretcast wrote: > > apaprocki wrote:

[PATCH] D58930: Add XCOFF triple object format type for AIX

sfertile accepted this revision. sfertile added a comment. LGTM. Comment at: llvm/lib/MC/MCContext.cpp:165 +case MCObjectFileInfo::IsXCOFF: + // TODO: Need to implement class MCSymbolXCOFF. + break; jasonliu wrote: > sfertile wrote: > > jasonliu

[PATCH] D61530: Add AIX Version Macros

sfertile accepted this revision. sfertile added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61530/new/ https://reviews.llvm.org/D61530 ___

[PATCH] D63767: [NFC] Make some ObjectFormatType switches covering

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

[PATCH] D64222: [sanitizers] Use covering ObjectFormatType switches

sfertile accepted this revision. sfertile 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/D64222/new/ https://reviews.llvm.org/D64222

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

sfertile added inline comments. Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:520 + +class XLCXXABI final : public ItaniumCXXABI { +public: Here would be a good place to add a comment to indicate that XL has several C++ ABIs, but this represents the one used

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

sfertile added inline comments. Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:283 llvm::FunctionCallee atexit = - CGM.CreateRuntimeFunction(atexitTy, "atexit", llvm::AttributeList(), -/*Local=*/true); +

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

sfertile added a comment. Please fix the formatting issues flagged by the pre-merge checks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74166/new/ https://reviews.llvm.org/D74166 ___ cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

sfertile added a comment. Aewsome, thanks for implementing this. I was on vacation for a bit and somehow missed this review in my queue when I cam back but having a look at it now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72829/new/

[PATCH] D72829: Implement -fsemantic-interposition

sfertile added a comment. In D72829#1846353 , @arichardson wrote: > As this is user-facing documentation I feel like there should be a slightly > longer explaning what the option does. +1 on this, otherwise LGTM. Thanks for implementing this!

[PATCH] D74631: [clang][XCOFF] Indicate that XCOFF does not support COMDATs

sfertile accepted this revision. sfertile added a comment. This revision is now accepted and ready to land. LGTM. Comment at: llvm/docs/LangRef.rst:913 -Note that the Mach-O platform doesn't support COMDATs, and ELF and WebAssembly -only support ``any`` as a selection kind.

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

sfertile added a comment. 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. Because of that I'm not comfortable being the one to accept the patch. CHANGES SINCE LAST

[PATCH] D74631: [clang][XCOFF] Indicate that XCOFF does not support COMDATs

sfertile added a comment. 1. We should probably update the COMDAT section of the lang ref to mention XCOFF doens't support COMDATS. 2. Will we report an error somewhere in the backend if we encounter a COMDAT? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

sfertile accepted this revision. sfertile 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/D74015/new/ https://reviews.llvm.org/D74015

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

sfertile added inline comments. Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:520 + +class XLCXXABI final : public ItaniumCXXABI { +public: Xiangling_L wrote: > sfertile wrote: > > Here would be a good place to add a comment to indicate that XL has several >

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

sfertile added inline comments. Comment at: clang/include/clang/Basic/TargetCXXABI.h:116 +/// - static initialization is adjusted to use sinit and sterm functions; +XL_Clang, + Xiangling_L wrote: > daltenty wrote: > > Why the underscore in the name?

[PATCH] D72363: [PowerPC] Default ppc64 linux-gnu/freebsd to -fno-PIC

sfertile added a comment. LGTM: I really didn't like setting PIC as the default just to work around some codegen bugs. I believe there are PowerPC codegen tests that will fail with this change though (or perhaps there is a PowerPC leak sanitizer build bot which begins to fail? @stefanp

[PATCH] D75494: [PowerPC] Delete PPCMachObjectWriter and triple for darwin

sfertile accepted this revision. sfertile 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/D75494/new/ https://reviews.llvm.org/D75494

[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

sfertile added a comment. A couple minor comments, but patch is almost ready otherwise. Comment at: clang/lib/CodeGen/TargetInfo.cpp:4249 uint64_t Size; // -msvr4-struct-return puts small aggregates in GPR3 and GPR4. Pedantic nit: Can we emit a fatal

[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

sfertile accepted this revision. sfertile 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/D76360/new/ https://reviews.llvm.org/D76360

[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

sfertile added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:4205 + +class PPCAIX32TargetCodeGenInfo : public TargetCodeGenInfo { +public: Xiangling_L wrote: > I have a question here. AIX32 falls into PPC32 target, so why we don't > inherit from

[PATCH] D76130: [PPC][AIX] Implement variadic function handling in LowerFormalArguments_AIX.

sfertile added a comment. As a first step, I suggest we break the clang changes and the LLVM changes into 2 separate patches. Repository: rZORG LLVM Github Zorg CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76130/new/ https://reviews.llvm.org/D76130

[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

sfertile added inline comments. Comment at: clang/test/CodeGen/ppc32-struct-return.c:53 + +// AIX-SVR4: fatal error: error in backend: -msvr4-struct-return not supported on AIX + jasonliu wrote: > If certain front end option is not supported on certain target,

[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

sfertile added inline comments. Comment at: clang/lib/Basic/Targets/PPC.h:373 +// This is the ELF definition, and is overridden by the Darwin and AIX +// sub-target. return TargetInfo::PowerABIBuiltinVaList; Minor nit: `target` --> `targets`.

[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

sfertile added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:4175 namespace { /// PPC32_SVR4_ABIInfo - The 32-bit PowerPC ELF (SVR4) ABI information. class PPC32_SVR4_ABIInfo : public DefaultABIInfo { ZarkoCA wrote: > sfertile wrote: > > This

[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

sfertile added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:4205 -CharUnits PPC32_SVR4_ABIInfo::getParamTypeAlignment(QualType Ty) const { +CharUnits PowerPC32ABIInfo::getParamTypeAlignment(QualType Ty) const { // Complex types are passed just like their

[PATCH] D87029: [AIX] Implement AIX special bitfield related alignment rules

sfertile added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1643 + } else { +// Handle AIX oversized Long Long bitfield under 32 bit compile mode. +if (StorageUnitSize > 32 && I get different results for using a long

[PATCH] D87029: [AIX] Implement AIX special bitfield related alignment rules

sfertile added inline comments. Comment at: clang/test/Layout/aix-bitfield-alignment.cpp:118 + +typedef __attribute__((aligned(32))) short mySHORT; +struct D { We should have a similar test for an overaligned long or long long. CHANGES SINCE LAST ACTION

[PATCH] D87029: [AIX] Implement AIX special bitfield related alignment rules

sfertile added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1633 + +if (BTy) { + BuiltinType::Kind BTyKind = BTy->getKind(); When can BTy be null? Should this instead be an assert? Instead can we implement this without looking at

[PATCH] D87029: [AIX] Implement AIX special bitfield related alignment rules

sfertile added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:16447 + +bool AIXBitfieldViolation = false; +if (const BuiltinType *BTy = FieldTy.getTypePtr()->getAs()) { Can this change can be split out into its own patch? If it can i would

[PATCH] D87029: [AIX] Implement AIX special bitfield related alignment rules

sfertile added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:16447 + +bool AIXBitfieldViolation = false; +if (const BuiltinType *BTy = FieldTy.getTypePtr()->getAs()) { Xiangling_L wrote: > sfertile wrote: > > Can this change can be split out

[PATCH] D89986: [AIX] do not emit visibility attribute into IR when there is -mignore-xcoff-visibility

sfertile added inline comments. Comment at: clang/test/CodeGen/aix-visibility-inlines-hidden.cpp:6 + +// RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large -fvisibility-inlines-hidden -emit-llvm -o - -x c++ %s | \ +// RUN: FileCheck

[PATCH] D89986: [AIX] do not emit visibility attribute into IR when there is -mignore-xcoff-visibility

sfertile added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:520 Options.DataSections = CodeGenOpts.DataSections; - Options.IgnoreXCOFFVisibility = CodeGenOpts.IgnoreXCOFFVisibility; Options.UniqueSectionNames = CodeGenOpts.UniqueSectionNames;

[PATCH] D89986: [AIX] do not emit visibility attribute into IR when there is -mignore-xcoff-visibility

sfertile added a comment. Sorry, missed a comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89986/new/ https://reviews.llvm.org/D89986 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D89986: [AIX] do not emit visibility attribute into IR when there is -mignore-xcoff-visibility

sfertile added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:520 Options.DataSections = CodeGenOpts.DataSections; - Options.IgnoreXCOFFVisibility = CodeGenOpts.IgnoreXCOFFVisibility; Options.UniqueSectionNames = CodeGenOpts.UniqueSectionNames;

[PATCH] D92445: [PowerPC] Add powerpcle target.

sfertile added a comment. > On FreeBSD, the main use of this will be on the new powerpc64le arch, where > we need to build a 32-bit LE bootloader for use with pseries. (it is easier > to retarget LLVM than make a cross-endian bootloader, as it would involve > rewriting filesystem code etc.)

[PATCH] D88676: [PPC][AIX] Add vector callee saved registers for AIX extended vector ABI

sfertile added inline comments. Comment at: llvm/lib/Target/PowerPC/PPCRegisterInfo.cpp:235 +return TM.isPPC64() + ? (Subtarget.hasAltivec() ? CSR_64_AllRegs_Altivec_RegMask + : CSR_PPC64_RegMask) ZarkoCA

[PATCH] D88676: [PPC][AIX] Add vector callee saved registers for AIX extended vector ABI

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

[PATCH] D90892: [AIX][FE] Support constructor/destructor attribute

sfertile accepted this revision. sfertile added a comment. Thanks for the updates. LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90892/new/ https://reviews.llvm.org/D90892 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D88676: [PPC][AIX] Add vector callee saved registers for AIX extended vector ABI

sfertile added inline comments. Comment at: llvm/lib/Target/PowerPC/PPCRegisterInfo.cpp:235 +return TM.isPPC64() + ? (Subtarget.hasAltivec() ? CSR_64_AllRegs_Altivec_RegMask + : CSR_PPC64_RegMask)

[PATCH] D90892: [AIX][FE] Support constructor/destructor attribute

sfertile added inline comments. Comment at: clang/test/CodeGen/aix-constructor-attribute.cpp:8 -int foo() __attribute__((constructor(180))); +// CHECK: @llvm.global_ctors = appending global [3 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* bitcast

[PATCH] D88676: [PPC][AIX] Add vector callee saved registers for AIX extended vector ABI

sfertile added inline comments. Comment at: llvm/lib/Target/PowerPC/PPCRegisterInfo.cpp:184 if (TM.isPPC64()) { -if (Subtarget.hasAltivec()) +if (Subtarget.hasAltivec()) { + if (Subtarget.isAIXABI() && !TM.getAIXExtendedAltivecABI()) I suggest

[PATCH] D87029: [AIX] Implement AIX special bitfield related alignment rules

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

[PATCH] D105635: [PowerPC][AIX] Fix Zero-width bit fields wrt MaxFieldAlign.

sfertile created this revision. sfertile added reviewers: stevewan, Jake-Egan, daltenty. sfertile added a project: PowerPC. Herald added subscribers: shchenz, nemanjai. sfertile requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. On AIX when

[PATCH] D97474: [PowerPC][AIX] Enable passing vectors in variadic functions (front-end).

sfertile created this revision. sfertile added reviewers: ZarkoCA, cebowleratibm, jasonliu. sfertile added a project: PowerPC. Herald added subscribers: shchenz, nemanjai. sfertile requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits.

[PATCH] D89986: [AIX] do not emit visibility attribute into IR when there is -mignore-xcoff-visibility

sfertile accepted this revision. sfertile added a subscriber: Xiangling_L. sfertile added a comment. > discussed with sean offline, the we do not call the > emitGlobalDtorWithCXAAtExit() in AIX. so we do not have the problem for the > "__dso_handle" I initially had concerns with the places

[PATCH] D97474: [PowerPC][AIX] Enable passing vectors in variadic functions (front-end).

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG3f40dbbbc71d: [PowerPC][AIX] Enable passing vectors in variadic functions. (authored by sfertile). Changed prior to commit:

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

sfertile added a comment. I suggest we separate the clang change and testing into a standalone patch, and the llvm backend tests into a standalone patch which we can commit separately. Comment at: clang/lib/CodeGen/TargetInfo.cpp:4646 + if (const ComplexType *CTy =

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

sfertile accepted this revision as: sfertile. sfertile added a comment. This revision is now accepted and ready to land. LGTM. Comment at: clang/test/CodeGen/aix32-complex-varargs.c:2 +// REQUIRES: powerpc-registered-target +// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff

[PATCH] D106900: [PowerPC][AIX] Packed zero-width bitfields do not affect alignment.

sfertile updated this revision to Diff 363147. sfertile added a comment. Fixed spelling and added a 'long long' zero width bitfield to the testing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106900/new/ https://reviews.llvm.org/D106900 Files:

[PATCH] D107522: [PowerPC][AIX] attribute aligned cannot decrease align of a vector var.

sfertile marked 3 inline comments as done. sfertile added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4081 +if (Ty->isVectorType() && AlignVal < 16) + return; + } aaron.ballman wrote: > This should produce a diagnostic rather than

[PATCH] D107497: [PowerPC][AIX] Limit attribute aligned to 4096.

This revision was automatically updated to reflect the committed changes. Closed by commit rG5181be344adb: [PowerPC][AIX] Limit attribute aligned to 4096. (authored by sfertile). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107497/new/

[PATCH] D107522: [PowerPC][AIX] attribute aligned cannot decrease align of a vector var.

This revision was automatically updated to reflect the committed changes. sfertile marked an inline comment as done. Closed by commit rGf888e442bcc5: [PowerPC][AIX] attribute aligned cannot decrease align of a vector var. (authored by sfertile). Changed prior to commit:

[PATCH] D107522: [PowerPC][AIX] attribute aligned cannot decrease align of a vector var.

sfertile updated this revision to Diff 364436. sfertile added a comment. - Fixed spelling mistake - Check that VD is non-nulll earlier. - added warning that the requested alignment is too small. - renamed test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D107598: [AIX] "aligned" attribute should not decrease type alignment returned by __alignof__

sfertile accepted this revision. sfertile added a comment. This revision is now accepted and ready to land. LGTM, other then 1 small test update. Comment at: clang/test/Layout/aix-alignof-align-and-pack-attr.cpp:21 +// CHECK: @{{.*}}test2{{.*}}c{{.*}} = global

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

sfertile updated this revision to Diff 364570. sfertile added a comment. Only emit diagnostic on bitfield members, which is the only difference in align(packed) behaviour XL and clang/xlclang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

sfertile added a comment. Good point Chris. The only difference in layout is related to bitfield members, so I have moved the warning to `VerifyBitField` as suggested. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107506/new/

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

sfertile updated this revision to Diff 364996. sfertile marked an inline comment as done. sfertile added a comment. Add a couple more struct layouts to the testing to show cases diagnostic is not issued. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D106900: [PowerPC][AIX] Packed zero-width bitfields do not affect alignment.

sfertile added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1781 - // pragma align(packed). - if (isAIXLayout(Context) && !MaxFieldAlignment.isZero() && !FieldSize) -FieldAlign = std::min(FieldAlign, MaxFieldAlignmentInBits); stevewan

[PATCH] D106900: [PowerPC][AIX] Packed zero-width bitfields do not affect alignment.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. sfertile marked an inline comment as done. Closed by commit rGb8f612e780e5: [PowerPC][AIX] Packed zero-width bitfields do not affect alignment. (authored by sfertile).

[PATCH] D107394: [AIX] "aligned" attribute does not decrease alignment

sfertile added inline comments. Comment at: clang/test/Layout/aix-alignof-align-and-pack-attr.cpp:11 + +// CHECK: @c = global %struct.C zeroinitializer, align 2 Minor nit: I think the other test can live in this file, highlighting the difference between the 2

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

sfertile created this revision. sfertile added reviewers: stevewan, Jake-Egan, cebowleratibm. sfertile added a project: PowerPC. Herald added subscribers: shchenz, nemanjai. sfertile requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. With xlc

[PATCH] D107497: [PowerPC][AIX] Limit attribute aligned to 4096.

sfertile updated this revision to Diff 364264. sfertile added a comment. Fix formatting. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107497/new/ https://reviews.llvm.org/D107497 Files: clang/lib/Sema/SemaDeclAttr.cpp

[PATCH] D107497: [PowerPC][AIX] Limit attribute aligned to 4096.

sfertile created this revision. sfertile added reviewers: Jake-Egan, stevewan. sfertile added a project: PowerPC. Herald added subscribers: shchenz, nemanjai. Herald added a reviewer: aaron.ballman. sfertile requested review of this revision. Herald added a project: clang. Herald added a

[PATCH] D106900: [PowerPC][AIX] Packed zero-width bitfields do not affect alignment.

sfertile updated this revision to Diff 364097. sfertile added a comment. Don't update the unpacked field align based on IsPacked. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106900/new/ https://reviews.llvm.org/D106900 Files:

[PATCH] D106900: [PowerPC][AIX] Packed zero-width bitfields do not affect alignment.

sfertile marked 2 inline comments as done. sfertile added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1783 +if (FieldPacked) { + FieldAlign = UnpackedFieldAlign = 1; +} else if (!MaxFieldAlignment.isZero()) { stevewan wrote: >

[PATCH] D107394: [AIX] "aligned" attribute does not decrease alignment

sfertile added a comment. Thanks Steven, LGTM. Comment at: clang/test/Layout/aix-alignof-align-and-pack-attr.cpp:11 + +// CHECK: @c = global %struct.C zeroinitializer, align 2 stevewan wrote: > sfertile wrote: > > Minor nit: I think the other test can live in

[PATCH] D107522: [PowerPC][AIX] attribute aligned cannot decrease align of a vector var.

sfertile created this revision. sfertile added reviewers: stevewan, Jake-Egan, hubert.reinterpretcast. Herald added subscribers: shchenz, nemanjai. Herald added a reviewer: aaron.ballman. sfertile requested review of this revision. Herald added a project: clang. Herald added a subscriber:

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

sfertile updated this revision to Diff 364814. sfertile added a comment. Fixed diagnostic to only emit when there is a bitfield member. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107506/new/ https://reviews.llvm.org/D107506 Files:

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

sfertile marked an inline comment as done. sfertile 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;

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

sfertile updated this revision to Diff 364782. sfertile added a comment. Moved the diagnostic emission to ` Sema::ActOnTagFinishDefinition` as suggested. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107506/new/ https://reviews.llvm.org/D107506

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

sfertile added a comment. In D107506#2931095 , @sfertile wrote: > Moved the diagnostic emission to ` Sema::ActOnTagFinishDefinition` as > suggested. Sorry, uploaded wrong diff. Will updated again shortly. Repository: rG LLVM Github Monorepo

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG9b10e2b1cf01: [PowerPC][AIX] Warn when using pragma align(packed) on AIX. (authored by sfertile). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107506/new/

[PATCH] D106900: [PowerPC][AIX] Packed zero-width bitfields do not affect alignment.

sfertile created this revision. sfertile added reviewers: stevewan, Jake-Egan. Herald added subscribers: shchenz, nemanjai. sfertile requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Zero-width bitfields on AIX pad out to the natral

[PATCH] D106900: [PowerPC][AIX] Packed zero-width bitfields do not affect alignment.

sfertile updated this revision to Diff 362365. sfertile added a comment. clang-formatted Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106900/new/ https://reviews.llvm.org/D106900 Files: clang/lib/AST/RecordLayoutBuilder.cpp

[PATCH] D118350: [Clang][Sema][AIX][PowerPC] Emit byval alignment warning only when struct is passed to a function

sfertile added a comment. Thanks for the updates Zarko. I think we are almost there. Comment at: clang/lib/Sema/SemaChecking.cpp:5242 +// Here we try to get information about the alignment of the struct member +// from the struct passed to the caller function.We only warn when

[PATCH] D118350: [Clang][Sema][AIX][PowerPC] Emit byval alignment warning only when struct member is passed to a function

sfertile added a comment. Hey Zarko. I really like how we only issue the warning when the struct is used, however I think this is still overly broad because of where the incompatibility lies between xlc and clang. I believe the layout of the structures will be the same for both compilers, and

[PATCH] D118350: [Clang][Sema][AIX][PowerPC] Emit byval alignment warning only when struct is passed to a function

sfertile added inline comments. Herald added a project: All. Comment at: clang/lib/Sema/SemaChecking.cpp:5246 + + if (const auto *ICE = dyn_cast(Arg->IgnoreParens())) { +if (const auto *DR = dyn_cast(ICE->getSubExpr())) { Nit: To avoid the deep nesting, can

[PATCH] D118477: [NFC][AIX]Disable new pcm tests on AIX

sfertile accepted this revision. sfertile added a comment. Thanks Steven. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118477/new/ https://reviews.llvm.org/D118477 ___ cfe-commits mailing list

[PATCH] D118474: [NFC][AIX][clang] un-XFAIL gcc profile flag compat test

sfertile added a comment. Thanks David. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118474/new/ https://reviews.llvm.org/D118474 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D118350: [Clang][Sema][AIX][PowerPC] Emit byval alignment warning only when struct is passed to a function

sfertile accepted this revision. sfertile added a comment. This revision is now accepted and ready to land. Herald added a subscriber: steakhal. LGTM Zarko, sorry about taking so long to review this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D130777: Enable embedded lto for XCOFF.

sfertile created this revision. sfertile added reviewers: hubert.reinterpretcast, DiggerLin, nemanjai, MaskRay, tejohnson, mehdi_amini, phosek, arda. Herald added subscribers: ormris, StephenFan, steven_wu, kbarton, hiraditya, inglorion, mgorny. Herald added a project: All. sfertile requested

[PATCH] D131618: [WIP][Do NOT review] LLD related changes for -ffat-lto-objects support

sfertile added inline comments. Comment at: llvm/lib/Bitcode/Writer/EmbedBitcodePass.cpp:26 + +PreservedAnalyses EmbedBitcodePass::run(Module , ModuleAnalysisManager ) { + if (M.getGlobalVariable("llvm.embedded.module", true)) From the discourse discussion: 1)