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

[Sean Fertile via Phabricator via cfe-commits]

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) it was suggested that we remove the existing -fembed-bitcode functionality 
as Apple has stop supporting it.
2) mentioned that MLGO uses the option to embed the bitcode at various points 
in the pipeline depending on if its using LTO our not.

Do we want the pass to be a bit more generic and be able to specify the global 
to use for embedding, and the section name to use as arguments? That way MLGO 
can keep using the section name it uses now . It also helps consuming tools to 
disambiguate between bitcode embedded for lto purpose from bitcode embedded for 
other purposes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131618/new/

https://reviews.llvm.org/D131618

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130777: Enable embedded lto for XCOFF.

[Sean Fertile via Phabricator via cfe-commits]

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 review of this revision.
Herald added projects: clang, LLVM.
Herald added a subscriber: cfe-commits.

The traditional system compiler (xlc) on AIX uses a 'fat' object format for LTO 
by default, embedding the intermediate representation into a special '.ipa' 
section in native XCOFF object files. Then at link time depending on command 
line options either a native link or an ipa link can be performed. This patch 
adds support for embedding the pre-link IR into the module which is then 
codegened to a native XCOFF object with the bitcode emebededed in the .info 
section. The .info section representation starts with a magic number, followed 
by an 8-byte size, an identifier string and a 4-byte unsigned difference, 
finally followed by the payload. The magic number and identifier string indcate 
to the linker that the embedded metadata is bitcode and it is appropriate for 
use in LTO.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130777

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/PowerPC/aix-embedded-bitcode.c
  clang/test/CodeGen/embed-lto-metadata.c
  clang/test/Driver/embedded-lto.c
  llvm/include/llvm/Bitcode/EmbedBitcodePass.h
  llvm/include/llvm/MC/MCStreamer.h
  llvm/include/llvm/MC/MCXCOFFStreamer.h
  llvm/lib/Bitcode/Writer/CMakeLists.txt
  llvm/lib/Bitcode/Writer/EmbedBitcodePass.cpp
  llvm/lib/MC/MCAsmStreamer.cpp
  llvm/lib/MC/MCStreamer.cpp
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Passes/PassRegistry.def
  llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
  llvm/test/Bitcode/embed-multiple.ll
  llvm/test/Bitcode/embed-unsupported-object-format.ll
  llvm/test/Bitcode/embed.ll
  llvm/test/CodeGen/PowerPC/aix-embeded-bitcode.ll
  llvm/test/CodeGen/PowerPC/aix-embeded-module-padding.ll
  llvm/test/Transforms/Util/embeded-lto-TLI-mappings.ll

Index: llvm/test/Transforms/Util/embeded-lto-TLI-mappings.ll
===
--- /dev/null
+++ llvm/test/Transforms/Util/embeded-lto-TLI-mappings.ll
@@ -0,0 +1,20 @@
+; RUN: opt -vector-library=MASSV  -passes='function(inject-tli-mappings),embed-bitcode' -S < %s | FileCheck %s
+
+target triple = "powerpc-unknown-aix"
+
+; CHECK: @llvm.compiler.used = appending global [3 x ptr] [ptr @__sind2, ptr @__log10f4, ptr @llvm.embedded.module], section "llvm.metadata"
+
+define double @sin_f64(double %in) {
+  %call = tail call double @sin(double %in)
+  ret double %call
+}
+
+declare double @sin(double) #0
+
+define float @call_llvm.log10.f32(float %in) {
+  %call = tail call float @llvm.log10.f32(float %in)
+  ret float %call
+}
+
+declare float @llvm.log10.f32(float) #0
+attributes #0 = { nounwind readnone }
Index: llvm/test/CodeGen/PowerPC/aix-embeded-module-padding.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-embeded-module-padding.ll
@@ -0,0 +1,20 @@
+; RUN: llc -mtriple powerpc-ibm-aix-xcoff < %s | \
+; RUN: FileCheck %s
+; RUN: llc -mtriple powerpc64-ibm-aix-xcoff < %s | \
+; RUN: FileCheck %s
+
+target datalayout = "E-m:a-p:32:32-i64:64-n32"
+target triple = "powerpc-ibm-aix7.2.0.0"
+
+@c = local_unnamed_addr global i8 -85, align 1
+@llvm.embedded.module = private constant [1647 x i8] c"BC\C0\DE5\14\00\00\05\00\00\00b\0C0$MY\BEf\9D\FB\B4O\1B\C8$D\012\05\00!\0C\00\00_\01\00\00\0B\02!\00\02\00\00\00\16\00\00\00\07\81#\91A\C8\04I\06\1029\92\01\84\0C%\05\08\19\1E\04\8Bb\80\0CE\02B\92\0BBd\102\148\08\18K\0A22\88Hp\C4!#D\12\87\8C\10A\92\02d\C8\08\B1\14 CF\88 \C9\0122\84\18*(*\901|\B0\\\91 \C3\C8\00\00\00\89 \00\00\0B\00\00\002\22\C8\08 bF\00!+$\98\0C!%$\98\0C\19'\0C\85\A4\90`2d\\ $c\82\80\98#@\08\03\01s\04`\00\00\13,xx\87{(\07y\80\87qh\83t\10\87vh\83pH\07|\B8\037\90\037\80\037\80\83\0DX)\B4A;\E8A8\B4\01<\E8\C1\1C\C8\81\1E\CC\81\1C\B4A:\D8\01\1D\E8\81\1D\D0A\1B\B8\C3\1C\C8\81\D2\03B\84\04\90!#EB\00\8D\10\86}$\A41\16\E27\16'\00\16_\D8!\01\01 \08@\00\00\80\00\00\00\00\04\80\C4\06\81\C2d\01\00\00Y \00\00\00\07\00\00\002\1E\98\0C\19\11L\90\8C\09\C6\04CB\AD\06\D0J\A0\08\CAa\04\00\00\00\B1\18\00\00\9B\00\00\003\08\80\1C\C4\E1\1Cf\14\01=\88C8\84\C3\8CB\80\07yx\07s\98q\0C\E6\00\0F\ED\10\0E\F4\80\0E3\0CB\1E\C2\C1\1D\CE\A1\1Cf0\05=\88C8\84\83\1B\CC\03=\C8C=\8C\03=\CCx\8Ctp\07{\08\07yH\87pp\07zp\03vx\87p 

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

[Sean Fertile via Phabricator via cfe-commits]

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

https://reviews.llvm.org/D118350

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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 the struct
+// is passed byval, hence the series of checks and early returns if we are a 
not

Real minor nit: Missing space after period.



Comment at: clang/lib/Sema/SemaChecking.cpp:5255
+  const auto *PVD = dyn_cast(
+  (dyn_cast(
+   (dyn_cast(Arg->IgnoreParens()))->getSubExpr()))

I find these longer lines hard to read. I way you had the original checks 
structured was good, my suggestion was really meant to change only the nesting. 
I think keeping the same format you would have ended up with something that 
looks like

```
const auto *ICE = dyn_cast(Arg->IgnoreParens());
if (!ICE)
  return;

const auto *DR = dyn_cast(ICE->getSubExpr());
if (!DR)
  return;

auto *PD = dyn_cast(DR->getDecl();
if (!PD || !PD->getType()->isRecordType())
  return;
```



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118350/new/

https://reviews.llvm.org/D118350

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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 we instead have a series of casts, and if 
the resulting pointer is null we return early?



Comment at: clang/test/Sema/aix-attr-align.c:10
+  int a[8] __attribute__((aligned(8)));  // no-warning
+  int b[8] __attribute__((aligned(16))); // expected-warning {{alignment of 16 
bytes for a struct member is not binary compatible with IBM XL C/C++ for AIX 
16.1.0 or older}}
 };

As far as I am aware, the layout of a 16-byte aligned member is exactly the 
same between the all the compilers on AIX, and the only incompatibility is when 
passing them byval. If that's true then warning on the declaration here is much 
too verbose, as most uses will be fine and warning on the callsite (and later 
on the function definition) calls attention to it only when there is indeed the 
possibility of an incompatability.



Comment at: clang/test/Sema/aix-attr-align.c:31
+
+  baz(p1, p2, s.b, p3, b, p5, s);// expected-note {{'b' used with 
potentially incompatible alignment here}}
+  jaz(p1, p2, a, p3, s.a, p5, t);// no-note

I get the following diagnostic from the compiler now:
```
baz(p1, p2, s.b, p3, b, p5, s);// expected-note {{'b' used with 
potentially incompatible alignment here}}
^
```
I can't seem to get the formatting exact, but the caret is pointing to 's' 
which is the correct argument to warn on, but the note uses the name 'b'.  
Before adjusting the warning using the member name made sense, but I think now 
that we are warning on the byval argument, we should be using `s`, and also 
reword the note to something like 
`passing byval argument 's'  which is 16 byte aligned may be incompatible with 
IBM XL C/C++ for AIX 16.1.0 or older`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118350/new/

https://reviews.llvm.org/D118350

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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 globals of this type will have the same alignment 
restrictions. A function like `void baz(int *);` will be compatible between the 
2 compilers since the argument is just a pointer. The difference occurs when 
passing the structure by value on the stack, where xlc doesn't align the struct 
to the expected alignment, and clang/llvm does.  Since the incompatibility is 
in the calling convention only when the struct is passed byval, that should be 
the only time we emit the diagnostic.

There are 2 other things we should verify since I'm not sure if they are 
compatible or not:

1. When passing these structures byval but in argument passing registers, I'm 
guessing that xlc doesn't skip registers whose image in the PSA is 
under-aligned while llvm will.
2. Whether xlc passes these misaligned when its done through va_args.

The answers to those 2 questions will determine if we need to warn for all 
byval arguments that have alignments from attribute that is greater than 8, or 
if we can limit it a bit further.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118350/new/

https://reviews.llvm.org/D118350

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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/

https://reviews.llvm.org/D107506

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/aix-pragma-align-packed-warn.c


Index: clang/test/Sema/aix-pragma-align-packed-warn.c
===
--- /dev/null
+++ clang/test/Sema/aix-pragma-align-packed-warn.c
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff  -fxl-pragma-pack -verify 
-fsyntax-only %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff  -fxl-pragma-pack -verify 
-fsyntax-only %s
+
+#pragma align(packed)
+struct A {  // expected-warning {{#pragma align(packed) may not be compatible 
with objects generated with AIX XL C/C++}}
+  short s1;
+  int   : 0;
+  short s2;
+};
+
+struct B {  // expected-warning {{#pragma align(packed) may not be compatible 
with objects generated with AIX XL C/C++}}
+  short a : 8;
+  short b : 8;
+  int c;
+};
+
+struct C {
+  int x, y, z;
+};
+
+struct D {
+  double d;
+  struct A a;
+};
+#pragma align(reset)
+
+struct E {
+  int a : 28;
+  int   : 0;
+  int b : 16;
+};
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16613,6 +16613,23 @@
   // Notify the consumer that we've defined a tag.
   if (!Tag->isInvalidDecl())
 Consumer.HandleTagDeclDefinition(Tag);
+
+  // Clangs implementation of #pragma align(packed) differs in bitfield layout
+  // from XLs and instead matches the XL #pragma pack(1) behavior.
+  if (Context.getTargetInfo().getTriple().isOSAIX() &&
+  AlignPackStack.hasValue()) {
+AlignPackInfo APInfo = AlignPackStack.CurrentValue;
+// Only diagnose #pragma align(packed).
+if (!APInfo.IsAlignAttr() || APInfo.getAlignMode() != 
AlignPackInfo::Packed)
+  return;
+const RecordDecl *RD = dyn_cast(Tag);
+if (!RD)
+  return;
+// Only warn if there is at least 1 bitfield member.
+if (llvm::any_of(RD->fields(),
+ [](const FieldDecl *FD) { return FD->isBitField(); }))
+  Diag(BraceRange.getBegin(), diag::warn_pragma_align_not_xl_compatible);
+  }
 }
 
 void Sema::ActOnObjCContainerFinishDefinition() {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -916,6 +916,9 @@
   InGroup;
 def err_pragma_options_align_mac68k_target_unsupported : Error<
   "mac68k alignment pragma is not supported on this target">;
+def warn_pragma_align_not_xl_compatible : Warning<
+  "#pragma align(packed) may not be compatible with objects generated with AIX 
XL C/C++">,
+  InGroup;
 def warn_pragma_pack_invalid_alignment : Warning<
   "expected #pragma pack parameter to be '1', '2', '4', '8', or '16'">,
   InGroup;


Index: clang/test/Sema/aix-pragma-align-packed-warn.c
===
--- /dev/null
+++ clang/test/Sema/aix-pragma-align-packed-warn.c
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff  -fxl-pragma-pack -verify -fsyntax-only %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff  -fxl-pragma-pack -verify -fsyntax-only %s
+
+#pragma align(packed)
+struct A {  // expected-warning {{#pragma align(packed) may not be compatible with objects generated with AIX XL C/C++}}
+  short s1;
+  int   : 0;
+  short s2;
+};
+
+struct B {  // expected-warning {{#pragma align(packed) may not be compatible with objects generated with AIX XL C/C++}}
+  short a : 8;
+  short b : 8;
+  int c;
+};
+
+struct C {
+  int x, y, z;
+};
+
+struct D {
+  double d;
+  struct A a;
+};
+#pragma align(reset)
+
+struct E {
+  int a : 28;
+  int   : 0;
+  int b : 16;
+};
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16613,6 +16613,23 @@
   // Notify the consumer that we've defined a tag.
   if (!Tag->isInvalidDecl())
 Consumer.HandleTagDeclDefinition(Tag);
+
+  // Clangs implementation of #pragma align(packed) differs in bitfield layout
+  // from XLs and instead matches the XL #pragma pack(1) behavior.
+  if (Context.getTargetInfo().getTriple().isOSAIX() &&
+  AlignPackStack.hasValue()) {
+AlignPackInfo APInfo = AlignPackStack.CurrentValue;
+// Only diagnose #pragma align(packed).
+if (!APInfo.IsAlignAttr() || APInfo.getAlignMode() != AlignPackInfo::Packed)
+  return;
+const RecordDecl *RD = dyn_cast(Tag);
+if (!RD)
+  return;
+// Only warn if there is 

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

[Sean Fertile via Phabricator via cfe-commits]

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 -emit-llvm -o - %s | 
FileCheck %s
+

ZarkoCA wrote:
> ZarkoCA wrote:
> > sfertile wrote:
> > > The code-gen for int and float won't change with this patch, lets 
> > > pre-commit this test without the _Complex short and _Complex char 
> > > portions now as an NFC patch.
> > I can't precommit this test unless I also remove the fatal error here:
> > ```
> >  if (Ty->isAnyComplexType())
> > llvm::report_fatal_error("complex type is not supported on AIX yet");
> > ``` 
> > 
> > But I believe that I can commit the llc tests and I'll go ahead and do 
> > that. 
> Sorry, not commit the llc tests but separate them in their own patch.
Sorry, I missed that. I was thinking since the word-size element types didn't 
change we should recommit heir tests, but since it was disabled with a fatal 
error that doesn't make sense.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106393/new/

https://reviews.llvm.org/D106393

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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 = Ty->getAs()) {
+CharUnits EltSize = TypeInfo.Width / 2;
+if (EltSize < SlotSize) {

Minor nit: the code for PPC64 and this is almost identical, I think it should 
be factored into a separate helper function.



Comment at: clang/test/CodeGen/aix32-complex-varargs.c:2
+// REQUIRES: powerpc-registered-target
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -emit-llvm -o - %s | 
FileCheck %s
+

The code-gen for int and float won't change with this patch, lets pre-commit 
this test without the _Complex short and _Complex char portions now as an NFC 
patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106393/new/

https://reviews.llvm.org/D106393

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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

https://reviews.llvm.org/D107506

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/aix-pragma-align-packed-warn.c


Index: clang/test/Sema/aix-pragma-align-packed-warn.c
===
--- /dev/null
+++ clang/test/Sema/aix-pragma-align-packed-warn.c
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff  -fxl-pragma-pack -verify 
-fsyntax-only %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff  -fxl-pragma-pack -verify 
-fsyntax-only %s
+
+#pragma align(packed)
+struct A {  // expected-warning {{#pragma align(packed) may not be compatible 
with objects generated with AIX XL C/C++}}
+  short s1;
+  int   : 0;
+  short s2;
+};
+
+struct B {  // expected-warning {{#pragma align(packed) may not be compatible 
with objects generated with AIX XL C/C++}}
+  short a : 8;
+  short b : 8;
+  int c;
+};
+
+struct C {
+  int x, y, z;
+};
+
+struct D {
+  double d;
+  struct A a;
+};
+#pragma align(reset)
+
+struct E {
+  int a : 28;
+  int   : 0;
+  int b : 16;
+};
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16573,6 +16573,23 @@
   // Notify the consumer that we've defined a tag.
   if (!Tag->isInvalidDecl())
 Consumer.HandleTagDeclDefinition(Tag);
