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

[Xiangling Liao via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8bee52bdb54a: [AIX][Frontend] C++ ABI customizations for AIX 
boilerplate (authored by Xiangling_L).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74015

Files:
  clang/include/clang/Basic/TargetCXXABI.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGen/static-init.cpp

Index: clang/test/CodeGen/static-init.cpp
===
--- /dev/null
+++ clang/test/CodeGen/static-init.cpp
@@ -0,0 +1,12 @@
+// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
+// RUN: 2>&1 | FileCheck %s
+
+// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
+// RUN: 2>&1 | FileCheck %s
+
+struct test {
+  test();
+  ~test();
+} t;
+
+// CHECK: error in backend: Static initialization has not been implemented on XL ABI yet.
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -516,6 +516,16 @@
   }
   bool canCallMismatchedFunctionType() const override { return false; }
 };
+
+class XLCXXABI final : public ItaniumCXXABI {
+public:
+  explicit XLCXXABI(CodeGen::CodeGenModule )
+  : ItaniumCXXABI(CGM) {}
+
+  void registerGlobalDtor(CodeGenFunction , const VarDecl ,
+  llvm::FunctionCallee dtor,
+  llvm::Constant *addr) override;
+};
 }
 
 CodeGen::CGCXXABI *CodeGen::CreateItaniumCXXABI(CodeGenModule ) {
@@ -546,6 +556,9 @@
   case TargetCXXABI::WebAssembly:
 return new WebAssemblyCXXABI(CGM);
 
+  case TargetCXXABI::XL:
+return new XLCXXABI(CGM);
+
   case TargetCXXABI::GenericItanium:
 if (CGM.getContext().getTargetInfo().getTriple().getArch()
 == llvm::Triple::le32) {
@@ -4407,3 +4420,11 @@
 NormalCleanup, cast(CGF.CurrentFuncletPad));
   ItaniumCXXABI::emitBeginCatch(CGF, C);
 }