+
+  // Clangs implementation of #pragma align(packed) differs in bitfield layout
+  // from XLs and instead matches the XL #pragma pack(1) behavior.
+  if (Context.getTargetInfo().getTriple().isOSAIX() &&
+  AlignPackStack.hasValue()) {
+AlignPackInfo APInfo = AlignPackStack.CurrentValue;
+// Only diagnose #pragma align(packed).
+if (!APInfo.IsAlignAttr() || APInfo.getAlignMode() != 
AlignPackInfo::Packed)
+  return;
+const RecordDecl *RD = dyn_cast(Tag);
+if (!RD)
+  return;
+// Only warn if there is at least 1 bitfield member.
+if (llvm::any_of(RD->fields(),
+ [](const FieldDecl *FD) { return FD->isBitField(); }))
+  Diag(BraceRange.getBegin(), diag::warn_pragma_align_not_xl_compatible);
+  }
 }
 
 void Sema::ActOnObjCContainerFinishDefinition() {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -913,6 +913,9 @@
   InGroup;
 def err_pragma_options_align_mac68k_target_unsupported : Error<
   "mac68k alignment pragma is not supported on this target">;
+def warn_pragma_align_not_xl_compatible : Warning<
+  "#pragma align(packed) may not be compatible with objects generated with AIX 
XL C/C++">,
+  InGroup;
 def warn_pragma_pack_invalid_alignment : Warning<
   "expected #pragma pack parameter to be '1', '2', '4', '8', or '16'">,
   InGroup;


Index: clang/test/Sema/aix-pragma-align-packed-warn.c
===
--- /dev/null
+++ clang/test/Sema/aix-pragma-align-packed-warn.c
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff  -fxl-pragma-pack -verify -fsyntax-only %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff  -fxl-pragma-pack -verify -fsyntax-only %s
+
+#pragma align(packed)
+struct A {  // expected-warning {{#pragma align(packed) may not be compatible with objects generated with AIX XL C/C++}}
+  short s1;
+  int   : 0;
+  short s2;
+};
+
+struct B {  // expected-warning {{#pragma align(packed) may not be compatible with objects generated with AIX XL C/C++}}
+  short a : 8;
+  short b : 8;
+  int c;
+};
+
+struct C {
+  int x, y, z;
+};
+
+struct D {
+  double d;
+  struct A a;
+};
+#pragma align(reset)
+
+struct E {
+  int a : 28;
+  int   : 0;
+  int b : 16;
+};
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16573,6 +16573,23 @@
   // Notify the consumer that we've defined a tag.
   if (!Tag->isInvalidDecl())
 Consumer.HandleTagDeclDefinition(Tag);
+
+  // Clangs implementation of #pragma align(packed) differs in bitfield layout
+  // from XLs and instead matches the XL #pragma pack(1) behavior.
+  if (Context.getTargetInfo().getTriple().isOSAIX() &&
+  AlignPackStack.hasValue()) {
+AlignPackInfo APInfo = AlignPackStack.CurrentValue;
+// Only diagnose #pragma align(packed).
+if (!APInfo.IsAlignAttr() || APInfo.getAlignMode() != AlignPackInfo::Packed)
+  return;
+const RecordDecl *RD = dyn_cast(Tag);
+if (!RD)
+  return;
+// Only 

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

[Sean Fertile via Phabricator via cfe-commits]

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;  // expected-warning {{#pragma align(packed) may not be 
compatible with objects generated with AIX XL C/C++}}
+  int c;

cebowleratibm wrote:
> It's undesirable to warn for each bitfield member.  Perhaps diagnose this in 
> Sema::ActOnTagFinishDefinition?
Thanks for the suggestion Chris, I've moved it as suggested.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107506/new/

https://reviews.llvm.org/D107506

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/aix-pragma-align-packed-warn.c


Index: clang/test/Sema/aix-pragma-align-packed-warn.c
===
--- /dev/null
+++ clang/test/Sema/aix-pragma-align-packed-warn.c
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff  -fxl-pragma-pack -verify 
-fsyntax-only %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff  -fxl-pragma-pack -verify 
-fsyntax-only %s
+
+#pragma align(packed)
+struct A {  // expected-warning {{#pragma align(packed) may not be compatible 
with objects generated with AIX XL C/C++}}
+  short s1;
+  int   : 0;
+  short s2;
+};
+
+struct B {  // expected-warning {{#pragma align(packed) may not be compatible 
with objects generated with AIX XL C/C++}}
+  short a : 8;
+  short b : 8;
+  int c;
+};
+
+struct C {
+  int x, y, z;
+};
+#pragma align(reset)
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16573,6 +16573,23 @@
   // Notify the consumer that we've defined a tag.
   if (!Tag->isInvalidDecl())
 Consumer.HandleTagDeclDefinition(Tag);
+
+  // Clangs implementation of #pragma align(packed) differs in bitfield layout
+  // from XLs and instead matches the XL #pragma pack(1) behavior.
+  if (Context.getTargetInfo().getTriple().isOSAIX() &&
+  AlignPackStack.hasValue()) {
+AlignPackInfo APInfo = AlignPackStack.CurrentValue;
+// Only diagnose #pragma align(packed).
+if (!APInfo.IsAlignAttr() || APInfo.getAlignMode() != 
AlignPackInfo::Packed)
+  return;
+const RecordDecl *RD = dyn_cast(Tag);
+if (!RD)
+  return;
+// Only warn if there is at least 1 bitfield member.
+if (llvm::any_of(RD->fields(),
+ [](const FieldDecl *FD) { return FD->isBitField(); }))
+  Diag(BraceRange.getBegin(), diag::warn_pragma_align_not_xl_compatible);
+  }
 }
 
 void Sema::ActOnObjCContainerFinishDefinition() {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -913,6 +913,9 @@
   InGroup;
 def err_pragma_options_align_mac68k_target_unsupported : Error<
   "mac68k alignment pragma is not supported on this target">;
+def warn_pragma_align_not_xl_compatible : Warning<
+  "#pragma align(packed) may not be compatible with objects generated with AIX 
XL C/C++">,
+  InGroup;
 def warn_pragma_pack_invalid_alignment : Warning<
   "expected #pragma pack parameter to be '1', '2', '4', '8', or '16'">,
   InGroup;


Index: clang/test/Sema/aix-pragma-align-packed-warn.c
===
--- /dev/null
+++ clang/test/Sema/aix-pragma-align-packed-warn.c
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff  -fxl-pragma-pack -verify -fsyntax-only %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff  -fxl-pragma-pack -verify -fsyntax-only %s
+
+#pragma align(packed)
+struct A {  // expected-warning {{#pragma align(packed) may not be compatible with objects generated with AIX XL C/C++}}
+  short s1;
+  int   : 0;
+  short s2;
+};
+
+struct B {  // expected-warning {{#pragma align(packed) may not be compatible with objects generated with AIX XL C/C++}}
+  short a : 8;
+  short b : 8;
+  int c;
+};
+
+struct C {
+  int x, y, z;
+};
+#pragma align(reset)
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16573,6 +16573,23 @@
   // Notify the consumer that we've defined a tag.
   if (!Tag->isInvalidDecl())
 Consumer.HandleTagDeclDefinition(Tag);
+
+  // Clangs implementation of #pragma align(packed) differs in bitfield layout
+  // from XLs and instead matches the XL #pragma pack(1) behavior.
+  if (Context.getTargetInfo().getTriple().isOSAIX() &&
+  AlignPackStack.hasValue()) {
+AlignPackInfo APInfo = AlignPackStack.CurrentValue;
+// Only diagnose #pragma align(packed).
+if (!APInfo.IsAlignAttr() || APInfo.getAlignMode() != AlignPackInfo::Packed)
+  return;
+const RecordDecl *RD = dyn_cast(Tag);
+if (!RD)
+  return;
+// Only warn if there is at least 1 bitfield member.
+if (llvm::any_of(RD->fields(),
+ [](const FieldDecl *FD) { return FD->isBitField(); }))
+  Diag(BraceRange.getBegin(), diag::warn_pragma_align_not_xl_compatible);
+  }
 }
 
 void 

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

[Sean Fertile via Phabricator via cfe-commits]

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

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107506/new/

https://reviews.llvm.org/D107506

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/aix-pragma-align-packed-warn.c


Index: clang/test/Sema/aix-pragma-align-packed-warn.c
===
--- /dev/null
+++ clang/test/Sema/aix-pragma-align-packed-warn.c
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff  -fxl-pragma-pack -verify 
-fsyntax-only %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff  -fxl-pragma-pack -verify 
-fsyntax-only %s
+
+#pragma align(packed)
+struct A {  // expected-warning {{#pragma align(packed) may not be compatible 
with objects generated with AIX XL C/C++}}
+  short s1;
+  int   : 0;
+  short s2;
+};
+
+struct B {  // expected-warning {{#pragma align(packed) may not be compatible 
with objects generated with AIX XL C/C++}}
+  short a : 8;
+  short b : 8;
+  int c;
+};
+#pragma align(reset)
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16573,6 +16573,15 @@
   // Notify the consumer that we've defined a tag.
   if (!Tag->isInvalidDecl())
 Consumer.HandleTagDeclDefinition(Tag);
+
+  // Clangs implementation of #pragma align(pack) differs in bitfield layout
+  // from XLs and instead matches the XL #pragma pack(1) behavior.
+  if (Context.getTargetInfo().getTriple().isOSAIX() &&
+  AlignPackStack.hasValue()) {
+AlignPackInfo APInfo = AlignPackStack.CurrentValue;
+if (APInfo.IsAlignAttr() && APInfo.getAlignMode() == AlignPackInfo::Packed)
+  Diag(BraceRange.getBegin(), diag::warn_pragma_align_not_xl_compatible);
+  }
 }
 
 void Sema::ActOnObjCContainerFinishDefinition() {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -913,6 +913,9 @@
   InGroup;
 def err_pragma_options_align_mac68k_target_unsupported : Error<
   "mac68k alignment pragma is not supported on this target">;
+def warn_pragma_align_not_xl_compatible : Warning<
+  "#pragma align(packed) may not be compatible with objects generated with AIX 
XL C/C++">,
+  InGroup;
 def warn_pragma_pack_invalid_alignment : Warning<
   "expected #pragma pack parameter to be '1', '2', '4', '8', or '16'">,
   InGroup;


Index: clang/test/Sema/aix-pragma-align-packed-warn.c
===
--- /dev/null
+++ clang/test/Sema/aix-pragma-align-packed-warn.c
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff  -fxl-pragma-pack -verify -fsyntax-only %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff  -fxl-pragma-pack -verify -fsyntax-only %s
+
+#pragma align(packed)
+struct A {  // expected-warning {{#pragma align(packed) may not be compatible with objects generated with AIX XL C/C++}}
+  short s1;
+  int   : 0;
+  short s2;
+};
+
+struct B {  // expected-warning {{#pragma align(packed) may not be compatible with objects generated with AIX XL C/C++}}
+  short a : 8;
+  short b : 8;
+  int c;
+};
+#pragma align(reset)
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16573,6 +16573,15 @@
   // Notify the consumer that we've defined a tag.
   if (!Tag->isInvalidDecl())
 Consumer.HandleTagDeclDefinition(Tag);
+
+  // Clangs implementation of #pragma align(pack) differs in bitfield layout
+  // from XLs and instead matches the XL #pragma pack(1) behavior.
+  if (Context.getTargetInfo().getTriple().isOSAIX() &&
+  AlignPackStack.hasValue()) {
+AlignPackInfo APInfo = AlignPackStack.CurrentValue;
+if (APInfo.IsAlignAttr() && APInfo.getAlignMode() == AlignPackInfo::Packed)
+  Diag(BraceRange.getBegin(), diag::warn_pragma_align_not_xl_compatible);
+  }
 }
 
 void Sema::ActOnObjCContainerFinishDefinition() {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -913,6 +913,9 @@
   InGroup;
 def err_pragma_options_align_mac68k_target_unsupported : Error<
   "mac68k alignment pragma is not supported on this target">;
+def warn_pragma_align_not_xl_compatible : Warning<
+  "#pragma align(packed) may not be compatible with objects generated with AIX XL C/C++">,
+  InGroup;
 def warn_pragma_pack_invalid_alignment : Warning<
   "expected #pragma pack parameter to be '1', '2', 

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

[Sean Fertile via Phabricator via cfe-commits]

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 %"struct.test2::C" 
zeroinitializer, align 2
+} // namespace test2

Minor nit: add 
```
namespace test3 {
struct __attribute__((__aligned__(16))) C {
double x;
} c;
```
as a third test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107598/new/

https://reviews.llvm.org/D107598

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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/

https://reviews.llvm.org/D107506

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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

https://reviews.llvm.org/D107506

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/aix-pragma-align-packed-warn.c


Index: clang/test/Sema/aix-pragma-align-packed-warn.c
===
--- /dev/null
+++ clang/test/Sema/aix-pragma-align-packed-warn.c
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff  -fxl-pragma-pack -verify 
-fsyntax-only %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff  -fxl-pragma-pack -verify 
-fsyntax-only %s
+
+#pragma align(packed)
+struct A {
+  short s1;
+  int   : 0;  // expected-warning {{#pragma align(packed) may not be 
compatible with objects generated with AIX XL C/C++}}
+  short s2;
+};
+
+struct B {
+  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 align(packed) may not be 
compatible with objects generated with AIX XL C/C++}}
+  int c;
+};
+#pragma align(reset)
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16701,6 +16701,13 @@
 }
   }
 
+  if (Context.getTargetInfo().getTriple().isOSAIX() &&
+  AlignPackStack.hasValue()) {
+AlignPackInfo APInfo = AlignPackStack.CurrentValue;
+if (APInfo.IsAlignAttr() && APInfo.getAlignMode() == AlignPackInfo::Packed)
+  Diag(FieldLoc, diag::warn_pragma_align_not_xl_compatible);
+  }
+
   return BitWidth;
 }
 
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -913,6 +913,9 @@
   InGroup;
 def err_pragma_options_align_mac68k_target_unsupported : Error<
   "mac68k alignment pragma is not supported on this target">;
+def warn_pragma_align_not_xl_compatible : Warning<
+  "#pragma align(packed) may not be compatible with objects generated with AIX 
XL C/C++">,
+  InGroup;
 def warn_pragma_pack_invalid_alignment : Warning<
   "expected #pragma pack parameter to be '1', '2', '4', '8', or '16'">,
   InGroup;


Index: clang/test/Sema/aix-pragma-align-packed-warn.c
===
--- /dev/null
+++ clang/test/Sema/aix-pragma-align-packed-warn.c
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff  -fxl-pragma-pack -verify -fsyntax-only %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff  -fxl-pragma-pack -verify -fsyntax-only %s
+
+#pragma align(packed)
+struct A {
+  short s1;
+  int   : 0;  // expected-warning {{#pragma align(packed) may not be compatible with objects generated with AIX XL C/C++}}
+  short s2;
+};
+
+struct B {
+  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 align(packed) may not be compatible with objects generated with AIX XL C/C++}}
+  int c;
+};
+#pragma align(reset)
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -16701,6 +16701,13 @@
 }
   }
 
+  if (Context.getTargetInfo().getTriple().isOSAIX() &&
+  AlignPackStack.hasValue()) {
+AlignPackInfo APInfo = AlignPackStack.CurrentValue;
+if (APInfo.IsAlignAttr() && APInfo.getAlignMode() == AlignPackInfo::Packed)
+  Diag(FieldLoc, diag::warn_pragma_align_not_xl_compatible);
+  }
+
   return BitWidth;
 }
 
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -913,6 +913,9 @@
   InGroup;
 def err_pragma_options_align_mac68k_target_unsupported : Error<
   "mac68k alignment pragma is not supported on this target">;
+def warn_pragma_align_not_xl_compatible : Warning<
+  "#pragma align(packed) may not be compatible with objects generated with AIX XL C/C++">,
+  InGroup;
 def warn_pragma_pack_invalid_alignment : Warning<
   "expected #pragma pack parameter to be '1', '2', '4', '8', or '16'">,
   InGroup;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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:
  https://reviews.llvm.org/D107522?vs=364436=364489#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107522/new/

https://reviews.llvm.org/D107522

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/aix-vector-attr-aligned.c
  clang/test/Sema/aix-attr-aligned-vector-warn.c

Index: clang/test/Sema/aix-attr-aligned-vector-warn.c
===
--- /dev/null
+++ clang/test/Sema/aix-attr-aligned-vector-warn.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -target-feature +altivec -target-cpu pwr7 -verify -fsyntax-only %s
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -target-feature +altivec -target-cpu pwr7 -verify -fsyntax-only %s
+
+int escape(vector int*);
+
+typedef vector int __attribute__((aligned(8))) UnderAlignedVI;
+UnderAlignedVI TypedefedGlobal;
+
+vector int V __attribute__((aligned(8))); // expected-warning {{requested alignment is less than minimum alignment of 16 for type '__vector int' (vector of 4 'int' values)}}
+
+int localTypedefed(void) {
+  UnderAlignedVI TypedefedLocal;
+  return escape(); // expected-warning {{passing 8-byte aligned argument to 16-byte aligned parameter 1 of 'escape' may result in an unaligned pointer access}}
+}
Index: clang/test/CodeGen/aix-vector-attr-aligned.c
===
--- /dev/null
+++ clang/test/CodeGen/aix-vector-attr-aligned.c
@@ -0,0 +1,33 @@
+// REQUIRES: powerpc-registered-target
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -target-feature +altivec -target-cpu pwr7 -emit-llvm -o - %s | \
+// RUN:   FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -target-feature +altivec -target-cpu pwr7 -emit-llvm -o - %s | \
+// RUN:   FileCheck %s
+
+typedef vector int __attribute__((aligned(8))) UnderAlignedVI;
+
+vector int g32 __attribute__((aligned(32)));
+vector int g8 __attribute__((aligned(8)));
+UnderAlignedVI TypedefedGlobal;
+
+int escape(vector int*);
+
+int local32(void) {
+  vector int l32 __attribute__((aligned(32)));
+  return escape();
+}
+
+int local8(void) {
+  vector int l8 __attribute__((aligned(8)));
+  return escape();
+}
+
+// CHECK: @g32 = global <4 x i32> zeroinitializer, align 32
+// CHECK: @g8 = global <4 x i32> zeroinitializer, align 16
+// CHECK: @TypedefedGlobal = global <4 x i32> zeroinitializer, align 8
+
+// CHECK-LABEL: @local32
+// CHECK: %l32 = alloca <4 x i32>, align 32
+//
+// CHECK-LABEL: @local8
+// CHECK: %l8 = alloca <4 x i32>, align 16
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -4063,12 +4063,12 @@
 return;
   }
 
-  if (Context.getTargetInfo().isTLSSupported()) {
+  const auto *VD = dyn_cast(D);
+  if (VD && Context.getTargetInfo().isTLSSupported()) {
 unsigned MaxTLSAlign =
 Context.toCharUnitsFromBits(Context.getTargetInfo().getMaxTLSAlign())
 .getQuantity();
-const auto *VD = dyn_cast(D);
-if (MaxTLSAlign && AlignVal > MaxTLSAlign && VD &&
+if (MaxTLSAlign && AlignVal > MaxTLSAlign &&
 VD->getTLSKind() != VarDecl::TLS_None) {
   Diag(VD->getLocation(), diag::err_tls_var_aligned_over_maximum)
   << (unsigned)AlignVal << VD << MaxTLSAlign;
@@ -4076,6 +4076,17 @@
 }
   }
 
+  // On AIX, an aligned attribute can not decrease the alignment when applied
+  // to a variable declaration with vector type.
+  if (VD && Context.getTargetInfo().getTriple().isOSAIX()) {
+const Type *Ty = VD->getType().getTypePtr();
+if (Ty->isVectorType() && AlignVal < 16) {
+  Diag(VD->getLocation(), diag::warn_aligned_attr_underaligned)
+  << VD->getType() << 16;
+  return;
+}
+  }
+
   AlignedAttr *AA = ::new (Context) AlignedAttr(Context, CI, true, ICE.get());
   AA->setPackExpansion(IsPackExpansion);
   D->addAttr(AA);
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2986,6 +2986,8 @@
   "redeclaration has different alignment requirement (%1 vs %0)">;
 def err_alignas_underaligned : Error<
   "requested alignment is less than minimum alignment of %1 for type %0">;
+def warn_aligned_attr_underaligned : Warning,
+  InGroup;
 def err_attribute_sizeless_type : Error<
   "%0 attribute cannot be applied to sizeless type %1">;
 def err_attribute_argument_n_type : Error<

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

[Sean Fertile via Phabricator via cfe-commits]

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/

https://reviews.llvm.org/D107497

Files:
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Sema/aix-attr-aligned-limit.c


Index: clang/test/Sema/aix-attr-aligned-limit.c
===
--- /dev/null
+++ clang/test/Sema/aix-attr-aligned-limit.c
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -fsyntax-only -verify %s
+//
+int a __attribute__((aligned(8192))); // expected-error {{requested alignment 
must be 4096 bytes or smaller}}
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -4054,6 +4054,9 @@
   unsigned MaximumAlignment = Sema::MaximumAlignment;
   if (Context.getTargetInfo().getTriple().isOSBinFormatCOFF())
 MaximumAlignment = std::min(MaximumAlignment, 8192u);
+  else if (Context.getTargetInfo().getTriple().isOSAIX())
+MaximumAlignment = std::min(MaximumAlignment, 4096u);
+
   if (AlignVal > MaximumAlignment) {
 Diag(AttrLoc, diag::err_attribute_aligned_too_great)
 << MaximumAlignment << E->getSourceRange();


Index: clang/test/Sema/aix-attr-aligned-limit.c
===
--- /dev/null
+++ clang/test/Sema/aix-attr-aligned-limit.c
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -fsyntax-only -verify %s
+//
+int a __attribute__((aligned(8192))); // expected-error {{requested alignment must be 4096 bytes or smaller}}
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -4054,6 +4054,9 @@
   unsigned MaximumAlignment = Sema::MaximumAlignment;
   if (Context.getTargetInfo().getTriple().isOSBinFormatCOFF())
 MaximumAlignment = std::min(MaximumAlignment, 8192u);
+  else if (Context.getTargetInfo().getTriple().isOSAIX())
+MaximumAlignment = std::min(MaximumAlignment, 4096u);
+
   if (AlignVal > MaximumAlignment) {
 Diag(AttrLoc, diag::err_attribute_aligned_too_great)
 << MaximumAlignment << E->getSourceRange();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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 silently drop the attribute. I'd 
> recommend `err_alignas_underaligned` if you're willing to make this an error. 
> If not, you could add a new warning with the same wording.
Thanks for the feedback Aaron. I think an error is too drastic a difference 
from the other compilers on AIX so I added the warning.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107522/new/

https://reviews.llvm.org/D107522

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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

https://reviews.llvm.org/D107522

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/aix-vector-attr-aligned.c
  clang/test/Sema/aix-attr-aligned-vector-warn.c

Index: clang/test/Sema/aix-attr-aligned-vector-warn.c
===
--- /dev/null
+++ clang/test/Sema/aix-attr-aligned-vector-warn.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -target-feature +altivec -target-cpu pwr7 -verify -fsyntax-only %s
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -target-feature +altivec -target-cpu pwr7 -verify -fsyntax-only %s
+
+int escape(vector int*);
+
+typedef vector int __attribute__((aligned(8))) UnderAlignedVI;
+UnderAlignedVI TypedefedGlobal;
+
+vector int V __attribute__((aligned(8))); // expected-warning {{requested alignment is less than minimum alignment of 16 for type '__vector int' (vector of 4 'int' values)}}
+
+int localTypedefed(void) {
+  UnderAlignedVI TypedefedLocal;
+  return escape(); // expected-warning {{passing 8-byte aligned argument to 16-byte aligned parameter 1 of 'escape' may result in an unaligned pointer access}}
+}
Index: clang/test/CodeGen/aix-vector-attr-aligned.c
===
--- /dev/null
+++ clang/test/CodeGen/aix-vector-attr-aligned.c
@@ -0,0 +1,33 @@
+// REQUIRES: powerpc-registered-target
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -target-feature +altivec -target-cpu pwr7 -emit-llvm -o - %s | \
+// RUN:   FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -target-feature +altivec -target-cpu pwr7 -emit-llvm -o - %s | \
+// RUN:   FileCheck %s
+
+typedef vector int __attribute__((aligned(8))) UnderAlignedVI;
+
+vector int g32 __attribute__((aligned(32)));
+vector int g8 __attribute__((aligned(8)));
+UnderAlignedVI TypedefedGlobal;
+
+int escape(vector int*);
+
+int local32(void) {
+  vector int l32 __attribute__((aligned(32)));
+  return escape();
+}
+
+int local8(void) {
+  vector int l8 __attribute__((aligned(8)));
+  return escape();
+}
+
+// CHECK: @g32 = global <4 x i32> zeroinitializer, align 32
+// CHECK: @g8 = global <4 x i32> zeroinitializer, align 16
+// CHECK: @TypedefedGlobal = global <4 x i32> zeroinitializer, align 8
+
+// CHECK-LABEL: @local32
+// CHECK: %l32 = alloca <4 x i32>, align 32
+//
+// CHECK-LABEL: @local8
+// CHECK: %l8 = alloca <4 x i32>, align 16
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -4060,12 +4060,12 @@
 return;
   }
 
-  if (Context.getTargetInfo().isTLSSupported()) {
+  const auto *VD = dyn_cast(D);
+  if (VD && Context.getTargetInfo().isTLSSupported()) {
 unsigned MaxTLSAlign =
 Context.toCharUnitsFromBits(Context.getTargetInfo().getMaxTLSAlign())
 .getQuantity();
-const auto *VD = dyn_cast(D);
-if (MaxTLSAlign && AlignVal > MaxTLSAlign && VD &&
+if (MaxTLSAlign && AlignVal > MaxTLSAlign &&
 VD->getTLSKind() != VarDecl::TLS_None) {
   Diag(VD->getLocation(), diag::err_tls_var_aligned_over_maximum)
   << (unsigned)AlignVal << VD << MaxTLSAlign;
@@ -4073,6 +4073,17 @@
 }
   }
 
+  // On AIX, an aligned attribute can not decrease the alignment when applied
+  // to a variable declaration with vector type.
+  if (VD && Context.getTargetInfo().getTriple().isOSAIX()) {
+const Type *Ty = VD->getType().getTypePtr();
+if (Ty->isVectorType() && AlignVal < 16) {
+  Diag(VD->getLocation(), diag::warn_aligned_attr_underaligned)
+  << VD->getType() << 16;
+  return;
+}
+  }
+
   AlignedAttr *AA = ::new (Context) AlignedAttr(Context, CI, true, ICE.get());
   AA->setPackExpansion(IsPackExpansion);
   D->addAttr(AA);
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2986,6 +2986,9 @@
   "redeclaration has different alignment requirement (%1 vs %0)">;
 def err_alignas_underaligned : Error<
   "requested alignment is less than minimum alignment of %1 for type %0">;
+def warn_aligned_attr_underaligned : Warning<
+  "requested alignment is less than minimum alignment of %1 for type %0">,
+  InGroup;
 def err_attribute_sizeless_type : Error<
   "%0 attribute cannot be applied to sizeless type %1">;
 def err_attribute_argument_n_type : Error<
___
cfe-commits mailing list

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

[Sean Fertile via Phabricator via cfe-commits]

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: cfe-commits.

On AIX an aligned attribute cannot decrease the alignment of a variable when 
placed on a variable declaration of vector type.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107522

Files:
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/aix-vector-attr-aligned.c
  clang/test/Sema/aix-attr-aligned-vector-typedef.c


Index: clang/test/Sema/aix-attr-aligned-vector-typedef.c
===
--- /dev/null
+++ clang/test/Sema/aix-attr-aligned-vector-typedef.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -target-feature +altivec 
-target-cpu pwr7 -verify -fsyntax-only %s
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -target-feature +altivec 
-target-cpu pwr7 -verify -fsyntax-only %s
+
+int escape(vector int*);
+
+typedef vector int __attribute__((aligned(8))) UnderAlignedVI;
+UnderAlignedVI TypedefedGlobal;
+
+int localTypedefed(void) {
+  UnderAlignedVI TypedefedLocal;
+  return escape(); // expected-warning {{passing 8-byte aligned 
argument to 16-byte aligned parameter 1 of 'escape' may result in an unaligned 
pointer access}}
+}
Index: clang/test/CodeGen/aix-vector-attr-aligned.c
===
--- /dev/null
+++ clang/test/CodeGen/aix-vector-attr-aligned.c
@@ -0,0 +1,33 @@
+// REQUIRES: powerpc-registered-target
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -target-feature +altivec 
-target-cpu pwr7 -emit-llvm -o - %s | \
+// RUN:   FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -target-feature +altivec 
-target-cpu pwr7 -emit-llvm -o - %s | \
+// RUN:   FileCheck %s
+
+typedef vector int __attribute__((aligned(8))) UnderAlignedVI;
+
+vector int g32 __attribute__((aligned(32)));
+vector int g8 __attribute__((aligned(8)));
+UnderAlignedVI TypedefedGlobal;
+
+int escape(vector int*);
+
+int local32(void) {
+  vector int l32 __attribute__((aligned(32)));
+  return escape();
+}
+
+int local8(void) {
+  vector int l8 __attribute__((aligned(8)));
+  return escape();
+}
+
+// CHECK: @g32 = global <4 x i32> zeroinitializer, align 32
+// CHECK: @g8 = global <4 x i32> zeroinitializer, align 16
+// CHECK: @TypedefedGlobal = global <4 x i32> zeroinitializer, align 8
+
+// CHECK-LABEL: @local32
+// CHECK: %l32 = alloca <4 x i32>, align 32
+//
+// CHECK-LABEL: @local8
+// CHECK: %l8 = alloca <4 x i32>, align 16
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -4060,11 +4060,11 @@
 return;
   }
 
+  const auto *VD = dyn_cast(D);
   if (Context.getTargetInfo().isTLSSupported()) {
 unsigned MaxTLSAlign =
 Context.toCharUnitsFromBits(Context.getTargetInfo().getMaxTLSAlign())
 .getQuantity();
-const auto *VD = dyn_cast(D);
 if (MaxTLSAlign && AlignVal > MaxTLSAlign && VD &&
 VD->getTLSKind() != VarDecl::TLS_None) {
   Diag(VD->getLocation(), diag::err_tls_var_aligned_over_maximum)
@@ -4073,6 +4073,14 @@
 }
   }
 
+  // On AIX, an aligned attribute can not decrease the alignemnt when applied
+  // to a variable declaration with vector type.
+  if (VD && Context.getTargetInfo().getTriple().isOSAIX()) {
+const Type *Ty = VD->getType().getTypePtr();
+if (Ty->isVectorType() && AlignVal < 16)
+  return;
+  }
+
   AlignedAttr *AA = ::new (Context) AlignedAttr(Context, CI, true, ICE.get());
   AA->setPackExpansion(IsPackExpansion);
   D->addAttr(AA);


Index: clang/test/Sema/aix-attr-aligned-vector-typedef.c
===
--- /dev/null
+++ clang/test/Sema/aix-attr-aligned-vector-typedef.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -target-feature +altivec -target-cpu pwr7 -verify -fsyntax-only %s
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -target-feature +altivec -target-cpu pwr7 -verify -fsyntax-only %s
+
+int escape(vector int*);
+
+typedef vector int __attribute__((aligned(8))) UnderAlignedVI;
+UnderAlignedVI TypedefedGlobal;
+
+int localTypedefed(void) {
+  UnderAlignedVI TypedefedLocal;
+  return escape(); // expected-warning {{passing 8-byte aligned argument to 16-byte aligned parameter 1 of 'escape' may result in an unaligned pointer access}}
+}
Index: clang/test/CodeGen/aix-vector-attr-aligned.c
===
--- /dev/null
+++ clang/test/CodeGen/aix-vector-attr-aligned.c
@@ -0,0 +1,33 @@
+// REQUIRES: powerpc-registered-target
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -target-feature +altivec -target-cpu pwr7 

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

[Sean Fertile via Phabricator via cfe-commits]

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 this file, highlighting the 
> > difference between the 2 cases is nice. Also I think we should add a 
> > typedef test in here as well.
> Merged the tests as suggested. We have the typedef coverage in 
> `aix-power-alignment-typedef.cpp` and `aix-power-alignment-typedef2.cpp`, not 
> sure if want to merge those as well.
No need to merge those as well.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107394/new/

https://reviews.llvm.org/D107394

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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
  clang/test/Sema/aix-attr-aligned-limit.c


Index: clang/test/Sema/aix-attr-aligned-limit.c
===
--- /dev/null
+++ clang/test/Sema/aix-attr-aligned-limit.c
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -fsyntax-only -verify %s
+//
+int a __attribute__((aligned(8192))); // expected-error {{requested alignment 
must be 4096 bytes or smaller}}
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -4054,6 +4054,9 @@
   unsigned MaximumAlignment = Sema::MaximumAlignment;
   if (Context.getTargetInfo().getTriple().isOSBinFormatCOFF())
 MaximumAlignment = std::min(MaximumAlignment, 8192u);
+  else if (Context.getTargetInfo().getTriple().isOSAIX())
+MaximumAlignment = std::min(MaximumAlignment, 4096u);
+
   if (AlignVal > MaximumAlignment) {
 Diag(AttrLoc, diag::err_attribute_aligned_too_great)
 << MaximumAlignment << E->getSourceRange();


Index: clang/test/Sema/aix-attr-aligned-limit.c
===
--- /dev/null
+++ clang/test/Sema/aix-attr-aligned-limit.c
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -fsyntax-only -verify %s
+//
+int a __attribute__((aligned(8192))); // expected-error {{requested alignment must be 4096 bytes or smaller}}
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -4054,6 +4054,9 @@
   unsigned MaximumAlignment = Sema::MaximumAlignment;
   if (Context.getTargetInfo().getTriple().isOSBinFormatCOFF())
 MaximumAlignment = std::min(MaximumAlignment, 8192u);
+  else if (Context.getTargetInfo().getTriple().isOSAIX())
+MaximumAlignment = std::min(MaximumAlignment, 4096u);
+
   if (AlignVal > MaximumAlignment) {
 Diag(AttrLoc, diag::err_attribute_aligned_too_great)
 << MaximumAlignment << E->getSourceRange();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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 and xlC pragma align(packed) will pack bitfields the same way as 
pragma align(bit_packed). xlclang, xlclang++ and clang will pack bitfields the 
same way as pragma pack(1). Issue a warning when source code using pragma 
align(packed)  to alert the user it may not be compatible with xlc/xlC.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107506

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaAttr.cpp
  clang/test/Sema/aix-pragma-align-packed-warn.c
  clang/test/Sema/aix-pragma-pack-and-align.c


Index: clang/test/Sema/aix-pragma-pack-and-align.c
===
--- clang/test/Sema/aix-pragma-pack-and-align.c
+++ clang/test/Sema/aix-pragma-pack-and-align.c
@@ -170,7 +170,7 @@
 } // namespace test7
 
 namespace test8 {
-#pragma align(packed)
+#pragma align(packed) // expected-warning {{#pragma align(packed) may not be 
compatible with objects generated with AIX XL C/C++}}
 #pragma pack(2)
 #pragma pack(show) // expected-warning {{value of #pragma pack(show) == 2}}
 struct A {
Index: clang/test/Sema/aix-pragma-align-packed-warn.c
===
--- /dev/null
+++ clang/test/Sema/aix-pragma-align-packed-warn.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff  -fxl-pragma-pack -verify 
-fsyntax-only %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff  -fxl-pragma-pack -verify 
-fsyntax-only %s
+
+#pragma align(packed) // expected-warning {{#pragma align(packed) may not be 
compatible with objects generated with AIX XL C/C++}}
+struct A {
+  int a : 8;
+  int   : 0;
+  short s;
+};
+#pragma align(reset)
Index: clang/lib/Sema/SemaAttr.cpp
===
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -234,6 +234,8 @@
 // Note that '#pragma options align=packed' is not equivalent to attribute
 // packed, it has a different precedence relative to attribute aligned.
   case POAK_Packed:
+if (this->Context.getTargetInfo().getTriple().isOSAIX())
+  Diag(PragmaLoc, diag::warn_pragma_align_not_xl_compatible);
 Action = Sema::PSK_Push_Set;
 ModeVal = AlignPackInfo::Packed;
 break;
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -913,6 +913,9 @@
   InGroup;
 def err_pragma_options_align_mac68k_target_unsupported : Error<
   "mac68k alignment pragma is not supported on this target">;
+def warn_pragma_align_not_xl_compatible : Warning<
+  "#pragma align(packed) may not be compatible with objects generated with AIX 
XL C/C++">,
+  InGroup;
 def warn_pragma_pack_invalid_alignment : Warning<
   "expected #pragma pack parameter to be '1', '2', '4', '8', or '16'">,
   InGroup;