+
+/// Register a global destructor as best as we know how.
+void XLCXXABI::registerGlobalDtor(CodeGenFunction , const VarDecl ,
+  llvm::FunctionCallee dtor,
+  llvm::Constant *addr) {
+  llvm::report_fatal_error("Static initialization has not been implemented on"
+   " XL ABI yet.");
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -83,6 +83,7 @@
   case TargetCXXABI::GenericMIPS:
   case TargetCXXABI::GenericItanium:
   case TargetCXXABI::WebAssembly:
+  case TargetCXXABI::XL:
 return CreateItaniumCXXABI(CGM);
   case TargetCXXABI::Microsoft:
 return CreateMicrosoftCXXABI(CGM);
Index: clang/lib/Basic/Targets/OSTargets.h
===
--- clang/lib/Basic/Targets/OSTargets.h
+++ clang/lib/Basic/Targets/OSTargets.h
@@ -706,6 +706,8 @@
 public:
   AIXTargetInfo(const llvm::Triple , const TargetOptions )
   : OSTargetInfo(Triple, Opts) {
+this->TheCXXABI.set(TargetCXXABI::XL);
+
 if (this->PointerWidth == 64) {
   this->WCharType = this->UnsignedInt;
 } else {
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -874,6 +874,7 @@
   case TargetCXXABI::GenericMIPS:
   case TargetCXXABI::GenericItanium:
   case TargetCXXABI::WebAssembly:
+  case TargetCXXABI::XL:
 return CreateItaniumCXXABI(*this);
   case TargetCXXABI::Microsoft:
 return CreateMicrosoftCXXABI(*this);
@@ -10253,6 +10254,7 @@
   case TargetCXXABI::iOS64:
   case TargetCXXABI::WebAssembly:
   case TargetCXXABI::WatchOS:
+  case TargetCXXABI::XL:
 return ItaniumMangleContext::create(*this, getDiagnostics());
   case TargetCXXABI::Microsoft:
 return MicrosoftMangleContext::create(*this, getDiagnostics());
Index: clang/include/clang/Basic/TargetCXXABI.h
===
--- clang/include/clang/Basic/TargetCXXABI.h
+++ clang/include/clang/Basic/TargetCXXABI.h
@@ -109,6 +109,13 @@
 ///   - constructors and destructors return 'this', as in ARM.
 Fuchsia,
 
+/// The XL ABI is the ABI used by IBM xlclang compiler and is a modified
+/// version of the Itanium ABI.
+///
+/// The relevant changes from the Itanium ABI are:
+///   - static initialization is adjusted to use sinit and sterm functions;
+XL,
+
 /// The Microsoft ABI is the ABI used by Microsoft Visual Studio (and
 /// compatible compilers).
 ///
@@ -148,6 +155,7 @@
 case WatchOS:
 case 

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

[Jason Liu via Phabricator via cfe-commits]

jasonliu added a comment.

Just kindly reminder that you might want to update the Summary in the commit 
message to reflect the new name we picked.


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

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L updated this revision to Diff 245844.
Xiangling_L marked an inline comment as done.
Xiangling_L added a comment.

Update the comment about ABI description;


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74015

Files:
  clang/include/clang/Basic/TargetCXXABI.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGen/static-init.cpp

Index: clang/test/CodeGen/static-init.cpp
===
--- /dev/null
+++ clang/test/CodeGen/static-init.cpp
@@ -0,0 +1,12 @@
+// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
+// RUN: 2>&1 | FileCheck %s
+
+// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
+// RUN: 2>&1 | FileCheck %s
+
+struct test {
+  test();
+  ~test();
+} t;
+
+// CHECK: error in backend: Static initialization has not been implemented on XL ABI yet.
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -516,6 +516,16 @@
   }
   bool canCallMismatchedFunctionType() const override { return false; }
 };
+
+class XLCXXABI final : public ItaniumCXXABI {
+public:
+  explicit XLCXXABI(CodeGen::CodeGenModule )
+  : ItaniumCXXABI(CGM) {}
+
+  void registerGlobalDtor(CodeGenFunction , const VarDecl ,
+  llvm::FunctionCallee dtor,
+  llvm::Constant *addr) override;
+};
 }
 
 CodeGen::CGCXXABI *CodeGen::CreateItaniumCXXABI(CodeGenModule ) {
@@ -546,6 +556,9 @@
   case TargetCXXABI::WebAssembly:
 return new WebAssemblyCXXABI(CGM);
 
+  case TargetCXXABI::XL:
+return new XLCXXABI(CGM);
+
   case TargetCXXABI::GenericItanium:
 if (CGM.getContext().getTargetInfo().getTriple().getArch()
 == llvm::Triple::le32) {
@@ -4407,3 +4420,11 @@
 NormalCleanup, cast(CGF.CurrentFuncletPad));
   ItaniumCXXABI::emitBeginCatch(CGF, C);
 }