Index: clang/test/Sema/aix-pragma-pack-and-align.c
===
--- clang/test/Sema/aix-pragma-pack-and-align.c
+++ clang/test/Sema/aix-pragma-pack-and-align.c
@@ -170,7 +170,7 @@
 } // namespace test7
 
 namespace test8 {
-#pragma align(packed)
+#pragma align(packed) // expected-warning {{#pragma align(packed) may not be compatible with objects generated with AIX XL C/C++}}
 #pragma pack(2)
 #pragma pack(show) // expected-warning {{value of #pragma pack(show) == 2}}
 struct A {
Index: clang/test/Sema/aix-pragma-align-packed-warn.c
===
--- /dev/null
+++ clang/test/Sema/aix-pragma-align-packed-warn.c
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff  -fxl-pragma-pack -verify -fsyntax-only %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff  -fxl-pragma-pack -verify -fsyntax-only %s
+
+#pragma align(packed) // expected-warning {{#pragma align(packed) may not be compatible with objects generated with AIX XL C/C++}}
+struct A {
+  int a : 8;
+  int   : 0;
+  short s;
+};
+#pragma align(reset)
Index: clang/lib/Sema/SemaAttr.cpp
===
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -234,6 +234,8 @@
 // Note that '#pragma options align=packed' is not equivalent to attribute
 // packed, it has a different precedence relative to attribute aligned.
   case POAK_Packed:
+if (this->Context.getTargetInfo().getTriple().isOSAIX())
+  Diag(PragmaLoc, diag::warn_pragma_align_not_xl_compatible);
 Action = Sema::PSK_Push_Set;
 ModeVal = AlignPackInfo::Packed;
 break;
Index: 

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

[Sean Fertile via Phabricator via cfe-commits]

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 subscriber: cfe-commits.

Limit the maximum alignment for attribute aligned to 4096 to match the limit of 
the .align pseudo op in the system assembler.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107497

Files:
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Sema/aix-attr-aligned-limit.c


Index: clang/test/Sema/aix-attr-aligned-limit.c
===
--- /dev/null
+++ clang/test/Sema/aix-attr-aligned-limit.c
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -fsyntax-only -verify %s
+//
+int a __attribute__((aligned(8192))); // expected-error {{requested alignment 
must be 4096 bytes or smaller}}
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -4054,6 +4054,9 @@
   unsigned MaximumAlignment = Sema::MaximumAlignment;
   if (Context.getTargetInfo().getTriple().isOSBinFormatCOFF())
 MaximumAlignment = std::min(MaximumAlignment, 8192u);
+  else if(Context.getTargetInfo().getTriple().isOSAIX())
+MaximumAlignment = std::min(MaximumAlignment, 4096u);
+
   if (AlignVal > MaximumAlignment) {
 Diag(AttrLoc, diag::err_attribute_aligned_too_great)
 << MaximumAlignment << E->getSourceRange();


Index: clang/test/Sema/aix-attr-aligned-limit.c
===
--- /dev/null
+++ clang/test/Sema/aix-attr-aligned-limit.c
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -fsyntax-only -verify %s
+//
+int a __attribute__((aligned(8192))); // expected-error {{requested alignment must be 4096 bytes or smaller}}
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -4054,6 +4054,9 @@
   unsigned MaximumAlignment = Sema::MaximumAlignment;
   if (Context.getTargetInfo().getTriple().isOSBinFormatCOFF())
 MaximumAlignment = std::min(MaximumAlignment, 8192u);
+  else if(Context.getTargetInfo().getTriple().isOSAIX())
+MaximumAlignment = std::min(MaximumAlignment, 4096u);
+
   if (AlignVal > MaximumAlignment) {
 Diag(AttrLoc, diag::err_attribute_aligned_too_great)
 << MaximumAlignment << E->getSourceRange();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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 cases is nice. Also I think we should add a typedef 
test in here as well.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107394/new/

https://reviews.llvm.org/D107394

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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).

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
  clang/test/Layout/aix-packed-bitfields.c


Index: clang/test/Layout/aix-packed-bitfields.c
===
--- clang/test/Layout/aix-packed-bitfields.c
+++ clang/test/Layout/aix-packed-bitfields.c
@@ -1,14 +1,18 @@
 // RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -fdump-record-layouts \
-// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | FileCheck  %s
+// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,32BIT %s
 
 // RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -fdump-record-layouts \
-// RUN: -fsyntax-only -fxl-pragma-pack -x c++ %s | FileCheck %s
-//
+// RUN: -fsyntax-only -fxl-pragma-pack -x c++ %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,32BIT %s
+
 // RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -fdump-record-layouts \
-// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | FileCheck  %s
-//
+// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,64BIT %s
+
 // RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -fdump-record-layouts \
-// RUN: -fsyntax-only -fxl-pragma-pack -x c++ %s | FileCheck %s
+// RUN: -fsyntax-only -fxl-pragma-pack -x c++ %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,64BIT %s
 
 struct A {
   int a1 : 30;
@@ -75,3 +79,35 @@
 // CHECK-NEXT: 3:6-35 |   int a2
 // CHECK-NEXT:  7:4-7 |   int a3
 // CHECK-NEXT:  sizeof=8, {{(dsize=8, )?}}align=2, preferredalign=2
+//
+struct __attribute__((packed)) PackedAttr {
+  char f1;
+  int : 0;
+  short : 3;
+  char f4 : 2;
+};
+
+int e = sizeof(struct PackedAttr);
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:  0 | struct PackedAttr
+// CHECK-NEXT:  0 |   char f1
+// CHECK-NEXT:4:- |   int
+// CHECK-NEXT:  4:0-2 |   short
+// CHECK-NEXT:  4:3-4 |   char f4
+// CHECK-NEXT:  sizeof=5, {{(dsize=5, )?}}align=1, preferredalign=1
+
+#pragma pack(2)
+struct __attribute__((packed)) PackedAttrAndPragma {
+  char f1;
+  long long : 0;
+};
+#pragma pack(pop)
+
+int f = sizeof(struct PackedAttrAndPragma);
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:  0 | struct PackedAttrAndPragma
+// CHECK-NEXT:  0 |   char f1
+// 32BIT-NEXT:4:- |   long long
+// 32BIT-NEXT:  sizeof=4, {{(dsize=4, )?}}align=1, preferredalign=1
+// 64BIT-NEXT:8:- |   long long
+// 64BIT-NEXT:  sizeof=8, {{(dsize=8, )?}}align=1, preferredalign=1
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1775,11 +1775,18 @@
   !D->getIdentifier())
 FieldAlign = UnpackedFieldAlign = 1;
 
-  // On AIX, zero-width bitfields pad out to the alignment boundary, but then
-  // do not affect overall record alignment if there is a pragma pack or
-  // pragma align(packed).
-  if (isAIXLayout(Context) && !MaxFieldAlignment.isZero() && !FieldSize)
-FieldAlign = std::min(FieldAlign, MaxFieldAlignmentInBits);
+  // On AIX, zero-width bitfields pad out to the natural alignment boundary,
+  // but do not increase the alignment greater than the MaxFieldAlignment, or 1
+  // if packed.
+  if (isAIXLayout(Context) && !FieldSize) {
+if (FieldPacked)
+  FieldAlign = 1;
+if (!MaxFieldAlignment.isZero()) {
+  UnpackedFieldAlign =
+  std::min(UnpackedFieldAlign, MaxFieldAlignmentInBits);
+  FieldAlign = std::min(FieldAlign, MaxFieldAlignmentInBits);
+}
+  }
 
   // Diagnose differences in layout due to padding or packing.
   if (!UseExternalLayout)


Index: clang/test/Layout/aix-packed-bitfields.c
===
--- clang/test/Layout/aix-packed-bitfields.c
+++ clang/test/Layout/aix-packed-bitfields.c
@@ -1,14 +1,18 @@
 // RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -fdump-record-layouts \
-// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | FileCheck  %s
+// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,32BIT %s
 
 // RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -fdump-record-layouts \
-// RUN: -fsyntax-only -fxl-pragma-pack -x c++ %s | FileCheck %s
-//
+// RUN: -fsyntax-only -fxl-pragma-pack -x c++ %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,32BIT %s
+
 // RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -fdump-record-layouts \
-// RUN: -fsyntax-only 

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

[Sean Fertile via Phabricator via cfe-commits]

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:
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/test/Layout/aix-packed-bitfields.c


Index: clang/test/Layout/aix-packed-bitfields.c
===
--- clang/test/Layout/aix-packed-bitfields.c
+++ clang/test/Layout/aix-packed-bitfields.c
@@ -1,14 +1,18 @@
 // RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -fdump-record-layouts \
-// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | FileCheck  %s
+// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,32BIT %s
 
 // RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -fdump-record-layouts \
-// RUN: -fsyntax-only -fxl-pragma-pack -x c++ %s | FileCheck %s
-//
+// RUN: -fsyntax-only -fxl-pragma-pack -x c++ %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,32BIT %s
+
 // RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -fdump-record-layouts \
-// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | FileCheck  %s
-//
+// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,64BIT %s
+
 // RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -fdump-record-layouts \
-// RUN: -fsyntax-only -fxl-pragma-pack -x c++ %s | FileCheck %s
+// RUN: -fsyntax-only -fxl-pragma-pack -x c++ %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,64BIT %s
 
 struct A {
   int a1 : 30;
@@ -75,3 +79,35 @@
 // CHECK-NEXT: 3:6-35 |   int a2
 // CHECK-NEXT:  7:4-7 |   int a3
 // CHECK-NEXT:  sizeof=8, {{(dsize=8, )?}}align=2, preferredalign=2
+//
+struct __attribute__((packed)) PackedAttr {
+  char f1;
+  int : 0;
+  short : 3;
+  char f4 : 2;
+};
+
+int e = sizeof(struct PackedAttr);
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:  0 | struct PackedAttr
+// CHECK-NEXT:  0 |   char f1
+// CHECK-NEXT:4:- |   int
+// CHECK-NEXT:  4:0-2 |   short
+// CHECK-NEXT:  4:3-4 |   char f4
+// CHECK-NEXT:  sizeof=5, {{(dsize=5, )?}}align=1, preferredalign=1
+
+#pragma pack(2)
+struct __attribute__((packed)) PackedAttrAndPragma {
+  char f1;
+  long long : 0;
+};
+#pragma pack(pop)
+
+int f = sizeof(struct PackedAttrAndPragma);
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:  0 | struct PackedAttrAndPragma
+// CHECK-NEXT:  0 |   char f1
+// 32BIT-NEXT:4:- |   long long
+// 32BIT-NEXT:  sizeof=4, {{(dsize=4, )?}}align=1, preferredalign=1
+// 64BIT-NEXT:8:- |   long long
+// 64BIT-NEXT:  sizeof=8, {{(dsize=8, )?}}align=1, preferredalign=1
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1775,11 +1775,18 @@
   !D->getIdentifier())
 FieldAlign = UnpackedFieldAlign = 1;
 
-  // On AIX, zero-width bitfields pad out to the alignment boundary, but then
-  // do not affect overall record alignment if there is a pragma pack or
-  // pragma align(packed).
-  if (isAIXLayout(Context) && !MaxFieldAlignment.isZero() && !FieldSize)
-FieldAlign = std::min(FieldAlign, MaxFieldAlignmentInBits);
+  // On AIX, zero-width bitfields pad out to the natural alignment boundary,
+  // but do not increase the alignment greater than the MaxFieldAlignment, or 1
+  // if packed.
+  if (isAIXLayout(Context) && !FieldSize) {
+if (FieldPacked)
+  FieldAlign = 1;
+if (!MaxFieldAlignment.isZero()) {
+  UnpackedFieldAlign =
+  std::min(UnpackedFieldAlign, MaxFieldAlignmentInBits);
+  FieldAlign = std::min(FieldAlign, MaxFieldAlignmentInBits);
+}
+  }
 
   // Diagnose differences in layout due to padding or packing.
   if (!UseExternalLayout)


Index: clang/test/Layout/aix-packed-bitfields.c
===
--- clang/test/Layout/aix-packed-bitfields.c
+++ clang/test/Layout/aix-packed-bitfields.c
@@ -1,14 +1,18 @@
 // RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -fdump-record-layouts \
-// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | FileCheck  %s
+// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,32BIT %s
 
 // RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -fdump-record-layouts \
-// RUN: -fsyntax-only -fxl-pragma-pack -x c++ %s | FileCheck %s
-//
+// RUN: -fsyntax-only -fxl-pragma-pack -x c++ %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,32BIT %s
+
 // RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -fdump-record-layouts \
-// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | FileCheck  %s
-//
+// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,64BIT %s
+
 // RUN: %clang_cc1 

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

[Sean Fertile via Phabricator via cfe-commits]

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:
> `UnpackedFieldAlign` is used to check if the packed attribute is unnecessary 
> (-Wpacked). here the attribute is making a difference, we probably shouldn't 
> set `FieldAlign = UnpackedFieldAlign`?
Good catch, updated.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106900/new/

https://reviews.llvm.org/D106900

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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:
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/test/Layout/aix-packed-bitfields.c


Index: clang/test/Layout/aix-packed-bitfields.c
===
--- clang/test/Layout/aix-packed-bitfields.c
+++ clang/test/Layout/aix-packed-bitfields.c
@@ -1,14 +1,18 @@
 // RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -fdump-record-layouts \
-// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | FileCheck  %s
+// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,32BIT %s
 
 // RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -fdump-record-layouts \
-// RUN: -fsyntax-only -fxl-pragma-pack -x c++ %s | FileCheck %s
-//
+// RUN: -fsyntax-only -fxl-pragma-pack -x c++ %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,32BIT %s
+
 // RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -fdump-record-layouts \
-// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | FileCheck  %s
-//
+// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,64BIT %s
+
 // RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -fdump-record-layouts \
-// RUN: -fsyntax-only -fxl-pragma-pack -x c++ %s | FileCheck %s
+// RUN: -fsyntax-only -fxl-pragma-pack -x c++ %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,64BIT %s
 
 struct A {
   int a1 : 30;
@@ -75,3 +79,35 @@
 // CHECK-NEXT: 3:6-35 |   int a2
 // CHECK-NEXT:  7:4-7 |   int a3
 // CHECK-NEXT:  sizeof=8, {{(dsize=8, )?}}align=2, preferredalign=2
+//
+struct __attribute__((packed)) PackedAttr {
+  char f1;
+  int : 0;
+  short : 3;
+  char f4 : 2;
+};
+
+int e = sizeof(struct PackedAttr);
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:  0 | struct PackedAttr
+// CHECK-NEXT:  0 |   char f1
+// CHECK-NEXT:4:- |   int
+// CHECK-NEXT:  4:0-2 |   short
+// CHECK-NEXT:  4:3-4 |   char f4
+// CHECK-NEXT:  sizeof=5, {{(dsize=5, )?}}align=1, preferredalign=1
+
+#pragma pack(2)
+struct __attribute__((packed)) PackedAttrAndPragma {
+  char f1;
+  long long : 0;
+};
+#pragma pack(pop)
+
+int f = sizeof(struct PackedAttrAndPragma);
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:  0 | struct PackedAttrAndPragma
+// CHECK-NEXT:  0 |   char f1
+// 32BIT-NEXT:4:- |   long long
+// 32BIT-NEXT:  sizeof=4, {{(dsize=4, )?}}align=1, preferredalign=1
+// 64BIT-NEXT:8:- |   long long
+// 64BIT-NEXT:  sizeof=8, {{(dsize=8, )?}}align=1, preferredalign=1
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1775,11 +1775,18 @@
   !D->getIdentifier())
 FieldAlign = UnpackedFieldAlign = 1;
 
-  // On AIX, zero-width bitfields pad out to the alignment boundary, but then
-  // do not affect overall record alignment if there is a pragma pack or
-  // pragma align(packed).
-  if (isAIXLayout(Context) && !MaxFieldAlignment.isZero() && !FieldSize)
-FieldAlign = std::min(FieldAlign, MaxFieldAlignmentInBits);
+  // On AIX, zero-width bitfields pad out to the natural alignment boundary,
+  // but do not increase the alignment greater than the MaxFieldAlignment, or 1
+  // if packed.
+  if (isAIXLayout(Context) && !FieldSize) {
+if (FieldPacked) {
+  FieldAlign = UnpackedFieldAlign = 1;
+} else if (!MaxFieldAlignment.isZero()) {
+  UnpackedFieldAlign =
+  std::min(UnpackedFieldAlign, MaxFieldAlignmentInBits);
+  FieldAlign = std::min(FieldAlign, MaxFieldAlignmentInBits);
+}
+  }
 
   // Diagnose differences in layout due to padding or packing.
   if (!UseExternalLayout)


Index: clang/test/Layout/aix-packed-bitfields.c
===
--- clang/test/Layout/aix-packed-bitfields.c
+++ clang/test/Layout/aix-packed-bitfields.c
@@ -1,14 +1,18 @@
 // RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -fdump-record-layouts \
-// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | FileCheck  %s
+// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,32BIT %s
 
 // RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -fdump-record-layouts \
-// RUN: -fsyntax-only -fxl-pragma-pack -x c++ %s | FileCheck %s
-//
+// RUN: -fsyntax-only -fxl-pragma-pack -x c++ %s | \
+// RUN:   FileCheck --check-prefixes=CHECK,32BIT %s
+
 // RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -fdump-record-layouts \
-// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | FileCheck  %s
-//
+// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | \
+// RUN:   FileCheck 

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

[Sean Fertile via Phabricator via cfe-commits]

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 wrote:
> Just noting that the comment says `MaxFieldAlignment - The maximum allowed 
> field alignment. This is set by #pragma pack`, but `__attribute__(packed)` 
> also seems to set it to some large value that is at least as large as the 
> FieldAlign. Maybe edit the comment accordingly for now, and a future 
> follow-on patch if necessary.
Sorry Steven, not sure what you are asking for. 

attribute packed will set 'FieldPacked` to true, in which case we ignore the 
max field alignment  (and why the new comment says, or 1 if packed). IIUC the 
MaXFieldAlign is set by only by pragma pack and pragma align in the Itanium 
recored layout builder, and the place where it is set based on attribute packed 
is int he Microsoft record layout builder and doesn't affect us here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106900/new/

https://reviews.llvm.org/D106900

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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
  clang/test/Layout/aix-packed-bitfields.c


Index: clang/test/Layout/aix-packed-bitfields.c
===
--- clang/test/Layout/aix-packed-bitfields.c
+++ clang/test/Layout/aix-packed-bitfields.c
@@ -75,3 +75,33 @@
 // CHECK-NEXT: 3:6-35 |   int a2
 // CHECK-NEXT:  7:4-7 |   int a3
 // CHECK-NEXT:  sizeof=8, {{(dsize=8, )?}}align=2, preferredalign=2
+//
+struct __attribute__((packed)) PackedAttr {
+  char f1;
+  int : 0;
+  short : 3;
+  char f4 : 2;
+};
+
+int e = sizeof(struct PackedAttr);
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:  0 | struct PackedAttr
+// CHECK-NEXT:  0 |   char f1
+// CHECK-NEXT:4:- |   int
+// CHECK-NEXT:  4:0-2 |   short
+// CHECK-NEXT:  4:3-4 |   char f4
+// CHECK-NEXT:  sizeof=5, {{(dsize=5, )?}}align=1, preferredalign=1
+
+#pragma pack(2)
+struct __attribute__((packed)) PackedAttrAndPragma {
+  char f1;
+  int : 0;
+};
+#pragma pack(pop)
+
+int f = sizeof(struct PackedAttrAndPragma);
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:  0 | struct PackedAttrAndPragma
+// CHECK-NEXT:  0 |   char f1
+// CHECK-NEXT:4:- |   int
+// CHECK-NEXT:  sizeof=4, {{(dsize=4, )?}}align=1, preferredalign=1
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1775,11 +1775,18 @@
   !D->getIdentifier())
 FieldAlign = UnpackedFieldAlign = 1;
 
-  // On AIX, zero-width bitfields pad out to the alignment boundary, but then
-  // do not affect overall record alignment if there is a pragma pack or
-  // pragma align(packed).
-  if (isAIXLayout(Context) && !MaxFieldAlignment.isZero() && !FieldSize)
-FieldAlign = std::min(FieldAlign, MaxFieldAlignmentInBits);
+  // On AIX, zero-width bitfields pad out to the natural alignment boundary,
+  // but dont increase the alignment greater than the MaxFieldAlignment, or 1
+  // if packed.
+  if (isAIXLayout(Context) && !FieldSize) {
+if (FieldPacked) {
+  FieldAlign = UnpackedFieldAlign = 1;
+} else if (!MaxFieldAlignment.isZero()) {
+  UnpackedFieldAlign =
+  std::min(UnpackedFieldAlign, MaxFieldAlignmentInBits);
+  FieldAlign = std::min(FieldAlign, MaxFieldAlignmentInBits);
+}
+  }
 
   // Diagnose differences in layout due to padding or packing.
   if (!UseExternalLayout)


Index: clang/test/Layout/aix-packed-bitfields.c
===
--- clang/test/Layout/aix-packed-bitfields.c
+++ clang/test/Layout/aix-packed-bitfields.c
@@ -75,3 +75,33 @@
 // CHECK-NEXT: 3:6-35 |   int a2
 // CHECK-NEXT:  7:4-7 |   int a3
 // CHECK-NEXT:  sizeof=8, {{(dsize=8, )?}}align=2, preferredalign=2
+//
+struct __attribute__((packed)) PackedAttr {
+  char f1;
+  int : 0;
+  short : 3;
+  char f4 : 2;
+};
+
+int e = sizeof(struct PackedAttr);
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:  0 | struct PackedAttr
+// CHECK-NEXT:  0 |   char f1
+// CHECK-NEXT:4:- |   int
+// CHECK-NEXT:  4:0-2 |   short
+// CHECK-NEXT:  4:3-4 |   char f4
+// CHECK-NEXT:  sizeof=5, {{(dsize=5, )?}}align=1, preferredalign=1
+
+#pragma pack(2)
+struct __attribute__((packed)) PackedAttrAndPragma {
+  char f1;
+  int : 0;
+};
+#pragma pack(pop)
+
+int f = sizeof(struct PackedAttrAndPragma);
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:  0 | struct PackedAttrAndPragma
+// CHECK-NEXT:  0 |   char f1
+// CHECK-NEXT:4:- |   int
+// CHECK-NEXT:  sizeof=4, {{(dsize=4, )?}}align=1, preferredalign=1
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1775,11 +1775,18 @@
   !D->getIdentifier())
 FieldAlign = UnpackedFieldAlign = 1;
 
-  // On AIX, zero-width bitfields pad out to the alignment boundary, but then
-  // do not affect overall record alignment if there is a pragma pack or
-  // pragma align(packed).
-  if (isAIXLayout(Context) && !MaxFieldAlignment.isZero() && !FieldSize)
-FieldAlign = std::min(FieldAlign, MaxFieldAlignmentInBits);
+  // On AIX, zero-width bitfields pad out to the natural alignment boundary,
+  // but dont increase the alignment greater than the MaxFieldAlignment, or 1
+  // if packed.
+  if (isAIXLayout(Context) && !FieldSize) {
+if (FieldPacked) {
+  FieldAlign = UnpackedFieldAlign = 1;
+} else if (!MaxFieldAlignment.isZero()) {
+  UnpackedFieldAlign =
+  

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

[Sean Fertile via Phabricator via cfe-commits]

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 alignment boundary but do not 
change the containing records alignment when they are packed.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D106900

Files:
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/test/Layout/aix-packed-bitfields.c


Index: clang/test/Layout/aix-packed-bitfields.c
===
--- clang/test/Layout/aix-packed-bitfields.c
+++ clang/test/Layout/aix-packed-bitfields.c
@@ -75,3 +75,33 @@
 // CHECK-NEXT: 3:6-35 |   int a2
 // CHECK-NEXT:  7:4-7 |   int a3
 // CHECK-NEXT:  sizeof=8, {{(dsize=8, )?}}align=2, preferredalign=2
+//
+struct __attribute__((packed)) PackedAttr {
+  char f1;
+  int : 0;
+  short : 3;
+  char f4 : 2;
+};
+
+int e = sizeof(struct PackedAttr);
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:  0 | struct PackedAttr
+// CHECK-NEXT:  0 |   char f1
+// CHECK-NEXT:4:- |   int
+// CHECK-NEXT:  4:0-2 |   short
+// CHECK-NEXT:  4:3-4 |   char f4
+// CHECK-NEXT:  sizeof=5, {{(dsize=5, )?}}align=1, preferredalign=1
+
+#pragma pack(2)
+struct __attribute__((packed)) PackedAttrAndPragma {
+  char f1;
+  int : 0;
+};
+#pragma pack(pop)
+
+int f = sizeof(struct PackedAttrAndPragma);
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:  0 | struct PackedAttrAndPragma
+// CHECK-NEXT:  0 |   char f1
+// CHECK-NEXT:4:- |   int
+// CHECK-NEXT:  sizeof=4, {{(dsize=4, )?}}align=1, preferredalign=1
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1775,11 +1775,17 @@
   !D->getIdentifier())
 FieldAlign = UnpackedFieldAlign = 1;
 
-  // On AIX, zero-width bitfields pad out to the alignment boundary, but then
-  // do not affect overall record alignment if there is a pragma pack or
-  // pragma align(packed).
-  if (isAIXLayout(Context) && !MaxFieldAlignment.isZero() && !FieldSize)
-FieldAlign = std::min(FieldAlign, MaxFieldAlignmentInBits);
+  // On AIX, zero-width bitfields pad out to the natural alignment boundary,
+  // but dont increase the alignment greater than the MaxFieldAlignment, or 1
+  // if packed.
+  if (isAIXLayout(Context) && !FieldSize) {
+if (FieldPacked) {
+  FieldAlign = UnpackedFieldAlign = 1;
+} else if(!MaxFieldAlignment.isZero()) {
+  UnpackedFieldAlign = std::min(UnpackedFieldAlign, 
MaxFieldAlignmentInBits);
+  FieldAlign = std::min(FieldAlign, MaxFieldAlignmentInBits);
+}
+  }
 
   // Diagnose differences in layout due to padding or packing.
   if (!UseExternalLayout)


Index: clang/test/Layout/aix-packed-bitfields.c
===
--- clang/test/Layout/aix-packed-bitfields.c
+++ clang/test/Layout/aix-packed-bitfields.c
@@ -75,3 +75,33 @@
 // CHECK-NEXT: 3:6-35 |   int a2
 // CHECK-NEXT:  7:4-7 |   int a3
 // CHECK-NEXT:  sizeof=8, {{(dsize=8, )?}}align=2, preferredalign=2
+//
+struct __attribute__((packed)) PackedAttr {
+  char f1;
+  int : 0;
+  short : 3;
+  char f4 : 2;
+};
+
+int e = sizeof(struct PackedAttr);
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:  0 | struct PackedAttr
+// CHECK-NEXT:  0 |   char f1
+// CHECK-NEXT:4:- |   int
+// CHECK-NEXT:  4:0-2 |   short
+// CHECK-NEXT:  4:3-4 |   char f4
+// CHECK-NEXT:  sizeof=5, {{(dsize=5, )?}}align=1, preferredalign=1
+
+#pragma pack(2)
+struct __attribute__((packed)) PackedAttrAndPragma {
+  char f1;
+  int : 0;
+};
+#pragma pack(pop)
+
+int f = sizeof(struct PackedAttrAndPragma);
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:  0 | struct PackedAttrAndPragma
+// CHECK-NEXT:  0 |   char f1
+// CHECK-NEXT:4:- |   int
+// CHECK-NEXT:  sizeof=4, {{(dsize=4, )?}}align=1, preferredalign=1
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1775,11 +1775,17 @@
   !D->getIdentifier())
 FieldAlign = UnpackedFieldAlign = 1;
 
-  // On AIX, zero-width bitfields pad out to the alignment boundary, but then
-  // do not affect overall record alignment if there is a pragma pack or
-  // pragma align(packed).
-  if (isAIXLayout(Context) && !MaxFieldAlignment.isZero() && !FieldSize)
-FieldAlign = std::min(FieldAlign, MaxFieldAlignmentInBits);
+  // On AIX, zero-width bitfields pad out to the natural alignment boundary,
+  // but dont increase the alignment greater than the MaxFieldAlignment, or 1
+  // if packed.
+  if 

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

[Sean Fertile via Phabricator via cfe-commits]

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 there is a pragma pack, or pragma align in effect then zero-width 
bitfields should pad out to the end of the bitfield container but not increase 
the alignment requirements of the struct greater then the max field align.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D105635

Files:
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/test/Layout/aix-bitfield-alignment.c
  clang/test/Layout/aix-packed-bitfields.c

Index: clang/test/Layout/aix-packed-bitfields.c
===
--- /dev/null
+++ clang/test/Layout/aix-packed-bitfields.c
@@ -0,0 +1,77 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -fdump-record-layouts \
+// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | FileCheck  %s
+
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -fdump-record-layouts \
+// RUN: -fsyntax-only -fxl-pragma-pack -x c++ %s | FileCheck %s
+//
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -fdump-record-layouts \
+// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | FileCheck  %s
+//
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -fdump-record-layouts \
+// RUN: -fsyntax-only -fxl-pragma-pack -x c++ %s | FileCheck %s
+
+struct A {
+  int a1 : 30;
+  int a2 : 30;
+  int a3 : 4;
+};
+
+int a = sizeof(struct A);
+
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:  0 | struct A
+// CHECK-NEXT: 0:0-29 |   int a1
+// CHECK-NEXT: 4:0-29 |   int a2
+// CHECK-NEXT:  8:0-3 |   int a3
+// CHECK-NEXT:  sizeof=12, {{(dsize=12, )?}}align=4, preferredalign=4
+
+#pragma align(packed)
+struct AlignPacked {
+  int a1 : 30;
+  int a2 : 30;
+  int a3 : 4;
+};
+#pragma align(reset)
+
+int b = sizeof(struct AlignPacked);
+
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:  0 | struct AlignPacked
+// CHECK-NEXT: 0:0-29 |   int a1
+// CHECK-NEXT: 3:6-35 |   int a2
+// CHECK-NEXT:  7:4-7 |   int a3
+// CHECK-NEXT:  sizeof=8, {{(dsize=8, )?}}align=1, preferredalign=1
+
+#pragma pack(1)
+struct Pack1 {
+  int a1 : 30;
+  int a2 : 30;
+  int a3 : 4;
+};
+#pragma pack(pop)
+
+int c = sizeof(struct Pack1);
+
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:  0 | struct Pack1
+// CHECK-NEXT: 0:0-29 |   int a1
+// CHECK-NEXT: 3:6-35 |   int a2
+// CHECK-NEXT:  7:4-7 |   int a3
+// CHECK-NEXT:  sizeof=8, {{(dsize=8, )?}}align=1, preferredalign=1
+
+#pragma pack(2)
+struct Pack2 {
+  int a1 : 30;
+  int a2 : 30;
+  int a3 : 4;
+};
+#pragma pack(pop)
+
+int d = sizeof(struct Pack2);
+
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:  0 | struct Pack2
+// CHECK-NEXT: 0:0-29 |   int a1
+// CHECK-NEXT: 3:6-35 |   int a2
+// CHECK-NEXT:  7:4-7 |   int a3
+// CHECK-NEXT:  sizeof=8, {{(dsize=8, )?}}align=2, preferredalign=2
Index: clang/test/Layout/aix-bitfield-alignment.c
===
--- clang/test/Layout/aix-bitfield-alignment.c
+++ clang/test/Layout/aix-bitfield-alignment.c
@@ -232,3 +232,37 @@
 // CHECK-NEXT:  0 | struct G
 // CHECK-NEXT: 0:0-44 |   long long ll
 // CHECK-NEXT:   sizeof=8, {{(dsize=8, )?}}align=8, preferredalign=8
+
+#pragma align(packed)
+struct H {
+   char c;
+   int : 0;
+   int i;
+} H;
+#pragma align(reset)
+
+int h = sizeof(H);
+
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:  0 | struct H
+// CHECK-NEXT:  0 |   char c
+// CHECK-NEXT:4:- |   int
+// CHECK-NEXT:  4 |   int i
+// CHECK-NEXT:  sizeof=8, {{(dsize=8, )?}}align=1, preferredalign=1
+
+#pragma pack(2)
+struct I {
+   char c;
+   int : 0;
+   int i;
+} I;
+#pragma pack(pop)
+
+int i = sizeof(I);
+
+// CHECK:  *** Dumping AST Record Layout
+// CHECK-NEXT:  0 | struct I
+// CHECK-NEXT:  0 |   char c
+// CHECK-NEXT:4:- |   int
+// CHECK-NEXT:  4 |   int i
+// CHECK-NEXT:  sizeof=8, {{(dsize=8, )?}}align=2, preferredalign=2
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1775,6 +1775,12 @@
   !D->getIdentifier())
 FieldAlign = UnpackedFieldAlign = 1;
 
+  // On AIX, zero-width bitfields pad out to the alignment boundary, but then
+  // do not affect overall record alignment if there is a pragma pack or
+  // pragma align(packed).
+  if (isAIXLayout(Context) && !MaxFieldAlignment.isZero() && !FieldSize)
+FieldAlign = std::min(FieldAlign, MaxFieldAlignmentInBits);
+
   // Diagnose differences in layout due to padding or packing.
   if (!UseExternalLayout)
 

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

[Sean Fertile via Phabricator via cfe-commits]

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
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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 clang introduces non-default 
visibility in codegen. The few cases I was worried we would hit were related to 
static init , but that was before @Xiangling_L static init related work.   
After those changes I have no other concerns. I would wait a day or two to 
ensure that there are no more comments  related to the round-trip-args 
behaviour but otherwise LGTM.


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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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:
  https://reviews.llvm.org/D97474?vs=326385=327167#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D97474/new/

https://reviews.llvm.org/D97474

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/aix-altivec-vaargs.c


Index: clang/test/CodeGen/aix-altivec-vaargs.c
===
--- /dev/null
+++ clang/test/CodeGen/aix-altivec-vaargs.c
@@ -0,0 +1,52 @@
+// REQUIRES: powerpc-registered-target
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -emit-llvm -target-feature 
+altivec -target-cpu pwr7 -o - %s | FileCheck %s --check-prefixes=CHECK,AIX32
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -emit-llvm -target-feature 
+altivec -target-cpu pwr7 -o - %s | FileCheck %s --check-prefixes=CHECK,AIX64
+
+vector double vector_varargs(int count, ...) {
+  __builtin_va_list arg_list;
+  __builtin_va_start(arg_list, count);
+
+  vector double ret;
+
+  for (int i = 0; i != count; ++i) {
+ret = __builtin_va_arg(arg_list, vector double);
+  }
+
+  __builtin_va_end(arg_list);
+  return ret;
+}
+
+// CHECK: %arg_list = alloca i8*
+// CHECK: %arg_list1 = bitcast i8** %arg_list to i8*
+// CHECK: call void @llvm.va_start(i8* %arg_list1)
+
+// AIX32:   for.body:
+// AIX32-NEXT:%argp.cur = load i8*, i8** %arg_list, align 4
+// AIX32-NEXT:%2 = ptrtoint i8* %argp.cur to i32
+// AIX32-NEXT:%3 = add i32 %2, 15
+// AIX32-NEXT:%4 = and i32 %3, -16
+// AIX32-NEXT:%argp.cur.aligned = inttoptr i32 %4 to i8*
+// AIX32-NEXT:%argp.next = getelementptr inbounds i8, i8* 
%argp.cur.aligned, i32 16
+// AIX32-NEXT:store i8* %argp.next, i8** %arg_list, align 4
+// AIX32-NEXT:%5 = bitcast i8* %argp.cur.aligned to <2 x double>*
+// AIX32-NEXT:%6 = load <2 x double>, <2 x double>* %5, align 16
+// AIX32-NEXT:store <2 x double> %6, <2 x double>* %ret, align 16
+// AIX32-NEXT:br label %for.inc
+
+// AIX64:   for.body:
+// AIX64-NEXT:%argp.cur = load i8*, i8** %arg_list, align 8
+// AIX64-NEXT:%2 = ptrtoint i8* %argp.cur to i64
+// AIX64-NEXT:%3 = add i64 %2, 15
+// AIX64-NEXT:%4 = and i64 %3, -16
+// AIX64-NEXT:%argp.cur.aligned = inttoptr i64 %4 to i8*
+// AIX64-NEXT:%argp.next = getelementptr inbounds i8, i8* 
%argp.cur.aligned, i64 16
+// AIX64-NEXT:store i8* %argp.next, i8** %arg_list, align 8
+// AIX64-NEXT:%5 = bitcast i8* %argp.cur.aligned to <2 x double>*
+// AIX64-NEXT:%6 = load <2 x double>, <2 x double>* %5, align 16
+// AIX64-NEXT:store <2 x double> %6, <2 x double>* %ret, align 16
+// AIX64-NEXT:br label %for.inc
+
+
+// CHECK:  for.end:
+// CHECK:%arg_list2 = bitcast i8** %arg_list to i8*
+// CHECK:call void @llvm.va_end(i8* %arg_list2)
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -4563,10 +4563,6 @@
   if (Ty->isAnyComplexType())
 llvm::report_fatal_error("complex type is not supported on AIX yet");
 