+
+/// Register a global destructor as best as we know how.
+void XLCXXABI::registerGlobalDtor(CodeGenFunction , const VarDecl ,
+  llvm::FunctionCallee dtor,
+  llvm::Constant *addr) {
+  llvm::report_fatal_error("Static initialization has not been implemented on"
+   " XL ABI yet.");
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -83,6 +83,7 @@
   case TargetCXXABI::GenericMIPS:
   case TargetCXXABI::GenericItanium:
   case TargetCXXABI::WebAssembly:
+  case TargetCXXABI::XL:
 return CreateItaniumCXXABI(CGM);
   case TargetCXXABI::Microsoft:
 return CreateMicrosoftCXXABI(CGM);
Index: clang/lib/Basic/Targets/OSTargets.h
===
--- clang/lib/Basic/Targets/OSTargets.h
+++ clang/lib/Basic/Targets/OSTargets.h
@@ -706,6 +706,8 @@
 public:
   AIXTargetInfo(const llvm::Triple , const TargetOptions )
   : OSTargetInfo(Triple, Opts) {
+this->TheCXXABI.set(TargetCXXABI::XL);
+
 if (this->PointerWidth == 64) {
   this->WCharType = this->UnsignedInt;
 } else {
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -874,6 +874,7 @@
   case TargetCXXABI::GenericMIPS:
   case TargetCXXABI::GenericItanium:
   case TargetCXXABI::WebAssembly:
+  case TargetCXXABI::XL:
 return CreateItaniumCXXABI(*this);
   case TargetCXXABI::Microsoft:
 return CreateMicrosoftCXXABI(*this);
@@ -10253,6 +10254,7 @@
   case TargetCXXABI::iOS64:
   case TargetCXXABI::WebAssembly:
   case TargetCXXABI::WatchOS:
+  case TargetCXXABI::XL:
 return ItaniumMangleContext::create(*this, getDiagnostics());
   case TargetCXXABI::Microsoft:
 return MicrosoftMangleContext::create(*this, getDiagnostics());
Index: clang/include/clang/Basic/TargetCXXABI.h
===
--- clang/include/clang/Basic/TargetCXXABI.h
+++ clang/include/clang/Basic/TargetCXXABI.h
@@ -109,6 +109,13 @@
 ///   - constructors and destructors return 'this', as in ARM.
 Fuchsia,
 
+/// The XL ABI is the ABI used by IBM xlclang compiler and is a modified
+/// version of the Itanium ABI.
+///
+/// The relevant changes from the Itanium ABI are:
+///   - static initialization is adjusted to use sinit and sterm functions;
+XL,
+
 /// The Microsoft ABI is the ABI used by Microsoft Visual Studio (and
 /// compatible compilers).
 ///
@@ -148,6 +155,7 @@
 case WatchOS:
 case GenericMIPS:
 case 

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

[Chris Bowler via Phabricator via cfe-commits]

cebowleratibm added a comment.

In D74015#1882933 , @sfertile wrote:

> Patch LGTM as far a formatting/naming/testing etc. C++ specifics is outside 
> my wheelhouse though, so I can't confirm things like the tail padding rules 
> are correct for AIX. Because of that I'm not comfortable being the one to 
> accept the patch.


I checked the IBM xlclang source and can confirm the implementation of 
getTailPaddingUseRules is consistent with the proposed patch.  Sean, does that 
address your concern?

The only change I would like to see is an update to the "XL" enumerator comment 
to be more descriptive of what the "XL" ABI is.




Comment at: clang/include/clang/Basic/TargetCXXABI.h:116
+///   - static initialization is adjusted to use sinit and sterm functions;
+XL_Clang,
+

cebowleratibm wrote:
> cebowleratibm wrote:
> > sfertile wrote:
> > > 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?
> > I suspect the concern is that "XL" ABI is ambiguious between legacy xlC and 
> > xlclang++.  The two differ at the C++11 language level so perhaps it makes 
> > sense to have "XLC++11"?  (and theoretically just "XL" if we ever decide 
> > xlC)
> Another suggestion: "IBMXL" or "IBMXLC++11"
To summarize some off phabricator discussion: we reached consensus on "XL".  It 
was not felt that "IBM" needs to be in the name and that a comment by the 
enumerator definition is sufficient.

Note that "XL" refers to ABI compatibiliity with the AIX xlclang compiler and 
not xlC.  If we need xlC compatibility that would be yet another ABI to add at 
another time.

The comment should be updated to provide more background on what the "XL" ABI 
is.


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] D74015: [AIX][Frontend] C++ ABI customizations for AIX boilerplate

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L added a comment.

In D74015#1880847 , @cebowleratibm 
wrote:

> From my perspective, the only issue holding this up is settling on the name.  
> I'd like to hammer that out and get this committed.


It looks everyone agrees on `XL` so far. As for your other suggestion `IBMXL`, 
if your intention is to clarify this ABI is IBM product specific, I think a 
good practice we can do is to leave a comment when we defined it as Microsoft 
does.


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

[Chris Bowler via Phabricator via cfe-commits]

cebowleratibm added a comment.

From my perspective, the only issue holding this up is settling on the name.  
I'd like to hammer that out and get this committed.




Comment at: clang/include/clang/Basic/TargetCXXABI.h:116
+///   - static initialization is adjusted to use sinit and sterm functions;
+XL_Clang,
+

cebowleratibm wrote:
> sfertile wrote:
> > 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?
> I suspect the concern is that "XL" ABI is ambiguious between legacy xlC and 
> xlclang++.  The two differ at the C++11 language level so perhaps it makes 
> sense to have "XLC++11"?  (and theoretically just "XL" if we ever decide xlC)
Another suggestion: "IBMXL" or "IBMXLC++11"


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

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L marked an inline comment as done.
Xiangling_L added a comment.

ping.


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

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L marked 3 inline comments as done.
Xiangling_L added inline comments.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:520
+
+class XLCXXABI final : public ItaniumCXXABI {
+public:

sfertile wrote:
> 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.
I see. Thank you for your clarification.


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:

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

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L marked 3 inline comments as done.
Xiangling_L added inline comments.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:520
+
+class XLCXXABI final : public ItaniumCXXABI {
+public:

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.


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

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L updated this revision to Diff 244476.
Xiangling_L marked 4 inline comments as done.
Xiangling_L added a comment.

Adjust ABI name to `XL`;
Simplify the testcase;


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

https://reviews.llvm.org/D74015

Files:
  clang/include/clang/Basic/TargetCXXABI.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGen/static-init.cpp

Index: clang/test/CodeGen/static-init.cpp
===
--- /dev/null
+++ clang/test/CodeGen/static-init.cpp
@@ -0,0 +1,12 @@
+// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
+// RUN: 2>&1 | FileCheck %s
+
+// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
+// RUN: 2>&1 | FileCheck %s
+
+struct test {
+  test();
+  ~test();
+} t;
+
+// CHECK: error in backend: Static initialization has not been implemented on XL ABI yet.
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -516,6 +516,16 @@
   }
   bool canCallMismatchedFunctionType() const override { return false; }
 };
+
+class XLCXXABI final : public ItaniumCXXABI {
+public:
+  explicit XLCXXABI(CodeGen::CodeGenModule )
+  : ItaniumCXXABI(CGM) {}
+
+  void registerGlobalDtor(CodeGenFunction , const VarDecl ,
+  llvm::FunctionCallee dtor,
+  llvm::Constant *addr) override;
+};
 }
 
 CodeGen::CGCXXABI *CodeGen::CreateItaniumCXXABI(CodeGenModule ) {
@@ -546,6 +556,9 @@
   case TargetCXXABI::WebAssembly:
 return new WebAssemblyCXXABI(CGM);
 
+  case TargetCXXABI::XL:
+return new XLCXXABI(CGM);
+
   case TargetCXXABI::GenericItanium:
 if (CGM.getContext().getTargetInfo().getTriple().getArch()
 == llvm::Triple::le32) {
@@ -4407,3 +4420,11 @@
 NormalCleanup, cast(CGF.CurrentFuncletPad));
   ItaniumCXXABI::emitBeginCatch(CGF, C);
 }
+
+/// Register a global destructor as best as we know how.
+void XLCXXABI::registerGlobalDtor(CodeGenFunction , const VarDecl ,
+  llvm::FunctionCallee dtor,
+  llvm::Constant *addr) {
+  llvm::report_fatal_error("Static initialization has not been implemented on"
+   " XL ABI yet.");
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -83,6 +83,7 @@
   case TargetCXXABI::GenericMIPS:
   case TargetCXXABI::GenericItanium:
   case TargetCXXABI::WebAssembly:
+  case TargetCXXABI::XL:
 return CreateItaniumCXXABI(CGM);
   case TargetCXXABI::Microsoft:
 return CreateMicrosoftCXXABI(CGM);
Index: clang/lib/Basic/Targets/OSTargets.h
===
--- clang/lib/Basic/Targets/OSTargets.h
+++ clang/lib/Basic/Targets/OSTargets.h
@@ -706,6 +706,8 @@
 public:
   AIXTargetInfo(const llvm::Triple , const TargetOptions )
   : OSTargetInfo(Triple, Opts) {
+this->TheCXXABI.set(TargetCXXABI::XL);
+
 if (this->PointerWidth == 64) {
   this->WCharType = this->UnsignedInt;
 } else {
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -874,6 +874,7 @@
   case TargetCXXABI::GenericMIPS:
   case TargetCXXABI::GenericItanium:
   case TargetCXXABI::WebAssembly:
+  case TargetCXXABI::XL:
 return CreateItaniumCXXABI(*this);
   case TargetCXXABI::Microsoft:
 return CreateMicrosoftCXXABI(*this);
@@ -10251,6 +10252,7 @@
   case TargetCXXABI::iOS64:
   case TargetCXXABI::WebAssembly:
   case TargetCXXABI::WatchOS:
+  case TargetCXXABI::XL:
 return ItaniumMangleContext::create(*this, getDiagnostics());
   case TargetCXXABI::Microsoft:
 return MicrosoftMangleContext::create(*this, getDiagnostics());
Index: clang/include/clang/Basic/TargetCXXABI.h
===
--- clang/include/clang/Basic/TargetCXXABI.h
+++ clang/include/clang/Basic/TargetCXXABI.h
@@ -109,6 +109,12 @@
 ///   - constructors and destructors return 'this', as in ARM.
 Fuchsia,
 
+/// The XL ABI is a modified version of the Itanium ABI.
+///
+/// The relevant changes from the Itanium ABI are:
+///   - static initialization is adjusted to use sinit and sterm functions;
+XL,
+
 /// The Microsoft ABI is the ABI used by Microsoft Visual Studio (and
 /// compatible compilers).
 ///
@@ -148,6 +154,7 @@
 case WatchOS:
 case GenericMIPS:
 case WebAssembly:
+case XL:
   return true;
 
 case Microsoft:
@@ -168,6 +175,7 @@
   

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

[Chris Bowler via Phabricator via cfe-commits]

cebowleratibm added a comment.

Looks fine, but we need to settle on the name for the ABI.  My preference would 
be "XLC++11", or perhaps "XLCXX11" (I propose the latter because of the common 
reference CXXABI.)




Comment at: clang/include/clang/Basic/TargetCXXABI.h:116
+///   - static initialization is adjusted to use sinit and sterm functions;
+XL_Clang,
+

sfertile wrote:
> 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?
I suspect the concern is that "XL" ABI is ambiguious between legacy xlC and 
xlclang++.  The two differ at the C++11 language level so perhaps it makes 
sense to have "XLC++11"?  (and theoretically just "XL" if we ever decide xlC)



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4428
+   llvm::Constant *addr) {
+  llvm::report_fatal_error("Static initialization has not been fully"
+   " implemented on XL_Clang ABI yet.");

Omit "fully".  Fix the name XL_Clang when we've settled on a name.



Comment at: clang/test/CodeGen/static-init.cpp:9
+public:
+test(int c) {a = c;}
+~test() {a = 0;}

Nit: formatting.  

The test is fine but I'd usually write this test even more briefly:
struct S { ~S(); } s;


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 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] D74015: [AIX][Frontend] C++ ABI customizations for AIX boilerplate

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L marked 4 inline comments as done.
Xiangling_L added inline comments.



Comment at: clang/include/clang/Basic/TargetCXXABI.h:116
+///   - static initialization is adjusted to use sinit and sterm functions;
+XL_Clang,
+

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.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:520
+
+class XLClangCXXABI final : public ItaniumCXXABI {
+public:

stevewan wrote:
> The class name here is inconsistent with how our ABI kind was called 
> previously, as David pointed out. Maybe rename the ABI kind `XLClang`? 
I understand you concerns, and please see my replies above.


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

[Steven Wan via Phabricator via cfe-commits]

stevewan added inline comments.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:520
+
+class XLClangCXXABI final : public ItaniumCXXABI {
+public:

The class name here is inconsistent with how our ABI kind was called 
previously, as David pointed out. Maybe rename the ABI kind `XLClang`? 


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

[David Tenty via Phabricator via cfe-commits]

daltenty added inline comments.



Comment at: clang/include/clang/Basic/TargetCXXABI.h:116
+///   - static initialization is adjusted to use sinit and sterm functions;
+XL_Clang,
+

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.


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

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L created this revision.
Xiangling_L added reviewers: hubert.reinterpretcast, cebowleratibm, 
yusra.syeda, sfertile, jasonliu, xingxue.
Xiangling_L added a project: LLVM.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This PR enables **XL_Clang** C++ ABI in frontend AST to IR codegen. And it is 
driven by static init work. The current kind in Clang by default is Generic 
Itanium, which has different behavior on static init with Clangtana on AIX.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74015

Files:
  clang/include/clang/Basic/TargetCXXABI.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Basic/Targets/OSTargets.h
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGen/static-init.cpp

Index: clang/test/CodeGen/static-init.cpp
===
--- /dev/null
+++ clang/test/CodeGen/static-init.cpp
@@ -0,0 +1,16 @@
+// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
+// RUN: 2>&1 | FileCheck %s
+
+// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
+// RUN: 2>&1 | FileCheck %s
+class test {
+   int a;
+public:
+test(int c) {a = c;}
+~test() {a = 0;}
+};
+
+test t(1);
+
+
+; CHECK: error in backend: Static initialization has not been fully implemented on XL_Clang ABI yet.
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -516,6 +516,16 @@
   }
   bool canCallMismatchedFunctionType() const override { return false; }
 };
+
+class XLClangCXXABI final : public ItaniumCXXABI {
+public:
+  explicit XLClangCXXABI(CodeGen::CodeGenModule )
+  : ItaniumCXXABI(CGM) {}
+
+  void registerGlobalDtor(CodeGenFunction , const VarDecl ,
+  llvm::FunctionCallee dtor,
+  llvm::Constant *addr) override;
+};
 }
 
 CodeGen::CGCXXABI *CodeGen::CreateItaniumCXXABI(CodeGenModule ) {
@@ -546,6 +556,9 @@
   case TargetCXXABI::WebAssembly:
 return new WebAssemblyCXXABI(CGM);
 
+  case TargetCXXABI::XL_Clang:
+return new XLClangCXXABI(CGM);
+
   case TargetCXXABI::GenericItanium:
 if (CGM.getContext().getTargetInfo().getTriple().getArch()
 == llvm::Triple::le32) {
@@ -4407,3 +4420,11 @@
 NormalCleanup, cast(CGF.CurrentFuncletPad));
   ItaniumCXXABI::emitBeginCatch(CGF, C);
 }
+
+/// Register a global destructor as best as we know how.
+void XLClangCXXABI::registerGlobalDtor(CodeGenFunction , const VarDecl ,
+   llvm::FunctionCallee dtor,
+   llvm::Constant *addr) {
+  llvm::report_fatal_error("Static initialization has not been fully"
+   " implemented on XL_Clang ABI yet.");
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -83,6 +83,7 @@
   case TargetCXXABI::GenericMIPS:
   case TargetCXXABI::GenericItanium:
   case TargetCXXABI::WebAssembly:
+  case TargetCXXABI::XL_Clang:
 return CreateItaniumCXXABI(CGM);
   case TargetCXXABI::Microsoft:
 return CreateMicrosoftCXXABI(CGM);
Index: clang/lib/Basic/Targets/OSTargets.h
===
--- clang/lib/Basic/Targets/OSTargets.h
+++ clang/lib/Basic/Targets/OSTargets.h
@@ -706,6 +706,8 @@
 public:
   AIXTargetInfo(const llvm::Triple , const TargetOptions )
   : OSTargetInfo(Triple, Opts) {
+this->TheCXXABI.set(TargetCXXABI::XL_Clang);
+
 if (this->PointerWidth == 64) {
   this->WCharType = this->UnsignedInt;
 } else {
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -874,6 +874,7 @@
   case TargetCXXABI::GenericMIPS:
   case TargetCXXABI::GenericItanium:
   case TargetCXXABI::WebAssembly:
+  case TargetCXXABI::XL_Clang:
 return CreateItaniumCXXABI(*this);
   case TargetCXXABI::Microsoft:
 return CreateMicrosoftCXXABI(*this);
@@ -10251,6 +10252,7 @@
   case TargetCXXABI::iOS64:
   case TargetCXXABI::WebAssembly:
   case TargetCXXABI::WatchOS:
+  case TargetCXXABI::XL_Clang:
 return ItaniumMangleContext::create(*this, getDiagnostics());
   case TargetCXXABI::Microsoft:
 return MicrosoftMangleContext::create(*this, getDiagnostics());
Index: clang/include/clang/Basic/TargetCXXABI.h
===
--- clang/include/clang/Basic/TargetCXXABI.h
+++ clang/include/clang/Basic/TargetCXXABI.h
@@ -109,6 +109,12 @@
 ///   - constructors and destructors return 'this', as in ARM.
 Fuchsia,
 
+/// The XL_Clang ABI is a modified version of the Itanium ABI.
+///
+