-  if (Ty->isVectorType())
-llvm::report_fatal_error(
-"vector types are not yet supported for variadic functions on AIX");
-
   auto TypeInfo = getContext().getTypeInfoInChars(Ty);
   TypeInfo.Align = getParamTypeAlignment(Ty);
 


Index: clang/test/CodeGen/aix-altivec-vaargs.c
===
--- /dev/null
+++ clang/test/CodeGen/aix-altivec-vaargs.c
@@ -0,0 +1,52 @@
+// REQUIRES: powerpc-registered-target
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -emit-llvm -target-feature +altivec -target-cpu pwr7 -o - %s | FileCheck %s --check-prefixes=CHECK,AIX32
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -emit-llvm -target-feature +altivec -target-cpu pwr7 -o - %s | FileCheck %s --check-prefixes=CHECK,AIX64
+
+vector double vector_varargs(int count, ...) {
+  __builtin_va_list arg_list;
+  __builtin_va_start(arg_list, count);
+
+  vector double ret;
+
+  for (int i = 0; i != count; ++i) {
+ret = __builtin_va_arg(arg_list, vector double);
+  }
+
+  __builtin_va_end(arg_list);
+  return ret;
+}
+
+// CHECK: %arg_list = alloca i8*
+// CHECK: %arg_list1 = bitcast i8** %arg_list to i8*
+// CHECK: call void @llvm.va_start(i8* %arg_list1)
+
+// AIX32:   for.body:
+// AIX32-NEXT:%argp.cur = load i8*, i8** %arg_list, align 4
+// AIX32-NEXT:%2 = ptrtoint i8* %argp.cur to i32
+// AIX32-NEXT:%3 = add i32 %2, 15
+// AIX32-NEXT:%4 = and i32 %3, -16
+// AIX32-NEXT:%argp.cur.aligned = inttoptr i32 %4 to i8*
+// AIX32-NEXT:%argp.next = getelementptr inbounds i8, i8* 

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

[Sean Fertile via Phabricator via cfe-commits]

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.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97474

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/aix-altivec-vaargs.c


Index: clang/test/CodeGen/aix-altivec-vaargs.c
===
--- /dev/null
+++ clang/test/CodeGen/aix-altivec-vaargs.c
@@ -0,0 +1,54 @@
+// REQUIRES: powerpc-registered-target
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -emit-llvm -target-feature 
+altivec -target-cpu pwr7 -o - %s | FileCheck %s --check-prefixes=CHECK,AIX32
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -emit-llvm -target-feature 
+altivec -target-cpu pwr7 -o - %s | FileCheck %s --check-prefixes=CHECK,AIX64
+
+#include 
+
+vector double vector_varargs(int count, ...) {
+  va_list arg_list;
+  va_start(arg_list, count);
+
+  vector double ret;
+
+  for (int i = 0; i != count; ++i) {
+ret = va_arg(arg_list, vector double);
+  }
+
+  va_end(arg_list);
+  return ret;
+}
+
+// CHECK: %arg_list = alloca i8*
+// CHECK: %arg_list1 = bitcast i8** %arg_list to i8*
+// CHECK: call void @llvm.va_start(i8* %arg_list1)
+
+// AIX32:   for.body:
+// AIX32-NEXT:%argp.cur = load i8*, i8** %arg_list, align 4
+// AIX32-NEXT:%2 = ptrtoint i8* %argp.cur to i32
+// AIX32-NEXT:%3 = add i32 %2, 15
+// AIX32-NEXT:%4 = and i32 %3, -16
+// AIX32-NEXT:%argp.cur.aligned = inttoptr i32 %4 to i8*
+// AIX32-NEXT:%argp.next = getelementptr inbounds i8, i8* 
%argp.cur.aligned, i32 16
+// AIX32-NEXT:store i8* %argp.next, i8** %arg_list, align 4
+// AIX32-NEXT:%5 = bitcast i8* %argp.cur.aligned to <2 x double>*
+// AIX32-NEXT:%6 = load <2 x double>, <2 x double>* %5, align 16
+// AIX32-NEXT:store <2 x double> %6, <2 x double>* %ret, align 16
+// AIX32-NEXT:br label %for.inc
+
+// AIX64:   for.body:
+// AIX64-NEXT:%argp.cur = load i8*, i8** %arg_list, align 8
+// AIX64-NEXT:%2 = ptrtoint i8* %argp.cur to i64
+// AIX64-NEXT:%3 = add i64 %2, 15
+// AIX64-NEXT:%4 = and i64 %3, -16
+// AIX64-NEXT:%argp.cur.aligned = inttoptr i64 %4 to i8*
+// AIX64-NEXT:%argp.next = getelementptr inbounds i8, i8* 
%argp.cur.aligned, i64 16
+// AIX64-NEXT:store i8* %argp.next, i8** %arg_list, align 8
+// AIX64-NEXT:%5 = bitcast i8* %argp.cur.aligned to <2 x double>*
+// AIX64-NEXT:%6 = load <2 x double>, <2 x double>* %5, align 16
+// AIX64-NEXT:store <2 x double> %6, <2 x double>* %ret, align 16
+// AIX64-NEXT:br label %for.inc
+
+
+// CHECK:  for.end:
+// CHECK:%arg_list2 = bitcast i8** %arg_list to i8*
+// CHECK:call void @llvm.va_end(i8* %arg_list2)
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -4563,10 +4563,6 @@
   if (Ty->isAnyComplexType())
 llvm::report_fatal_error("complex type is not supported on AIX yet");
 
-  if (Ty->isVectorType())
-llvm::report_fatal_error(
-"vector types are not yet supported for variadic functions on AIX");
-
   auto TypeInfo = getContext().getTypeInfoInChars(Ty);
   TypeInfo.Align = getParamTypeAlignment(Ty);
 


Index: clang/test/CodeGen/aix-altivec-vaargs.c
===
--- /dev/null
+++ clang/test/CodeGen/aix-altivec-vaargs.c
@@ -0,0 +1,54 @@
+// REQUIRES: powerpc-registered-target
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -emit-llvm -target-feature +altivec -target-cpu pwr7 -o - %s | FileCheck %s --check-prefixes=CHECK,AIX32
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -emit-llvm -target-feature +altivec -target-cpu pwr7 -o - %s | FileCheck %s --check-prefixes=CHECK,AIX64
+
+#include 
+
+vector double vector_varargs(int count, ...) {
+  va_list arg_list;
+  va_start(arg_list, count);
+
+  vector double ret;
+
+  for (int i = 0; i != count; ++i) {
+ret = va_arg(arg_list, vector double);
+  }
+
+  va_end(arg_list);
+  return ret;
+}
+
+// CHECK: %arg_list = alloca i8*
+// CHECK: %arg_list1 = bitcast i8** %arg_list to i8*
+// CHECK: call void @llvm.va_start(i8* %arg_list1)
+
+// AIX32:   for.body:
+// AIX32-NEXT:%argp.cur = load i8*, i8** %arg_list, align 4
+// AIX32-NEXT:%2 = ptrtoint i8* %argp.cur to i32
+// AIX32-NEXT:%3 = add i32 %2, 15
+// AIX32-NEXT:%4 = and i32 %3, -16
+// AIX32-NEXT:%argp.cur.aligned = inttoptr i32 %4 to i8*
+// AIX32-NEXT:%argp.next = getelementptr inbounds i8, i8* %argp.cur.aligned, i32 16
+// AIX32-NEXT:store i8* %argp.next, i8** %arg_list, align 4
+// AIX32-NEXT:%5 = bitcast i8* %argp.cur.aligned to <2 x double>*

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

[Sean Fertile via Phabricator via cfe-commits]

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.)

Excuse my ignorance, but what are there technical limitations preventing 
writing n 64-bit LE boot loader and avoid having a 32-bit LE target 
all-together?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92445/new/

https://reviews.llvm.org/D92445

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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 wrote:
> sfertile wrote:
> > `CSR_64_AllRegs_Altivec_RegMask` should be `CSR_PPC64_Altivec_RegMask`.  
> > FWIW I don't think this is testable without D86476. If that's the case, 
> > then it should go in that patch, not this patch. 
> Are you suggesting that I also leave the error in if I were to move this 
> change to D84676? 
Can you still run the tests that are part of this commit with that error in? My 
understanding was that it didn't interfere, but I didn't verify that. If we can 
still run the tests then yes leave the error in. If we can't then it probably 
gives us a clue about how to test the change in this patch without needing 
D84676, in which case we can keep the change and simply add the testing that 
exercises it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88676/new/

https://reviews.llvm.org/D88676

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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)

`CSR_64_AllRegs_Altivec_RegMask` should be `CSR_PPC64_Altivec_RegMask`.  FWIW I 
don't think this is testable without D86476. If that's the case, then it should 
go in that patch, not this patch. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88676/new/

https://reviews.llvm.org/D88676

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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 (i32 ()* @foo3 to void 
()*), i8* null }, { i32, void ()*, i8* } { i32 180, void ()* bitcast (i32 ()* 
@foo2 to void ()*), i8* null }, { i32, void ()*, i8* } { i32 180, void ()* 
bitcast (i32 ()* @foo to void ()*), i8* null }]
 

Did you mean for this test to be a C or C++ test? If it is meant to be C++ it 
needs to mangle the function names, but considering the director it is in and 
the fact we have the same test in CodeGenCXX directory I expect this was meant 
to be C in which case it needs a `.c` extension and lose the `-x c++` in the 
run steps.



Comment at: clang/test/CodeGen/aix-destructor-attribute.cpp:1
-// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -x c++ -emit-llvm < %s \
-// RUN: 2>&1 | \
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -emit-llvm \
+// RUN: -fno-use-cxa-atexit < %s | \

Similarly if this is meant to be a C test, give the file a .c extension.



Comment at: clang/test/CodeGen/aix-sinit-register-global-dtors-with-atexit.cpp:1
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -emit-llvm \
+// RUN: -fno-use-cxa-atexit -fregister-global-dtors-with-atexit < %s | \

I really think this test should be included in aix-destructor-attribute.c to 
highlight the codeine differences with and without the option.



Comment at: clang/test/CodeGenCXX/aix-destructor-attribute.cpp:1
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -x c++ -emit-llvm \
+// RUN: -fno-use-cxa-atexit < %s | \

Can I ask why this is added as a new file? Its the same test as 
`aix-sinit-register-global-dtors-with-atexit.cpp` but without using 
`-fregister-global-dtors-with-atexit`. I suggest combining the 2.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90892/new/

https://reviews.llvm.org/D90892

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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 doing the error checking once before  getting into the 32-bit/64-bit 
blocks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88676/new/

https://reviews.llvm.org/D88676

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

sfertile added inline comments.



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

DiggerLin wrote:
> sfertile wrote:
> > DiggerLin wrote:
> > > jasonliu wrote:
> > > > DiggerLin wrote:
> > > > > DiggerLin wrote:
> > > > > > jasonliu wrote:
> > > > > > > DiggerLin wrote:
> > > > > > > > jasonliu wrote:
> > > > > > > > > DiggerLin wrote:
> > > > > > > > > > jasonliu wrote:
> > > > > > > > > > > Instead of just removing this line, should this get 
> > > > > > > > > > > replaced with the new LangOpts option?
> > > > > > > > > > I do not think we need a CodeGenOp of 
> > > > > > > > > > ignore-xcoff-visibility in clang, we only need the LangOpt 
> > > > > > > > > > of the ignore-xcoff-visilbity to control whether we will  
> > > > > > > > > > generate the visibility in the IR,  when the LangOpt of 
> > > > > > > > > > ignore-xcoff-visibility do not generate the visibility 
> > > > > > > > > > attribute of GV in the IR. it do not need CodeGenOp of 
> > > > > > > > > > ignore-xcoff-visibility any more for the clang .
> > > > > > > > > > 
> > > > > > > > > > we have still CodeGen ignore-xcoff-visibility op in  llc.
> > > > > > > > > We removed the visibility from IR level with this patch. But 
> > > > > > > > > there is also visibility settings coming from CodeGen part of 
> > > > > > > > > clang, which needs to get ignore when we are doing the code 
> > > > > > > > > gen in llc. So I think you still need to set the options 
> > > > > > > > > correct for llc.
> > > > > > > > yes we have the set the options correct for llc in the code.
> > > > > > > > 
> > > > > > > > in the source file llvm/lib/CodeGen/CommandFlags.cpp, we have 
> > > > > > > > (in the patch https://reviews.llvm.org/D87451 add new option 
> > > > > > > > -mignore-xcoff-visibility) , the function
> > > > > > > > TargetOptions codegen::InitTargetOptionsFromCodeGenFlags() {
> > > > > > > > 
> > > > > > > > Options.IgnoreXCOFFVisibility = getIgnoreXCOFFVisibility(); 
> > > > > > > > ...}
> > > > > > > > 
> > > > > > > What I'm saying is... 
> > > > > > > I think we need a line like this:
> > > > > > > `Options.IgnoreXCOFFVisibility = LangOpts.IgnoreXCOFFVisibility;`
> > > > > > > so that when you invoke clang, backend would get the correct 
> > > > > > > setting as well. 
> > > > > > I do not think so, from the clang FE, we do not generated the 
> > > > > > visibility in the IR. so there is no need these line.
> > > > > or we can say that because we do not set the hidden visibility into 
> > > > > the GlobalValue , so we do not need the 
> > > > > Options.IgnoreXCOFFVisibility = LangOpts.IgnoreXCOFFVisibility;
> > > > I think I mentioned this before, we could have extra visibility 
> > > > settings in clang/lib/CodeGen that's not depending on the existing 
> > > > visibility setting in the IR. (You could search for `setVisibility` 
> > > > there.) That was the reason we did it in llc first. 
> > > I will add Options.IgnoreXCOFFVisibility = 
> > > LangOpts.IgnoreXCOFFVisibility;  here.
> > > I think I mentioned this before, we could have extra visibility settings 
> > > in clang/lib/CodeGen that's not depending on the existing visibility 
> > > setting in the IR. (You could search for setVisibility there.) That was 
> > > the reason we did it in llc first.
> > 
> >  A lot of these are in places we wouldn't encounter with AIX, like for 
> > Objective-C code gen. But are others like [[ 
> > https://github.com/llvm/llvm-project/blob/b03ea054db1bcf9452b3a70e21d3372b6e58759a/clang/lib/CodeGen/ItaniumCXXABI.cpp#L2507
> >  | this]]  an issue? Should they be addressed in this patch?
> after I added the Options.IgnoreXCOFFVisibility = 
> LangOpts.IgnoreXCOFFVisibility , even there is  
> GV->setVisibility(llvm::GlobalValue::HiddenVisibility);  it do not effect our 
> output.
> 
> there is following code in the function void 
> PPCAIXAsmPrinter::emitLinkage(const GlobalValue *GV,
>MCSymbol *GVSym) const
> {
>  . 
>   if (!TM.getIgnoreXCOFFVisibility()) {
> switch (GV->getVisibility()) {
> 
> // TODO: "exported" and "internal" Visibility needs to go here.
> case GlobalValue::DefaultVisibility:
>   break;
> case GlobalValue::HiddenVisibility:
>   VisibilityAttr = MAI->getHiddenVisibilityAttr();
>   break;
> case GlobalValue::ProtectedVisibility:
>   VisibilityAttr = MAI->getProtectedVisibilityAttr();
>   break;
> }
>   }
> 
> ...
> }
> it do not effect our output.
It can if we set the GlobalValue to be dso_local though ... that is the whole 
point of this patch.



Comment at: clang/test/CodeGen/aix-visibility-inlines-hidden.cpp:1
+// REQUIRES: powerpc-registered-target
+

DiggerLin wrote:
> jasonliu wrote:
> > sfertile wrote:

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

[Sean Fertile via Phabricator via cfe-commits]

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 -check-prefixes=COMMON-IR,NOVISIBILITY-IR %s

Can you break up this RUN step similar to how you formatted the following one.


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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

sfertile added inline comments.



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

DiggerLin wrote:
> jasonliu wrote:
> > DiggerLin wrote:
> > > DiggerLin wrote:
> > > > jasonliu wrote:
> > > > > DiggerLin wrote:
> > > > > > jasonliu wrote:
> > > > > > > DiggerLin wrote:
> > > > > > > > jasonliu wrote:
> > > > > > > > > Instead of just removing this line, should this get replaced 
> > > > > > > > > with the new LangOpts option?
> > > > > > > > I do not think we need a CodeGenOp of ignore-xcoff-visibility 
> > > > > > > > in clang, we only need the LangOpt of the 
> > > > > > > > ignore-xcoff-visilbity to control whether we will  generate the 
> > > > > > > > visibility in the IR,  when the LangOpt of 
> > > > > > > > ignore-xcoff-visibility do not generate the visibility 
> > > > > > > > attribute of GV in the IR. it do not need CodeGenOp of 
> > > > > > > > ignore-xcoff-visibility any more for the clang .
> > > > > > > > 
> > > > > > > > we have still CodeGen ignore-xcoff-visibility op in  llc.
> > > > > > > We removed the visibility from IR level with this patch. But 
> > > > > > > there is also visibility settings coming from CodeGen part of 
> > > > > > > clang, which needs to get ignore when we are doing the code gen 
> > > > > > > in llc. So I think you still need to set the options correct for 
> > > > > > > llc.
> > > > > > yes we have the set the options correct for llc in the code.
> > > > > > 
> > > > > > in the source file llvm/lib/CodeGen/CommandFlags.cpp, we have (in 
> > > > > > the patch https://reviews.llvm.org/D87451 add new option 
> > > > > > -mignore-xcoff-visibility) , the function
> > > > > > TargetOptions codegen::InitTargetOptionsFromCodeGenFlags() {
> > > > > > 
> > > > > > Options.IgnoreXCOFFVisibility = getIgnoreXCOFFVisibility(); 
> > > > > > ...}
> > > > > > 
> > > > > What I'm saying is... 
> > > > > I think we need a line like this:
> > > > > `Options.IgnoreXCOFFVisibility = LangOpts.IgnoreXCOFFVisibility;`
> > > > > so that when you invoke clang, backend would get the correct setting 
> > > > > as well. 
> > > > I do not think so, from the clang FE, we do not generated the 
> > > > visibility in the IR. so there is no need these line.
> > > or we can say that because we do not set the hidden visibility into the 
> > > GlobalValue , so we do not need the 
> > > Options.IgnoreXCOFFVisibility = LangOpts.IgnoreXCOFFVisibility;
> > I think I mentioned this before, we could have extra visibility settings in 
> > clang/lib/CodeGen that's not depending on the existing visibility setting 
> > in the IR. (You could search for `setVisibility` there.) That was the 
> > reason we did it in llc first. 
> I will add Options.IgnoreXCOFFVisibility = LangOpts.IgnoreXCOFFVisibility;  
> here.
> I think I mentioned this before, we could have extra visibility settings in 
> clang/lib/CodeGen that's not depending on the existing visibility setting in 
> the IR. (You could search for setVisibility there.) That was the reason we 
> did it in llc first.

 A lot of these are in places we wouldn't encounter with AIX, like for 
Objective-C code gen. But are others like [[ 
https://github.com/llvm/llvm-project/blob/b03ea054db1bcf9452b3a70e21d3372b6e58759a/clang/lib/CodeGen/ItaniumCXXABI.cpp#L2507
 | this]]  an issue? Should they be addressed in this patch?



Comment at: clang/test/CodeGen/aix-ignore-xcoff-visibility.cpp:1
 // REQUIRES: powerpc-registered-target
 

You shouldn't need this requires anymore.



Comment at: clang/test/CodeGen/aix-ignore-xcoff-visibility.cpp:67
+// NOVISIBILITY-IR:define void @_Z7prambarv()
+

minor nit: Remove the blank line.



Comment at: clang/test/CodeGen/aix-visibility-inlines-hidden.cpp:1
+// REQUIRES: powerpc-registered-target
+

Shouldn't need this requires either.



Comment at: clang/test/CodeGen/aix-visibility-inlines-hidden.cpp:4
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large -emit-llvm -o - 
-x c++ %s  | \
+// RUN: FileCheck -check-prefixes=COMMON-IR,NOVISIBILITY-IR %s
+

Can we drop the COMMON-IR part of the test? I don't think it adds much value 
but clutters the run lines.



Comment at: clang/test/CodeGen/aix-visibility-inlines-hidden.cpp:13
+
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -mcmodel=large 
-mignore-xcoff-visibility -fvisibility-inlines-hidden \ 
+// RUN:-fvisibility default -emit-llvm -o - -x c++ %s  | \

There is trailing white space at the end of this line.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89986/new/

https://reviews.llvm.org/D89986

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

[Sean Fertile via Phabricator via cfe-commits]

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 the underlying type of the 
bitfield?
Considering StorageUnitSize:
* for types smaller then unsigned we always increase the storage size to that 
of unsigned.
* for int, and long in 32-bit the storage size is already 32 and we don't 
adjust it.
* for long long in 32-bit we decrease the size to 32.
* For long long and long in 64-bit we leave the storage size as 64.

This could be implemented as
```
if (StorageUnitSize <  Context.getTypeSize(Context.UnsignedIntTy) || 
(StorageUnitSize > Context.getTypeSize(Context.UnsignedIntTy) &&  
Context.getTargetInfo().getTriple().isArch32Bit()))
  StorageUnitSize =   Context.getTypeSize(Context.UnsignedIntTy);
```

Without relying on extracting the underlying type and having to worry about the 
case where BTy is null.
If we can also handle `FieldAlign `without knowing the underlying type then I 
think we should do so and avoid trying to extract the underlying type 
altogether.



Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1638
+  BTyKind != BuiltinType::LongLong) {
+// Only set bitfields alignment to unsigned when it does
+// not have attribute align specified or is less than

`to unsigned` --> `to alignof(unsigned)`



Comment at: clang/test/Layout/aix-bitfield-alignment.cpp:1
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -fdump-record-layouts \
+// RUN: -fsyntax-only -faix-pragma-pack -x c++ %s | \

I suggest we split the test cases which are C++ only into a sperate file, and 
run the cases that are valid in both C and C++ as both to ensure there are no 
differences between C and C++ layout fir bit fields.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87029/new/

https://reviews.llvm.org/D87029

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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 into its own patch? If it can i would 
> > suggest doing so.
> I was expecting our buildbot can pick up all bitfield related changes at one 
> time. Also if we split this out, that means we either need to wait for this 
> second part to land first or need to add more LIT to oversized long long to 
> the first part, which then needs to be removed whenever this second part 
> land. It seems we are complicating the patch. Can you give me your rationale 
> about why we want to split out this part?
> I was expecting our buildbot can pick up all bitfield related changes at one 
> time.
IIUC `clang/test/Layout/aix-oversized-bitfield.cpp` works with just this change 
and isn't dependent on D87702. Its disjoint from the other changes in this 
patch, and packaging it into a commit with unrelated changes even if they are 
on the same theme is not beneficial. Its better to have those run through the 
build bot (or be bisectable) as distinct changes.

> Also if we split this out, that means we either need to wait for this second 
> part to land first or need to add more LIT to oversized long long to the 
> first part, which then needs to be removed whenever this second part land.  
> It seems we are complicating the patch.

I don't understand why it would need to wait or require extra testing to be 
added. Its a diagnostic and your lit test shows the error for 32-bit (where we 
want it emitted)  and expected layout for 64-bit. The whole point of splitting 
it out is that its simple,does exactly one thing, is testable on its own,  and 
we don't need the context of the other changes packaged with it to properly 
review it. I am asking to split it out because I see it as making this easier 
to review and commit.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87029/new/

https://reviews.llvm.org/D87029

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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

https://reviews.llvm.org/D87029

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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 long bitfield, and using an enum with 
an underlying long long type:


```
struct S {
unsigned long long  field : 32;
};

extern "C" int printf(const char*, ...);

int main(void) {
printf("%lu %lu\n", sizeof(struct S), alignof(struct S));
}

```
```
*** Dumping AST Record Layout
 0 | struct S
0:0-31 |   unsigned long long field
   | [sizeof=4, dsize=4, align=4, preferredalign=4,
   |  nvsize=4, nvalign=4, preferrednvalign=4]
```
__

```
enum e : unsigned long long { E = 1 };

struct S {
enum e field : 32;
};

extern "C" int printf(const char*, ...);

int main(void) {
printf("%lu %lu\n", sizeof(struct S), alignof(struct S));
}
```

```
*** Dumping AST Record Layout
 0 | struct S
0:0-31 |   enum e field
   | [sizeof=8, dsize=8, align=8, preferredalign=8,
   |  nvsize=8, nvalign=8, preferrednvalign=8]
```
Shouldn't we be truncating the size/align of the enum typed bitfield as well?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87029/new/

https://reviews.llvm.org/D87029

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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 suggest 
doing so.



Comment at: clang/test/Layout/aix-oversized-bitfield.cpp:1
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -fsyntax-only -verify %s
+

Is it possible to run this test as both C and C++?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87029/new/

https://reviews.llvm.org/D87029

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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, I think it 
> makes more sense to have a standard diagnostic in the driver component, 
> instead of "crash" in the backend. 
> i.e. What if we specify this option on a Windows machine? Maybe we should 
> pursue the same behavior. 
Its not that we don't intend to support the option on AIX, but that support for 
the option takes further verification on AIX.  Since there is a difference how  
AIX justifies subregister sized values in its argument passing, we can't just 
assume that this option will pack the return values the same way on AIX and 
Linux. 

The focus of this patch was originally to enable and verify the proper IR 
generation of va-arg/va-lis/va-start, however the scope of the patch has kept 
growing. If there are codegen differences in packing the return register with 
the svr4-return option enabled it will grow this patch once again. The fatal 
error lets us limit the scope of this patch, while not silently emiting bad 
codegen if there is a difference in how gcc initializes the return  registers. 
If during verification we decide we don't want to support the option on AIX, 
then adopting  a standard diagnostic in the driver component becomes the 
appropriate behavior. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76360/new/

https://reviews.llvm.org/D76360



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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 error if `-msvr4-struct-return` is specified 
on AIX. Then we can add a lit test which checks for the error, which will 
change once we verify the options behavior on AIX and enable it. 

Note: FWIW both Zarko and I have verified the option is accepted and work as 
expected by passing in r3/r4, I'm just not sure if any padding is handled the 
same way on AIX as described below.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4483
+  // AIX does not have SPE registers so we don't set 111 - 113.
+  if (getABIInfo().getTarget().getTriple().isOSAIX()) {
+AssignToArrayRange(Builder, Address, Four8, 109, 110);

Minor nit:

Can we restructure like this:

```
 // 109: vrsave
// 110: vscr
AssignToArrayRange(Builder, Address, Four8, 109, 110);

  // AIX does not have SPE registers so we don't set 111 - 113.
  if (getABIInfo().getTarget().getTriple().isOSAIX()) 
  return false;

// 111: spe_acc
  // 112: spefscr
  // 113: sfp
  AssignToArrayRange(Builder, Address, Four8, 111, 113);

return false;
```



Comment at: clang/test/CodeGen/ppc64-aix32-dwarf.c:1
+// RUN: %clang_cc1 -triple powerpc64-unknown-unknown -emit-llvm %s -o - | 
FileCheck %s --check-prefix=PPC64
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -emit-llvm  %s -o - | 
FileCheck %s --check-prefix=AIX32

Minor nit: How about naming the test file ppc-dwarf.c


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76360/new/

https://reviews.llvm.org/D76360



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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 `PPC32TargetCodeGenInfo` instead?
Do we need a separate AIX specific class? We are implementing 2 functions, 1 of 
which is the same implementation as its `PPC32TargetCodeGenInfo` counterpart. 
If we have access to the triple, we can  return true when the OS is AIX in 
`PPC32TargetCodeGenInfo::initDwarfEHRegSizeTable`. With the implementations 
being nearly identical (and after enabling DwarfEHRegSizeTable they will be 
identical) I think we are better to not add a new class if we can avoid it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76360/new/

https://reviews.llvm.org/D76360



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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 elements

jasonliu wrote:
> So for AIX we are taking PowerPC32ABIInfo right now. And we know EmitVAArg of 
> this target does the right thing on AIX after the change. 
> But for other functions, for example, getParamTypeAlignment, 
> initDwarfEHRegSizeTable... Are we sure it's doing the right thing on AIX?
> If we are not sure, is there anything we want to do (etc, put a comment on 
> where it gets used or at the function definition)? Or are we fine to just 
> leave it as it is and have a TODO in our head?
Looking at the values in `initDwarfEHRegSizeTable` it has the right sizes for 
all the registers. Even though the OS is different the underlying hardware is 
the same. I'm not sure it's something that makes sense to support for AIX 
though, in which case I think its valid to return `true` to indicate its not 
supported. 

`getParamTypeAlignment` is used only in the context of the alignment for vaarg, 
we should make sure its correct for AIX since supporting vaarg is the scope of 
this patch.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:4232
+Address PowerPC32ABIInfo::EmitVAArg(CodeGenFunction , Address VAList,
   QualType Ty) const {
+  // TODO: Add AIX ABI Info. Currently, we are relying on PowerPC32ABIInfo to

Make sure to address the formatting here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76360/new/

https://reviews.llvm.org/D76360



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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 name and comment is misleading, the class is used for both SVR4 and 
> > Darwin, and after this patch AIX. We need to fix the name comment to 
> > reflect that.
> Does this wording of the comment work? 
Missed updating the class name. Suggested: `PPC32_SVR4_ABIInfo` --> 
`PowerPC32ABIInfo`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76360/new/

https://reviews.llvm.org/D76360



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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



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 {

This name and comment is misleading, the class is used for both SVR4 and 
Darwin, and after this patch AIX. We need to fix the name comment to reflect 
that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76360/new/

https://reviews.llvm.org/D76360



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

sfertile added inline comments.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:283
   llvm::FunctionCallee atexit =
-  CGM.CreateRuntimeFunction(atexitTy, "atexit", llvm::AttributeList(),
-/*Local=*/true);
+  CGM.CreateRuntimeFunction(atexitTy, "atexit", llvm::AttributeList());
   if (llvm::Function *atexitFn = dyn_cast(atexit.getCallee()))

The default value for `Local` is false, was this change intentional? If so why 
is it needed?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74166/new/

https://reviews.llvm.org/D74166



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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 mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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

https://reviews.llvm.org/D74015



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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.
+Note that XCOFF and the Mach-O platform don't support COMDATs, and ELF and
+WebAssembly only support ``any`` as a selection kind.

really minor nit: The current wording sounds a bit odd to my ear. Maybe either 
`XCOFF and Mach-O platforms` or  `XCOFF and Mach-O don't support COMDATs, ...` 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74631/new/

https://reviews.llvm.org/D74631



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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

https://reviews.llvm.org/D74631



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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 
> > C++ ABIs, but this represents the one used in 'xlClang++'.
> You mean we have legacy XLC and XLClang++ ABI? But for static init, they have 
> same implementation. So it's not a must to point it out. 
> 
> And also AFAIK, `static init` is the only thing we will differ from Generic 
> Itanium ABI in the frontend, so basically it's the only thing we will add in 
> this ABI.
> 
> I am okay with either way with a little concern that legacy XLC user may 
> wonder is there any difference of static init implementation between XLC and 
> XLClang++ ABI if we add the comment.
Sorry, I had a matching comment on the 'XL' enum, but I must have deleted it 
accidentally before submitting. I said I agreed with using just 'XL' since 
there is only one XL C++ ABI implemented in Clang we don't have to worry about 
differentiating between the 'legacy' XL and the C++11 XL ABIs. If you did want 
to clarify then adding a comment here would be the only thing I suggest.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74015/new/

https://reviews.llvm.org/D74015



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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 in 'xlClang++'.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74015/new/

https://reviews.llvm.org/D74015



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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? This is a bit inconsistent with both the 
> > LLVM naming convention here and the name as it appears in other sources.
> There are various AIX ABI. So to distinguish the one we are implementing, we 
> choose `XL` and `Clang` as two parts of the abi name. 
> `XL` - not g++;
> `Clang` - it's a  ABI implemented in Clang;
> 
> And also `XLClang` is misleading because it represents our AIX XL C/C++ 
> compiler itself externally.
So do we need the 'Clang' part in the name? For example the ABI below is not 
`Microsoft_Clang`. Or is the `_Clang` differentiating between multiple XL ABIs?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74015/new/

https://reviews.llvm.org/D74015



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

[Sean Fertile via Phabricator via cfe-commits]

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!




Comment at: llvm/include/llvm/IR/GlobalValue.h:426
   /// Return true if this global's definition can be substituted with an
   /// *arbitrary* definition at link time.  We cannot do any IPO or inlinining
   /// across interposable call edges, since the callee can be replaced with

Minor nit: We have extended this to now determine if a definition can be 
substituted with an arbitrary definition at link time or load time. 

I would suggest changing the first `link time` to `link time or load time`, and 
changing
`since the callee can be replaced with something arbitrary at link time.` to 
`since the callee can be replaced with something arbitrary.`,


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72829/new/

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72829: Implement -fsemantic-interposition

[Sean Fertile via Phabricator via cfe-commits]

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/

https://reviews.llvm.org/D72829



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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 probably knows what exposed the BE issue 
previously).

A couple minor questions though:  Is there a way to emit a warning if the leak 
sanitizer is used on ELF powerpc64 and PIC/PIE is not also specified? Where 
should we document that the leak sanitizer needs additional options for a 
specific target?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72363/new/

https://reviews.llvm.org/D72363



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61530: Add AIX Version Macros

[Sean Fertile via Phabricator via cfe-commits]

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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59048: Add AIX Target Info

[Sean Fertile via Phabricator via cfe-commits]

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:
> > > XL on AIX emits `#define _LP64 1` in 64-bit mode and `#define _ILP32 1` 
> > > in 32-bit mode in the pre-defined macros. Is that important to capture?
> > I think so. The v16.1 XL compiler's `xlclang` also produces these.
> > 
> > ```
> > #define __LP64__ 1
> > #define _LP64 1
> > ```
> > 
> > ```
> > #define __ILP32__ 1
> > #define _ILP32 1
> > ```
> It seems GCC on AIX only defines the macros for 64-bit, and not the 32-bit 
> versions. The system headers do not appear to depend on the 32-bit versions. 
> It makes sense to start with the common intersection between the GCC and XL 
> predefined macros first. We can add `__LP64__` and `_LP64` with this patch. I 
> think we can leave more macros for later.
>It makes sense to start with the common intersection between the GCC and XL 
>predefined macros first
Agreed




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59048/new/

https://reviews.llvm.org/D59048



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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 wrote:
> > > JDevlieghere wrote:
> > > > See previous comment.
> > > It is certain that we will need MCSymbolXCOFF. But before we run into 
> > > cases where we actually need a MCSymbolXCOFF, we could use the generic 
> > > MCSymbol first for XCOFF platform. So I don't want to put a 
> > > llvm_unreachable here. 
> > Would it make sense to add an llvm_unreachable now, and the first patch 
> > that actually uses an MCSymbol stubs out the class and removes the 
> > unreachable?
> The first patch that uses MCSymbol do not necessarily need to stub out 
> MCSymbolXCOFF, as MCSymbol seems to be generic and usable until we are doing 
> some XCOFF specific things that needs to be represented by MCSymbolXCOFF. If 
> the intention is MCSymbol should never get used, and different object file 
> should have their own MCSymbolXXX class from the start, then I could add in 
> the llvm_unreachable here, and I would also propose to replace the "return 
> new (Name, *this) MCSymbol(MCSymbol::SymbolKindUnset, Name, IsTemporary);" 
> with an llvm_unreachable as well. 
> 
Ok I think falling through to create the generic MCSymbol in this patch is 
reasonable. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58930/new/

https://reviews.llvm.org/D58930



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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 that we will need MCSymbolXCOFF. But before we run into cases 
> where we actually need a MCSymbolXCOFF, we could use the generic MCSymbol 
> first for XCOFF platform. So I don't want to put a llvm_unreachable here. 
Would it make sense to add an llvm_unreachable now, and the first patch that 
actually uses an MCSymbol stubs out the class and removes the unreachable?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58930/new/

https://reviews.llvm.org/D58930



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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) + 2;
-  ElemIdx1 = (~Index & 2) >> 1;
-} else { // BigEndian
-  ElemIdx0 = (Index & 2) >> 1;
-  ElemIdx1 = 2 + (Index & 1);
-}
+unsigned ElemIdx0 = (Index & 2) >> 1;;
+unsigned ElemIdx1 = 2 + (Index & 1);;

There is an extra `;`on each line.


Repository:
  rC Clang

https://reviews.llvm.org/D49424



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41318: Start setting dso_local in clang

[Sean Fertile via Phabricator via cfe-commits]

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
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41318: Start setting dso_local in clang

[Sean Fertile via Phabricator via cfe-commits]

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 ppc, where we need to 
restore the toc pointer after the plt stubs returns to the original call site.


https://reviews.llvm.org/D41318



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41318: Start setting dso_local in clang

[Sean Fertile via Phabricator via cfe-commits]

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
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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 need mno_save_toc_indirect here too.
nit: The new option splits the mvsx and mno_vsx pair. I think we should keep 
the related options together. 


https://reviews.llvm.org/D39376



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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

https://reviews.llvm.org/D38517

Files:
  cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
  cfe/trunk/test/Driver/gold-lto-new-pass-man.c


Index: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
@@ -454,6 +454,14 @@
   CmdArgs.push_back(
   Args.MakeArgString(Twine("-plugin-opt=sample-profile=") + FName));
   }
+
+  // Need this flag to turn on new pass manager via Gold plugin.
+  if (Args.hasFlag(options::OPT_fexperimental_new_pass_manager,
+   options::OPT_fno_experimental_new_pass_manager,
+   /* Default */ false)) {
+CmdArgs.push_back("-plugin-opt=new-pass-manager");
+  }
+
 }
 
 void tools::addArchSpecificRPath(const ToolChain , const ArgList ,
Index: cfe/trunk/test/Driver/gold-lto-new-pass-man.c
===
--- cfe/trunk/test/Driver/gold-lto-new-pass-man.c
+++ cfe/trunk/test/Driver/gold-lto-new-pass-man.c
@@ -0,0 +1,7 @@
+// RUN: touch %t.o
+//
+// RUN: %clang -target ppc64le-unknown-linux -### %t.o -flto 2>&1 \
+// RUN: -Wl,-plugin-opt=foo -O3 \
+// RUN: -fexperimental-new-pass-manager \
+// RUN: | FileCheck %s
+// CHECK: "-plugin-opt=new-pass-manager"


Index: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
@@ -454,6 +454,14 @@
   CmdArgs.push_back(
   Args.MakeArgString(Twine("-plugin-opt=sample-profile=") + FName));
   }
+
+  // Need this flag to turn on new pass manager via Gold plugin.
+  if (Args.hasFlag(options::OPT_fexperimental_new_pass_manager,
+   options::OPT_fno_experimental_new_pass_manager,
+   /* Default */ false)) {
+CmdArgs.push_back("-plugin-opt=new-pass-manager");
+  }
+
 }
 
 void tools::addArchSpecificRPath(const ToolChain , const ArgList ,
Index: cfe/trunk/test/Driver/gold-lto-new-pass-man.c
===
--- cfe/trunk/test/Driver/gold-lto-new-pass-man.c
+++ cfe/trunk/test/Driver/gold-lto-new-pass-man.c
@@ -0,0 +1,7 @@
+// RUN: touch %t.o
+//
+// RUN: %clang -target ppc64le-unknown-linux -### %t.o -flto 2>&1 \
+// RUN: -Wl,-plugin-opt=foo -O3 \
+// RUN: -fexperimental-new-pass-manager \
+// RUN: | FileCheck %s
+// CHECK: "-plugin-opt=new-pass-manager"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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.


https://reviews.llvm.org/D30415



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28620: Guard __gnuc_va_list typedef

[Sean Fertile via Phabricator via cfe-commits]

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
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sean Fertile via Phabricator via cfe-commits]

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

https://reviews.llvm.org/D26546

Files:
  include/clang/Basic/BuiltinsPPC.def
  lib/CodeGen/CGBuiltin.cpp
  lib/Headers/altivec.h
  test/CodeGen/builtins-ppc-error.c
  test/CodeGen/builtins-ppc-extractword-error.c
  test/CodeGen/builtins-ppc-insertword-error.c
  test/CodeGen/builtins-ppc-p9vector.c

Index: test/CodeGen/builtins-ppc-p9vector.c
===
--- test/CodeGen/builtins-ppc-p9vector.c
+++ test/CodeGen/builtins-ppc-p9vector.c
@@ -1166,17 +1166,52 @@
 // CHECK-BE: shufflevector <8 x i16> {{.+}}, <8 x i16> {{.+}}, <8 x i32> 
 // CHECK-BE: @llvm.ppc.vsx.xvcvhpsp(<8 x i16> {{.+}})
 // CHECK-BE-NEXT: ret <4 x float>
-// CHECK-LE: shufflevector <8 x i16> {{.+}}, <8 x i16> {{.+}}, <8 x i32> 
-// CHECK-LE: @llvm.ppc.vsx.xvcvhpsp(<8 x i16> {{.+}})
-// CHECK-LE-NEXT: ret <4 x float>
+// CHECK: shufflevector <8 x i16> {{.+}}, <8 x i16> {{.+}}, <8 x i32> 
+// CHECK: @llvm.ppc.vsx.xvcvhpsp(<8 x i16> {{.+}})
+// CHECK-NEXT: ret <4 x float>
   return vec_extract_fp32_from_shorth(vusa);
 }
 vector float test115(void) {
 // CHECK-BE: shufflevector <8 x i16> {{.+}}, <8 x i16> {{.+}}, <8 x i32> 
 // CHECK-BE: @llvm.ppc.vsx.xvcvhpsp(<8 x i16> {{.+}})
 // CHECK-BE-NEXT: ret <4 x float>
-// CHECK-LE: shufflevector <8 x i16> {{.+}}, <8 x i16> {{.+}}, <8 x i32> 
-// CHECK-LE: @llvm.ppc.vsx.xvcvhpsp(<8 x i16> {{.+}})
-// CHECK-LE-NEXT: ret <4 x float>
+// CHECK: shufflevector <8 x i16> {{.+}}, <8 x i16> {{.+}}, <8 x i32> 
+// CHECK: @llvm.ppc.vsx.xvcvhpsp(<8 x i16> {{.+}})
+// CHECK-NEXT: ret <4 x float>
   return vec_extract_fp32_from_shortl(vusa);
 }
+vector unsigned char test116(void) {
+// CHECK-BE: [[T1:%.+]] = call <4 x i32> @llvm.ppc.vsx.xxinsertw(<4 x i32> {{.+}}, <2 x i64> {{.+}}, i32 7)
+// CHECK-BE-NEXT: bitcast <4 x i32> [[T1]] to <16 x i8>
+// CHECK: [[T1:%.+]] = shufflevector <2 x i64> {{.+}}, <2 x i64> {{.+}}, <2 x i32> 
+// CHECK-NEXT: [[T2:%.+]] =  bitcast <2 x i64> [[T1]] to <4 x i32>
+// CHECK-NEXT: [[T3:%.+]] = call <4 x i32> @llvm.ppc.vsx.xxinsertw(<4 x i32> [[T2]], <2 x i64> {{.+}}, i32 5)
+// CHECK-NEXT: bitcast <4 x i32> [[T3]] to <16 x i8>
+  return vec_insert4b(vuia, vuca, 7);
+}
+vector unsigned char test117(void) {
+// CHECK-BE: [[T1:%.+]] = call <4 x i32> @llvm.ppc.vsx.xxinsertw(<4 x i32> {{.+}}, <2 x i64> {{.+}}, i32 12)
+// CHECK-BE-NEXT: bitcast <4 x i32> [[T1]] to <16 x i8>
+// CHECK: [[T1:%.+]] = shufflevector <2 x i64> {{.+}}, <2 x i64> {{.+}}, <2 x i32> 
+// CHECK-NEXT: [[T2:%.+]] =  bitcast <2 x i64> [[T1]] to <4 x i32>
+// CHECK-NEXT: [[T3:%.+]] = call <4 x i32> @llvm.ppc.vsx.xxinsertw(<4 x i32> [[T2]], <2 x i64> {{.+}}, i32 0)
+// CHECK-NEXT: bitcast <4 x i32> [[T3]] to <16 x i8>
+  return vec_insert4b(vuia, vuca, 13);
+}
+vector unsigned long long test118(void) {
+// CHECK-BE: call <2 x i64> @llvm.ppc.vsx.xxextractuw(<2 x i64> {{.+}}, i32 11)
+// CHECK-BE-NEXT: ret <2 x i64>
+// CHECK: [[T1:%.+]] = call <2 x i64> @llvm.ppc.vsx.xxextractuw(<2 x i64> {{.+}}, i32 1)
+// CHECK-NEXT: shufflevector <2 x i64> [[T1]], <2 x i64> [[T1]], <2 x i32> 
+// CHECK-NEXT: ret <2 x i64>
+  return vec_extract4b(vuca, 11);
+}
+vector unsigned long long test119(void) {
+// CHECK-BE: call <2 x i64> @llvm.ppc.vsx.xxextractuw(<2 x i64> {{.+}}, i32 0)
+// CHECK-BE-NEXT: ret <2 x i64>
+// CHECK: [[T1:%.+]] = call <2 x i64> @llvm.ppc.vsx.xxextractuw(<2 x i64> {{.+}}, i32 12)
+// CHECK-NEXT: shufflevector <2 x i64> [[T1]], <2 x i64> [[T1]], <2 x i32> 
+// CHECK-NEXT: ret <2 x i64>
+  return vec_extract4b(vuca, -5);
+}
+
Index: test/CodeGen/builtins-ppc-insertword-error.c
===
--- /dev/null
+++ test/CodeGen/builtins-ppc-insertword-error.c
@@ -0,0 +1,16 @@
+// REQUIRES: powerpc-registered-target
+// XFAIL: powerpc
+
+// RUN: %clang -faltivec -target powerpc64le-unknown-unknown -mcpu=power8 \
+// RUN: -Wall -Werror -c %s
+
+// RUN: %clang -faltivec -target powerpc64-unknown-unknown -mcpu=power8 \
+// RUN: -Wall -Werror -c %s
+
+// expect to fail  with diagnostic: "cannot compile this builtin function yet"
+extern vector signed int vsi;
+extern vector unsigned char vuc;
+
+vector  unsigned char testInsertWord(void) {
+  return __builtin_vsx_insertword(vsi, vuc, 0);
+}
Index: test/CodeGen/builtins-ppc-extractword-error.c
===
--- /dev/null
+++ test/CodeGen/builtins-ppc-extractword-error.c
@@ -0,0 +1,15 @@
+// REQUIRES: powerpc-registered-target
+// XFAIL: powerpc
+
+// RUN: %clang -faltivec -target powerpc64le-unknown-unknown  -mcpu=power8 \
+// RUN: -Wall -Wextra -c %s
+// RUN: %clang -faltivec -target powerpc64-unknown-unknown  -mcpu=power8 \
+// RUN: 

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

[Sean Fertile via Phabricator via cfe-commits]

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.


Repository:
  rL LLVM

https://reviews.llvm.org/D26546

Files:
  include/clang/Basic/BuiltinsPPC.def
  lib/CodeGen/CGBuiltin.cpp
  lib/Headers/altivec.h
  test/CodeGen/builtins-ppc-error.c
  test/CodeGen/builtins-ppc-extractword-error.c
  test/CodeGen/builtins-ppc-insertword-error.c
  test/CodeGen/builtins-ppc-p9vector.c

Index: test/CodeGen/builtins-ppc-p9vector.c
===
--- test/CodeGen/builtins-ppc-p9vector.c
+++ test/CodeGen/builtins-ppc-p9vector.c
@@ -1166,17 +1166,41 @@
 // CHECK-BE: shufflevector <8 x i16> {{.+}}, <8 x i16> {{.+}}, <8 x i32> 
 // CHECK-BE: @llvm.ppc.vsx.xvcvhpsp(<8 x i16> {{.+}})
 // CHECK-BE-NEXT: ret <4 x float>
-// CHECK-LE: shufflevector <8 x i16> {{.+}}, <8 x i16> {{.+}}, <8 x i32> 
-// CHECK-LE: @llvm.ppc.vsx.xvcvhpsp(<8 x i16> {{.+}})
-// CHECK-LE-NEXT: ret <4 x float>
+// CHECK: shufflevector <8 x i16> {{.+}}, <8 x i16> {{.+}}, <8 x i32> 
+// CHECK: @llvm.ppc.vsx.xvcvhpsp(<8 x i16> {{.+}})
+// CHECK-NEXT: ret <4 x float>
   return vec_extract_fp32_from_shorth(vusa);
 }
 vector float test115(void) {
 // CHECK-BE: shufflevector <8 x i16> {{.+}}, <8 x i16> {{.+}}, <8 x i32> 
 // CHECK-BE: @llvm.ppc.vsx.xvcvhpsp(<8 x i16> {{.+}})
 // CHECK-BE-NEXT: ret <4 x float>
-// CHECK-LE: shufflevector <8 x i16> {{.+}}, <8 x i16> {{.+}}, <8 x i32> 
-// CHECK-LE: @llvm.ppc.vsx.xvcvhpsp(<8 x i16> {{.+}})
-// CHECK-LE-NEXT: ret <4 x float>
+// CHECK: shufflevector <8 x i16> {{.+}}, <8 x i16> {{.+}}, <8 x i32> 
+// CHECK: @llvm.ppc.vsx.xvcvhpsp(<8 x i16> {{.+}})
+// CHECK-NEXT: ret <4 x float>
   return vec_extract_fp32_from_shortl(vusa);
 }
+vector unsigned char test116(void) {
+// CHECK-BE: [[T1:%.+]] = call <4 x i32> @llvm.ppc.vsx.xxinsertw(<4 x i32> {{.+}}, <2 x i64> {{.+}}, i32 7)
+// CHECK-BE-NEXT: bitcast <4 x i32> [[T1]] to <16 x i8>
+// CHECK: [[T1:%.+]] = shufflevector <2 x i64> {{.+}}, <2 x i64> {{.+}}, <2 x i32> 
+// CHECK-NEXT: [[T2:%.+]] = call <4 x i32> @llvm.ppc.vsx.xxinsertw(<4 x i32> {{.+}}, <2 x i64> [[T1]], i32 5)
+// CHECK-NEXT: bitcast <4 x i32> [[T2]] to <16 x i8>
+  return vec_insert4b(vuia, vuca, 7);
+}
+vector unsigned char test117(void) {
+// CHECK-BE: [[T1:%.+]] = call <4 x i32> @llvm.ppc.vsx.xxinsertw(<4 x i32> {{.+}}, <2 x i64> {{.+}}, i32 5)
+// CHECK-BE-NEXT: bitcast <4 x i32> [[T1]] to <16 x i8>
+// CHECK: [[T1:%.+]] = shufflevector <2 x i64> {{.+}}, <2 x i64> {{.+}}, <2 x i32> 
+// CHECK-NEXT: [[T2:%.+]] = call <4 x i32> @llvm.ppc.vsx.xxinsertw(<4 x i32> {{.+}}, <2 x i64> [[T1]], i32 7)
+// CHECK-NEXT: bitcast <4 x i32> [[T2]] to <16 x i8>
+  return vec_insert4b(vsia, vuca, 5);
+}
+vector unsigned long long test118(void) {
+// CHECK-BE: call <2 x i64> @llvm.ppc.vsx.xxextractuw(<2 x i64> {{.+}}, i32 11)
+// CHECK-BE-NEXT: ret <2 x i64>
+// CHECK: [[T1:%.+]] = call <2 x i64> @llvm.ppc.vsx.xxextractuw(<2 x i64> {{.+}}, i32 1)
+// CHECK-NEXT: shufflevector <2 x i64> [[T1]], <2 x i64> [[T1]], <2 x i32> 
+// CHECK-NEXT: ret <2 x i64>
+  return vec_extract4b(vuca, 11);
+}
Index: test/CodeGen/builtins-ppc-insertword-error.c
===
--- /dev/null
+++ test/CodeGen/builtins-ppc-insertword-error.c
@@ -0,0 +1,16 @@
+// REQUIRES: powerpc-registered-target
+// XFAIL: powerpc
+
+// RUN: %clang -faltivec -target powerpc64le-unknown-unknown -mcpu=power8 \
+// RUN: -Wall -Werror -c %s
+
+// RUN: %clang -faltivec -target powerpc64-unknown-unknown -mcpu=power8 \
+// RUN: -Wall -Werror -c %s
+
+// expect to fail  with diagnostic: "cannot compile this builtin function yet"
+extern vector signed int vsi;
+extern vector unsigned char vuc;
+
+vector  unsigned char testInsertWord(void) {
+  return __builtin_vsx_insertword(vsi, vuc, 0);
+}
Index: test/CodeGen/builtins-ppc-extractword-error.c
===
--- /dev/null
+++ test/CodeGen/builtins-ppc-extractword-error.c
@@ -0,0 +1,15 @@
+// REQUIRES: powerpc-registered-target
+// XFAIL: powerpc
+
+// RUN: %clang -faltivec -target powerpc64le-unknown-unknown  -mcpu=power8 \
+// RUN: -Wall -Wextra -c %s
+// RUN: %clang -faltivec -target powerpc64-unknown-unknown  -mcpu=power8 \
+// RUN: -Wall -Wextra -c %s
+
+// Expect the compile to fail ith "cannot compile this builtin function yet"
+extern vector signed int vsi;
+extern vector unsigned char vuc;
+
+vector unsigned long long testExtractWord(void) {
+  return  __builtin_vsx_extractuword(vuc, 12);
+}
Index: test/CodeGen/builtins-ppc-error.c
===
--- /dev/null
+++ test/CodeGen/builtins-ppc-error.c
@@ -0,0 +1,20 @@
+// REQUIRES: powerpc-registered-target
+
+// RUN: 

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

[Sean Fertile via Phabricator via cfe-commits]

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 defined in "clang/Config/config.h" 
which isn't meant to be included in other headers.


Repository:
  rL LLVM

https://reviews.llvm.org/D25263



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits