[PATCH] D58930: Add XCOFF triple object format type for AIX
jasonliu created this revision.
jasonliu added reviewers: hubert.reinterpretcast, sfertile, chandlerc,
apaprocki.
Herald added subscribers: llvm-commits, lldb-commits, cfe-commits, dexonsmith,
aheejin, hiraditya, javed.absar.
Herald added projects: clang, LLDB, LLVM.
This patch adds an XCOFF triple object format type into LLVM. This XCOFF triple
object file type will be used later by object file and assembly generation for
the AIX platform.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D58930
Files:
clang/lib/CodeGen/BackendUtil.cpp
clang/lib/CodeGen/CodeGenModule.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
llvm/include/llvm/ADT/Triple.h
llvm/include/llvm/MC/MCObjectFileInfo.h
llvm/lib/MC/MCContext.cpp
llvm/lib/MC/MCObjectFileInfo.cpp
llvm/lib/MC/MCParser/AsmParser.cpp
llvm/lib/Support/Triple.cpp
llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
llvm/unittests/ADT/TripleTest.cpp
Index: llvm/unittests/ADT/TripleTest.cpp
===
--- llvm/unittests/ADT/TripleTest.cpp
+++ llvm/unittests/ADT/TripleTest.cpp
@@ -1258,6 +1258,11 @@
EXPECT_EQ(Triple::Wasm,
Triple("wasm64-unknown-wasi-musl-wasm").getObjectFormat());
+ EXPECT_EQ(Triple::XCOFF, Triple("powerpc-ibm-aix").getObjectFormat());
+ EXPECT_EQ(Triple::XCOFF, Triple("powerpc64-ibm-aix").getObjectFormat());
+ EXPECT_EQ(Triple::XCOFF, Triple("powerpc---xcoff").getObjectFormat());
+ EXPECT_EQ(Triple::XCOFF, Triple("powerpc64---xcoff").getObjectFormat());
+
Triple MSVCNormalized(Triple::normalize("i686-pc-windows-msvc-elf"));
EXPECT_EQ(Triple::ELF, MSVCNormalized.getObjectFormat());
@@ -1276,6 +1281,9 @@
T.setObjectFormat(Triple::MachO);
EXPECT_EQ(Triple::MachO, T.getObjectFormat());
+
+ T.setObjectFormat(Triple::XCOFF);
+ EXPECT_EQ(Triple::XCOFF, T.getObjectFormat());
}
TEST(TripleTest, NormalizeWindows) {
Index: llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
===
--- llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
+++ llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
@@ -5594,6 +5594,9 @@
case MCObjectFileInfo::IsWasm:
CurrentFormat = WASM;
break;
+ case MCObjectFileInfo::IsXCOFF:
+llvm_unreachable("unexpected object format");
+break;
}
if (~Prefix->SupportedFormats & CurrentFormat) {
Index: llvm/lib/Support/Triple.cpp
===
--- llvm/lib/Support/Triple.cpp
+++ llvm/lib/Support/Triple.cpp
@@ -534,6 +534,9 @@
static Triple::ObjectFormatType parseFormat(StringRef EnvironmentName) {
return StringSwitch(EnvironmentName)
+// FIXME: We have to put XCOFF before COFF;
+// perhaps an order-independent pattern matching is desired?
+.EndsWith("xcoff", Triple::XCOFF)
.EndsWith("coff", Triple::COFF)
.EndsWith("elf", Triple::ELF)
.EndsWith("macho", Triple::MachO)
@@ -622,6 +625,7 @@
case Triple::ELF: return "elf";
case Triple::MachO: return "macho";
case Triple::Wasm: return "wasm";
+ case Triple::XCOFF: return "xcoff";
}
llvm_unreachable("unknown object format type");
}
@@ -686,6 +690,8 @@
case Triple::ppc64:
if (T.isOSDarwin())
return Triple::MachO;
+else if (T.isOSAIX())
+ return Triple::XCOFF;
return Triple::ELF;
case Triple::wasm32:
Index: llvm/lib/MC/MCParser/AsmParser.cpp
===
--- llvm/lib/MC/MCParser/AsmParser.cpp
+++ llvm/lib/MC/MCParser/AsmParser.cpp
@@ -710,6 +710,9 @@
case MCObjectFileInfo::IsWasm:
PlatformParser.reset(createWasmAsmParser());
break;
+ case MCObjectFileInfo::IsXCOFF:
+// TODO: Need to implement createXCOFFAsmParser for XCOFF format.
+break;
}
PlatformParser->Initialize(*this);
Index: llvm/lib/MC/MCObjectFileInfo.cpp
===
--- llvm/lib/MC/MCObjectFileInfo.cpp
+++ llvm/lib/MC/MCObjectFileInfo.cpp
@@ -801,6 +801,10 @@
Env = IsWasm;
initWasmMCObjectFileInfo(TT);
break;
+ case Triple::XCOFF:
+Env = IsXCOFF;
+// TODO: Initialize MCObjectFileInfo for XCOFF format when MCSectionXCOFF is ready.
+break;
case Triple::UnknownObjectFormat:
report_fatal_error("Cannot initialize MC for unknown object file format.");
break;
@@ -816,6 +820,7 @@
case Triple::MachO:
case Triple::COFF:
case Triple::Wasm:
+ case Triple::XCOFF:
case Triple::UnknownObjectFormat:
report_fatal_error("Cannot get DWARF comdat section for this object file "
"format: not implemented.");
Index: llvm/lib/MC/MCContext.cpp
===
--- llvm/lib/MC/MCContext.cpp
+++ llvm/lib/MC/MCContext.cpp
@@ -161,6 +161,9 @@
return new (Name, *this) MCSymbolMachO(Name, IsTemporary);
case MCObjectFile
[PATCH] D58930: Add XCOFF triple object format type for AIX
jasonliu marked 2 inline comments as done.
jasonliu added inline comments.
Comment at:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:2079
+ if (log)
+log->Printf("sorry: unimplemented for XCOFF");
+ return false;
apaprocki wrote:
> No need to be `sorry:` :) This should probably just say `error: XCOFF is
> unimplemented` to be more direct in case anything is expecting "error:" in
> the output.
Sure. Will address in next revision.
Comment at: llvm/lib/Support/Triple.cpp:537
return StringSwitch(EnvironmentName)
+// FIXME: We have to put XCOFF before COFF;
+// perhaps an order-independent pattern matching is desired?
apaprocki wrote:
> hubert.reinterpretcast wrote:
> > If the conclusion is that checking XCOFF before COFF is fine, then we
> > should remove the FIXME and just leave a normal comment.
> Agreed, existing code seems fine as long as there is a comment explaining
> that `xcoff` must come before `coff` in case it isn't obvious at a quick
> glance.
Sounds good. I will remove FIXME and leave a normal comment to indicate the
order dependency.
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
jasonliu updated this revision to Diff 189387.
jasonliu added a comment.
Address some review comments
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58930/new/
https://reviews.llvm.org/D58930
Files:
clang/lib/CodeGen/BackendUtil.cpp
clang/lib/CodeGen/CodeGenModule.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
llvm/include/llvm/ADT/Triple.h
llvm/include/llvm/MC/MCObjectFileInfo.h
llvm/lib/MC/MCContext.cpp
llvm/lib/MC/MCObjectFileInfo.cpp
llvm/lib/MC/MCParser/AsmParser.cpp
llvm/lib/Support/Triple.cpp
llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
llvm/unittests/ADT/TripleTest.cpp
Index: llvm/unittests/ADT/TripleTest.cpp
===
--- llvm/unittests/ADT/TripleTest.cpp
+++ llvm/unittests/ADT/TripleTest.cpp
@@ -1258,6 +1258,11 @@
EXPECT_EQ(Triple::Wasm,
Triple("wasm64-unknown-wasi-musl-wasm").getObjectFormat());
+ EXPECT_EQ(Triple::XCOFF, Triple("powerpc-ibm-aix").getObjectFormat());
+ EXPECT_EQ(Triple::XCOFF, Triple("powerpc64-ibm-aix").getObjectFormat());
+ EXPECT_EQ(Triple::XCOFF, Triple("powerpc---xcoff").getObjectFormat());
+ EXPECT_EQ(Triple::XCOFF, Triple("powerpc64---xcoff").getObjectFormat());
+
Triple MSVCNormalized(Triple::normalize("i686-pc-windows-msvc-elf"));
EXPECT_EQ(Triple::ELF, MSVCNormalized.getObjectFormat());
@@ -1276,6 +1281,9 @@
T.setObjectFormat(Triple::MachO);
EXPECT_EQ(Triple::MachO, T.getObjectFormat());
+
+ T.setObjectFormat(Triple::XCOFF);
+ EXPECT_EQ(Triple::XCOFF, T.getObjectFormat());
}
TEST(TripleTest, NormalizeWindows) {
Index: llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
===
--- llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
+++ llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
@@ -5594,6 +5594,9 @@
case MCObjectFileInfo::IsWasm:
CurrentFormat = WASM;
break;
+ case MCObjectFileInfo::IsXCOFF:
+llvm_unreachable("unexpected object format");
+break;
}
if (~Prefix->SupportedFormats & CurrentFormat) {
Index: llvm/lib/Support/Triple.cpp
===
--- llvm/lib/Support/Triple.cpp
+++ llvm/lib/Support/Triple.cpp
@@ -534,6 +534,9 @@
static Triple::ObjectFormatType parseFormat(StringRef EnvironmentName) {
return StringSwitch(EnvironmentName)
+// "xcoff" must come before "coff" because of the order-dependendent
+// pattern matching.
+.EndsWith("xcoff", Triple::XCOFF)
.EndsWith("coff", Triple::COFF)
.EndsWith("elf", Triple::ELF)
.EndsWith("macho", Triple::MachO)
@@ -622,6 +625,7 @@
case Triple::ELF: return "elf";
case Triple::MachO: return "macho";
case Triple::Wasm: return "wasm";
+ case Triple::XCOFF: return "xcoff";
}
llvm_unreachable("unknown object format type");
}
@@ -686,6 +690,8 @@
case Triple::ppc64:
if (T.isOSDarwin())
return Triple::MachO;
+else if (T.isOSAIX())
+ return Triple::XCOFF;
return Triple::ELF;
case Triple::wasm32:
Index: llvm/lib/MC/MCParser/AsmParser.cpp
===
--- llvm/lib/MC/MCParser/AsmParser.cpp
+++ llvm/lib/MC/MCParser/AsmParser.cpp
@@ -710,6 +710,9 @@
case MCObjectFileInfo::IsWasm:
PlatformParser.reset(createWasmAsmParser());
break;
+ case MCObjectFileInfo::IsXCOFF:
+// TODO: Need to implement createXCOFFAsmParser for XCOFF format.
+break;
}
PlatformParser->Initialize(*this);
Index: llvm/lib/MC/MCObjectFileInfo.cpp
===
--- llvm/lib/MC/MCObjectFileInfo.cpp
+++ llvm/lib/MC/MCObjectFileInfo.cpp
@@ -801,6 +801,10 @@
Env = IsWasm;
initWasmMCObjectFileInfo(TT);
break;
+ case Triple::XCOFF:
+Env = IsXCOFF;
+// TODO: Initialize MCObjectFileInfo for XCOFF format when MCSectionXCOFF is ready.
+break;
case Triple::UnknownObjectFormat:
report_fatal_error("Cannot initialize MC for unknown object file format.");
break;
@@ -816,6 +820,7 @@
case Triple::MachO:
case Triple::COFF:
case Triple::Wasm:
+ case Triple::XCOFF:
case Triple::UnknownObjectFormat:
report_fatal_error("Cannot get DWARF comdat section for this object file "
"format: not implemented.");
Index: llvm/lib/MC/MCContext.cpp
===
--- llvm/lib/MC/MCContext.cpp
+++ llvm/lib/MC/MCContext.cpp
@@ -161,6 +161,9 @@
return new (Name, *this) MCSymbolMachO(Name, IsTemporary);
case MCObjectFileInfo::IsWasm:
return new (Name, *this) MCSymbolWasm(Name, IsTemporary);
+case MCObjectFileInfo::IsXCOFF:
+ // TODO: Need to implement class MCSymbolXCOFF.
+ break;
}
}
return new (Name, *this) MCSymbol(MCSymbol::SymbolKindUnset, Name,
Index: llvm/include/llvm/MC/M
[PATCH] D58930: Add XCOFF triple object format type for AIX
jasonliu marked an inline comment as done.
jasonliu added inline comments.
Comment at:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:2079
+ if (log)
+log->Printf("sorry: unimplemented for XCOFF");
+ return false;
JDevlieghere wrote:
> jasonliu wrote:
> > apaprocki wrote:
> > > No need to be `sorry:` :) This should probably just say `error: XCOFF is
> > > unimplemented` to be more direct in case anything is expecting "error:"
> > > in the output.
> > Sure. Will address in next revision.
> Just bundle this with the WASM case, the error message is correct for both.
I think they are different.
The error message for WASM seems to suggest that it will never ever get
supported on WASM.
But it is not the case for XCOFF, we want to indicate that it is not
implemented yet.
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
jasonliu marked 3 inline comments as done.
jasonliu added inline comments.
Comment at: clang/lib/CodeGen/BackendUtil.cpp:1470
+ case Triple::XCOFF:
+// TODO: Falling through for XCOFF format for now.
+break;
JDevlieghere wrote:
> This is confusing, you say fall through but you break? I would prefer a
> `llvm_unreachable("XCOFF not yet implemented");` here and elsewhere in this
> patch.
>
Thanks for the comment. Will address in next revision.
Comment at: clang/lib/CodeGen/BackendUtil.cpp:1486
+ case Triple::XCOFF:
+// TODO: Falling through for XCOFF format for now.
+break;
JDevlieghere wrote:
> See previous comment.
Will address in next revision.
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:4410
+ case llvm::Triple::XCOFF:
+llvm_unreachable("to be determined for XCOFF format");
case llvm::Triple::COFF:
JDevlieghere wrote:
> See previous comment.
Will address in next revision.
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
jasonliu marked an inline comment as done. jasonliu added inline comments. Comment at: llvm/lib/MC/MCContext.cpp:165 +case MCObjectFileInfo::IsXCOFF: + // TODO: Need to implement class MCSymbolXCOFF. + break; 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. 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
jasonliu updated this revision to Diff 189514.
jasonliu added a comment.
Address comments in last revision.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58930/new/
https://reviews.llvm.org/D58930
Files:
clang/lib/CodeGen/BackendUtil.cpp
clang/lib/CodeGen/CodeGenModule.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
llvm/include/llvm/ADT/Triple.h
llvm/include/llvm/MC/MCObjectFileInfo.h
llvm/lib/MC/MCContext.cpp
llvm/lib/MC/MCObjectFileInfo.cpp
llvm/lib/MC/MCParser/AsmParser.cpp
llvm/lib/Support/Triple.cpp
llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
llvm/unittests/ADT/TripleTest.cpp
Index: llvm/unittests/ADT/TripleTest.cpp
===
--- llvm/unittests/ADT/TripleTest.cpp
+++ llvm/unittests/ADT/TripleTest.cpp
@@ -1258,6 +1258,11 @@
EXPECT_EQ(Triple::Wasm,
Triple("wasm64-unknown-wasi-musl-wasm").getObjectFormat());
+ EXPECT_EQ(Triple::XCOFF, Triple("powerpc-ibm-aix").getObjectFormat());
+ EXPECT_EQ(Triple::XCOFF, Triple("powerpc64-ibm-aix").getObjectFormat());
+ EXPECT_EQ(Triple::XCOFF, Triple("powerpc---xcoff").getObjectFormat());
+ EXPECT_EQ(Triple::XCOFF, Triple("powerpc64---xcoff").getObjectFormat());
+
Triple MSVCNormalized(Triple::normalize("i686-pc-windows-msvc-elf"));
EXPECT_EQ(Triple::ELF, MSVCNormalized.getObjectFormat());
@@ -1276,6 +1281,9 @@
T.setObjectFormat(Triple::MachO);
EXPECT_EQ(Triple::MachO, T.getObjectFormat());
+
+ T.setObjectFormat(Triple::XCOFF);
+ EXPECT_EQ(Triple::XCOFF, T.getObjectFormat());
}
TEST(TripleTest, NormalizeWindows) {
Index: llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
===
--- llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
+++ llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
@@ -5594,6 +5594,9 @@
case MCObjectFileInfo::IsWasm:
CurrentFormat = WASM;
break;
+ case MCObjectFileInfo::IsXCOFF:
+llvm_unreachable("unexpected object format");
+break;
}
if (~Prefix->SupportedFormats & CurrentFormat) {
Index: llvm/lib/Support/Triple.cpp
===
--- llvm/lib/Support/Triple.cpp
+++ llvm/lib/Support/Triple.cpp
@@ -534,6 +534,9 @@
static Triple::ObjectFormatType parseFormat(StringRef EnvironmentName) {
return StringSwitch(EnvironmentName)
+// "xcoff" must come before "coff" because of the order-dependendent
+// pattern matching.
+.EndsWith("xcoff", Triple::XCOFF)
.EndsWith("coff", Triple::COFF)
.EndsWith("elf", Triple::ELF)
.EndsWith("macho", Triple::MachO)
@@ -622,6 +625,7 @@
case Triple::ELF: return "elf";
case Triple::MachO: return "macho";
case Triple::Wasm: return "wasm";
+ case Triple::XCOFF: return "xcoff";
}
llvm_unreachable("unknown object format type");
}
@@ -686,6 +690,8 @@
case Triple::ppc64:
if (T.isOSDarwin())
return Triple::MachO;
+else if (T.isOSAIX())
+ return Triple::XCOFF;
return Triple::ELF;
case Triple::wasm32:
Index: llvm/lib/MC/MCParser/AsmParser.cpp
===
--- llvm/lib/MC/MCParser/AsmParser.cpp
+++ llvm/lib/MC/MCParser/AsmParser.cpp
@@ -710,6 +710,9 @@
case MCObjectFileInfo::IsWasm:
PlatformParser.reset(createWasmAsmParser());
break;
+ case MCObjectFileInfo::IsXCOFF:
+// TODO: Need to implement createXCOFFAsmParser for XCOFF format.
+break;
}
PlatformParser->Initialize(*this);
Index: llvm/lib/MC/MCObjectFileInfo.cpp
===
--- llvm/lib/MC/MCObjectFileInfo.cpp
+++ llvm/lib/MC/MCObjectFileInfo.cpp
@@ -801,6 +801,10 @@
Env = IsWasm;
initWasmMCObjectFileInfo(TT);
break;
+ case Triple::XCOFF:
+Env = IsXCOFF;
+// TODO: Initialize MCObjectFileInfo for XCOFF format when MCSectionXCOFF is ready.
+break;
case Triple::UnknownObjectFormat:
report_fatal_error("Cannot initialize MC for unknown object file format.");
break;
@@ -816,6 +820,7 @@
case Triple::MachO:
case Triple::COFF:
case Triple::Wasm:
+ case Triple::XCOFF:
case Triple::UnknownObjectFormat:
report_fatal_error("Cannot get DWARF comdat section for this object file "
"format: not implemented.");
Index: llvm/lib/MC/MCContext.cpp
===
--- llvm/lib/MC/MCContext.cpp
+++ llvm/lib/MC/MCContext.cpp
@@ -161,6 +161,9 @@
return new (Name, *this) MCSymbolMachO(Name, IsTemporary);
case MCObjectFileInfo::IsWasm:
return new (Name, *this) MCSymbolWasm(Name, IsTemporary);
+case MCObjectFileInfo::IsXCOFF:
+ // TODO: Need to implement class MCSymbolXCOFF.
+ break;
}
}
return new (Name, *this) MCSymbol(MCSymbol::SymbolKindUnset, Name,
Index: llvm/include/llv
[PATCH] D58930: Add XCOFF triple object format type for AIX
jasonliu marked 2 inline comments as done.
jasonliu added inline comments.
Comment at:
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:2079
+ if (log)
+log->Printf("sorry: unimplemented for XCOFF");
+ return false;
hubert.reinterpretcast wrote:
> davide wrote:
> > hubert.reinterpretcast wrote:
> > > JDevlieghere wrote:
> > > > jasonliu wrote:
> > > > > JDevlieghere wrote:
> > > > > > jasonliu wrote:
> > > > > > > apaprocki wrote:
> > > > > > > > No need to be `sorry:` :) This should probably just say `error:
> > > > > > > > XCOFF is unimplemented` to be more direct in case anything is
> > > > > > > > expecting "error:" in the output.
> > > > > > > Sure. Will address in next revision.
> > > > > > Just bundle this with the WASM case, the error message is correct
> > > > > > for both.
> > > > > I think they are different.
> > > > > The error message for WASM seems to suggest that it will never ever
> > > > > get supported on WASM.
> > > > > But it is not the case for XCOFF, we want to indicate that it is not
> > > > > implemented yet.
> > > > I don't think the error message suggests that at all, and it's
> > > > definitely not true. At this point neither XCOFF nor WASM is supported,
> > > > and that's exactly what the log message says.
> > > >
> > > I agree that the error message for WASM does not indicate that the lack
> > > of support is inherent or intended to be permanent; however, it is not
> > > indicative either of an intent to implement the support. I am not sure
> > > what the intent is for WASM, but I do know that the intent for XCOFF is
> > > to eventually implement the support. I do not see how using an ambiguous
> > > message in this commit (when we know what the intent is) is superior to
> > > the alternative of having an unambiguous message.
> > I think we should keep this consistent with the other target so my vote is
> > for grouping XCOFF with WASM. After all, if it's going to be implemented
> > soon, the message will go away :)
> Well, I don't know about "soon"...
> Using the WASM message for XCOFF is not actually wrong; so, I can be okay
> with it.
Okay. Shared message it is.
Comment at: llvm/lib/MC/MCContext.cpp:165
+case MCObjectFileInfo::IsXCOFF:
+ // TODO: Need to implement class MCSymbolXCOFF.
+ break;
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.
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
jasonliu updated this revision to Diff 189713.
jasonliu added a comment.
Share the same "unsupported error" message with WASM.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58930/new/
https://reviews.llvm.org/D58930
Files:
clang/lib/CodeGen/BackendUtil.cpp
clang/lib/CodeGen/CodeGenModule.cpp
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
llvm/include/llvm/ADT/Triple.h
llvm/include/llvm/MC/MCObjectFileInfo.h
llvm/lib/MC/MCContext.cpp
llvm/lib/MC/MCObjectFileInfo.cpp
llvm/lib/MC/MCParser/AsmParser.cpp
llvm/lib/Support/Triple.cpp
llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
llvm/unittests/ADT/TripleTest.cpp
Index: llvm/unittests/ADT/TripleTest.cpp
===
--- llvm/unittests/ADT/TripleTest.cpp
+++ llvm/unittests/ADT/TripleTest.cpp
@@ -1258,6 +1258,11 @@
EXPECT_EQ(Triple::Wasm,
Triple("wasm64-unknown-wasi-musl-wasm").getObjectFormat());
+ EXPECT_EQ(Triple::XCOFF, Triple("powerpc-ibm-aix").getObjectFormat());
+ EXPECT_EQ(Triple::XCOFF, Triple("powerpc64-ibm-aix").getObjectFormat());
+ EXPECT_EQ(Triple::XCOFF, Triple("powerpc---xcoff").getObjectFormat());
+ EXPECT_EQ(Triple::XCOFF, Triple("powerpc64---xcoff").getObjectFormat());
+
Triple MSVCNormalized(Triple::normalize("i686-pc-windows-msvc-elf"));
EXPECT_EQ(Triple::ELF, MSVCNormalized.getObjectFormat());
@@ -1276,6 +1281,9 @@
T.setObjectFormat(Triple::MachO);
EXPECT_EQ(Triple::MachO, T.getObjectFormat());
+
+ T.setObjectFormat(Triple::XCOFF);
+ EXPECT_EQ(Triple::XCOFF, T.getObjectFormat());
}
TEST(TripleTest, NormalizeWindows) {
Index: llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
===
--- llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
+++ llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
@@ -5594,6 +5594,9 @@
case MCObjectFileInfo::IsWasm:
CurrentFormat = WASM;
break;
+ case MCObjectFileInfo::IsXCOFF:
+llvm_unreachable("unexpected object format");
+break;
}
if (~Prefix->SupportedFormats & CurrentFormat) {
Index: llvm/lib/Support/Triple.cpp
===
--- llvm/lib/Support/Triple.cpp
+++ llvm/lib/Support/Triple.cpp
@@ -534,6 +534,9 @@
static Triple::ObjectFormatType parseFormat(StringRef EnvironmentName) {
return StringSwitch(EnvironmentName)
+// "xcoff" must come before "coff" because of the order-dependendent
+// pattern matching.
+.EndsWith("xcoff", Triple::XCOFF)
.EndsWith("coff", Triple::COFF)
.EndsWith("elf", Triple::ELF)
.EndsWith("macho", Triple::MachO)
@@ -622,6 +625,7 @@
case Triple::ELF: return "elf";
case Triple::MachO: return "macho";
case Triple::Wasm: return "wasm";
+ case Triple::XCOFF: return "xcoff";
}
llvm_unreachable("unknown object format type");
}
@@ -686,6 +690,8 @@
case Triple::ppc64:
if (T.isOSDarwin())
return Triple::MachO;
+else if (T.isOSAIX())
+ return Triple::XCOFF;
return Triple::ELF;
case Triple::wasm32:
Index: llvm/lib/MC/MCParser/AsmParser.cpp
===
--- llvm/lib/MC/MCParser/AsmParser.cpp
+++ llvm/lib/MC/MCParser/AsmParser.cpp
@@ -710,6 +710,9 @@
case MCObjectFileInfo::IsWasm:
PlatformParser.reset(createWasmAsmParser());
break;
+ case MCObjectFileInfo::IsXCOFF:
+// TODO: Need to implement createXCOFFAsmParser for XCOFF format.
+break;
}
PlatformParser->Initialize(*this);
Index: llvm/lib/MC/MCObjectFileInfo.cpp
===
--- llvm/lib/MC/MCObjectFileInfo.cpp
+++ llvm/lib/MC/MCObjectFileInfo.cpp
@@ -801,6 +801,10 @@
Env = IsWasm;
initWasmMCObjectFileInfo(TT);
break;
+ case Triple::XCOFF:
+Env = IsXCOFF;
+// TODO: Initialize MCObjectFileInfo for XCOFF format when MCSectionXCOFF is ready.
+break;
case Triple::UnknownObjectFormat:
report_fatal_error("Cannot initialize MC for unknown object file format.");
break;
@@ -816,6 +820,7 @@
case Triple::MachO:
case Triple::COFF:
case Triple::Wasm:
+ case Triple::XCOFF:
case Triple::UnknownObjectFormat:
report_fatal_error("Cannot get DWARF comdat section for this object file "
"format: not implemented.");
Index: llvm/lib/MC/MCContext.cpp
===
--- llvm/lib/MC/MCContext.cpp
+++ llvm/lib/MC/MCContext.cpp
@@ -161,6 +161,9 @@
return new (Name, *this) MCSymbolMachO(Name, IsTemporary);
case MCObjectFileInfo::IsWasm:
return new (Name, *this) MCSymbolWasm(Name, IsTemporary);
+case MCObjectFileInfo::IsXCOFF:
+ // TODO: Need to implement class MCSymbolXCOFF.
+ break;
}
}
return new (Name, *this) MCSymbol(MCSymbol::SymbolKindUnset, Name,
Inde
[PATCH] D58930: Add XCOFF triple object format type for AIX
This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL355989: Add XCOFF triple object format type for AIX
(authored by jasonliu, committed by ).
Herald added a subscriber: kristina.
Changed prior to commit:
https://reviews.llvm.org/D58930?vs=189713&id=190346#toc
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58930/new/
https://reviews.llvm.org/D58930
Files:
cfe/trunk/lib/CodeGen/BackendUtil.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
llvm/trunk/include/llvm/ADT/Triple.h
llvm/trunk/include/llvm/MC/MCObjectFileInfo.h
llvm/trunk/lib/MC/MCContext.cpp
llvm/trunk/lib/MC/MCObjectFileInfo.cpp
llvm/trunk/lib/MC/MCParser/AsmParser.cpp
llvm/trunk/lib/Support/Triple.cpp
llvm/trunk/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
llvm/trunk/unittests/ADT/TripleTest.cpp
Index: llvm/trunk/include/llvm/ADT/Triple.h
===
--- llvm/trunk/include/llvm/ADT/Triple.h
+++ llvm/trunk/include/llvm/ADT/Triple.h
@@ -219,6 +219,7 @@
ELF,
MachO,
Wasm,
+XCOFF,
};
private:
@@ -598,6 +599,11 @@
!isAndroid();
}
+ /// Tests whether the OS is AIX.
+ bool isOSAIX() const {
+return getOS() == Triple::AIX;
+ }
+
/// Tests whether the OS uses the ELF binary format.
bool isOSBinFormatELF() const {
return getObjectFormat() == Triple::ELF;
@@ -618,6 +624,11 @@
return getObjectFormat() == Triple::Wasm;
}
+ /// Tests whether the OS uses the XCOFF binary format.
+ bool isOSBinFormatXCOFF() const {
+return getObjectFormat() == Triple::XCOFF;
+ }
+
/// Tests whether the target is the PS4 CPU
bool isPS4CPU() const {
return getArch() == Triple::x86_64 &&
Index: llvm/trunk/include/llvm/MC/MCObjectFileInfo.h
===
--- llvm/trunk/include/llvm/MC/MCObjectFileInfo.h
+++ llvm/trunk/include/llvm/MC/MCObjectFileInfo.h
@@ -380,7 +380,7 @@
return EHFrameSection;
}
- enum Environment { IsMachO, IsELF, IsCOFF, IsWasm };
+ enum Environment { IsMachO, IsELF, IsCOFF, IsWasm, IsXCOFF };
Environment getObjectFileType() const { return Env; }
bool isPositionIndependent() const { return PositionIndependent; }
Index: llvm/trunk/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
===
--- llvm/trunk/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
+++ llvm/trunk/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
@@ -5594,6 +5594,9 @@
case MCObjectFileInfo::IsWasm:
CurrentFormat = WASM;
break;
+ case MCObjectFileInfo::IsXCOFF:
+llvm_unreachable("unexpected object format");
+break;
}
if (~Prefix->SupportedFormats & CurrentFormat) {
Index: llvm/trunk/lib/Support/Triple.cpp
===
--- llvm/trunk/lib/Support/Triple.cpp
+++ llvm/trunk/lib/Support/Triple.cpp
@@ -534,6 +534,9 @@
static Triple::ObjectFormatType parseFormat(StringRef EnvironmentName) {
return StringSwitch(EnvironmentName)
+// "xcoff" must come before "coff" because of the order-dependendent
+// pattern matching.
+.EndsWith("xcoff", Triple::XCOFF)
.EndsWith("coff", Triple::COFF)
.EndsWith("elf", Triple::ELF)
.EndsWith("macho", Triple::MachO)
@@ -622,6 +625,7 @@
case Triple::ELF: return "elf";
case Triple::MachO: return "macho";
case Triple::Wasm: return "wasm";
+ case Triple::XCOFF: return "xcoff";
}
llvm_unreachable("unknown object format type");
}
@@ -686,6 +690,8 @@
case Triple::ppc64:
if (T.isOSDarwin())
return Triple::MachO;
+else if (T.isOSAIX())
+ return Triple::XCOFF;
return Triple::ELF;
case Triple::wasm32:
Index: llvm/trunk/lib/MC/MCParser/AsmParser.cpp
===
--- llvm/trunk/lib/MC/MCParser/AsmParser.cpp
+++ llvm/trunk/lib/MC/MCParser/AsmParser.cpp
@@ -710,6 +710,9 @@
case MCObjectFileInfo::IsWasm:
PlatformParser.reset(createWasmAsmParser());
break;
+ case MCObjectFileInfo::IsXCOFF:
+// TODO: Need to implement createXCOFFAsmParser for XCOFF format.
+break;
}
PlatformParser->Initialize(*this);
Index: llvm/trunk/lib/MC/MCContext.cpp
===
--- llvm/trunk/lib/MC/MCContext.cpp
+++ llvm/trunk/lib/MC/MCContext.cpp
@@ -161,6 +161,9 @@
return new (Name, *this) MCSymbolMachO(Name, IsTemporary);
case MCObjectFileInfo::IsWasm:
return new (Name, *this) MCSymbolWasm(Name, IsTemporary);
+case MCObjectFileInfo::IsXCOFF:
+ // TODO: Need to implement class MCSymbolXCOFF.
+ break;
}
}
return new (Name, *this) MCSymbol(MCSymbol::SymbolKindUnset, Name,
I
[PATCH] D59048: Add AIX Target Info
This revision was automatically updated to reflect the committed changes. Closed by commit rC356060: Add AIX Target Info (authored by jasonliu, committed by ). Changed prior to commit: https://reviews.llvm.org/D59048?vs=190133&id=190425#toc Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59048/new/ https://reviews.llvm.org/D59048 Files: lib/Basic/Targets.cpp lib/Basic/Targets/OSTargets.h lib/Basic/Targets/PPC.cpp lib/Basic/Targets/PPC.h test/Driver/types.c test/Headers/max_align.c test/Preprocessor/init.c test/Sema/varargs-aix.c Index: test/Preprocessor/init.c === --- test/Preprocessor/init.c +++ test/Preprocessor/init.c @@ -6409,6 +6409,209 @@ // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-none-none -target-feature +float128 -target-cpu power9 -fno-signed-char < /dev/null | FileCheck -check-prefix PPC-FLOAT128 %s // PPC-FLOAT128:#define __FLOAT128__ 1 // +// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc64-ibm-aix7.1.0.0 -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPC64-AIX %s +// +// PPC64-AIX:#define _AIX 1 +// PPC64-AIX:#define _ARCH_PPC 1 +// PPC64-AIX:#define _ARCH_PPC64 1 +// PPC64-AIX:#define _BIG_ENDIAN 1 +// PPC64-AIX:#define _IBMR2 1 +// PPC64-AIX-NOT:#define _ILP32 1 +// PPC64-AIX:#define _LONG_LONG 1 +// PPC64-AIX:#define _LP64 1 +// PPC64-AIX:#define _POWER 1 +// PPC64-AIX:#define __64BIT__ 1 +// PPC64-AIX:#define __BIGGEST_ALIGNMENT__ 8 +// PPC64-AIX:#define __BIG_ENDIAN__ 1 +// PPC64-AIX:#define __BYTE_ORDER__ __ORDER_BIG_ENDIAN__ +// PPC64-AIX:#define __CHAR16_TYPE__ unsigned short +// PPC64-AIX:#define __CHAR32_TYPE__ unsigned int +// PPC64-AIX:#define __CHAR_BIT__ 8 +// PPC64-AIX:#define __CHAR_UNSIGNED__ 1 +// PPC64-AIX:#define __DBL_DENORM_MIN__ 4.9406564584124654e-324 +// PPC64-AIX:#define __DBL_DIG__ 15 +// PPC64-AIX:#define __DBL_EPSILON__ 2.2204460492503131e-16 +// PPC64-AIX:#define __DBL_HAS_DENORM__ 1 +// PPC64-AIX:#define __DBL_HAS_INFINITY__ 1 +// PPC64-AIX:#define __DBL_HAS_QUIET_NAN__ 1 +// PPC64-AIX:#define __DBL_MANT_DIG__ 53 +// PPC64-AIX:#define __DBL_MAX_10_EXP__ 308 +// PPC64-AIX:#define __DBL_MAX_EXP__ 1024 +// PPC64-AIX:#define __DBL_MAX__ 1.7976931348623157e+308 +// PPC64-AIX:#define __DBL_MIN_10_EXP__ (-307) +// PPC64-AIX:#define __DBL_MIN_EXP__ (-1021) +// PPC64-AIX:#define __DBL_MIN__ 2.2250738585072014e-308 +// PPC64-AIX:#define __DECIMAL_DIG__ __LDBL_DECIMAL_DIG__ +// PPC64-AIX:#define __FLT_DENORM_MIN__ 1.40129846e-45F +// PPC64-AIX:#define __FLT_DIG__ 6 +// PPC64-AIX:#define __FLT_EPSILON__ 1.19209290e-7F +// PPC64-AIX:#define __FLT_EVAL_METHOD__ 1 +// PPC64-AIX:#define __FLT_HAS_DENORM__ 1 +// PPC64-AIX:#define __FLT_HAS_INFINITY__ 1 +// PPC64-AIX:#define __FLT_HAS_QUIET_NAN__ 1 +// PPC64-AIX:#define __FLT_MANT_DIG__ 24 +// PPC64-AIX:#define __FLT_MAX_10_EXP__ 38 +// PPC64-AIX:#define __FLT_MAX_EXP__ 128 +// PPC64-AIX:#define __FLT_MAX__ 3.40282347e+38F +// PPC64-AIX:#define __FLT_MIN_10_EXP__ (-37) +// PPC64-AIX:#define __FLT_MIN_EXP__ (-125) +// PPC64-AIX:#define __FLT_MIN__ 1.17549435e-38F +// PPC64-AIX:#define __FLT_RADIX__ 2 +// PPC64-AIX-NOT:#define __ILP32__ 1 +// PPC64-AIX:#define __INT16_C_SUFFIX__ +// PPC64-AIX:#define __INT16_FMTd__ "hd" +// PPC64-AIX:#define __INT16_FMTi__ "hi" +// PPC64-AIX:#define __INT16_MAX__ 32767 +// PPC64-AIX:#define __INT16_TYPE__ short +// PPC64-AIX:#define __INT32_C_SUFFIX__ +// PPC64-AIX:#define __INT32_FMTd__ "d" +// PPC64-AIX:#define __INT32_FMTi__ "i" +// PPC64-AIX:#define __INT32_MAX__ 2147483647 +// PPC64-AIX:#define __INT32_TYPE__ int +// PPC64-AIX:#define __INT64_C_SUFFIX__ L +// PPC64-AIX:#define __INT64_FMTd__ "ld" +// PPC64-AIX:#define __INT64_FMTi__ "li" +// PPC64-AIX:#define __INT64_MAX__ 9223372036854775807L +// PPC64-AIX:#define __INT64_TYPE__ long int +// PPC64-AIX:#define __INT8_C_SUFFIX__ +// PPC64-AIX:#define __INT8_FMTd__ "hhd" +// PPC64-AIX:#define __INT8_FMTi__ "hhi" +// PPC64-AIX:#define __INT8_MAX__ 127 +// PPC64-AIX:#define __INT8_TYPE__ signed char +// PPC64-AIX:#define __INTMAX_C_SUFFIX__ L +// PPC64-AIX:#define __INTMAX_FMTd__ "ld" +// PPC64-AIX:#define __INTMAX_FMTi__ "li" +// PPC64-AIX:#define __INTMAX_MAX__ 9223372036854775807L +// PPC64-AIX:#define __INTMAX_TYPE__ long int +// PPC64-AIX:#define __INTMAX_WIDTH__ 64 +// PPC64-AIX:#define __INTPTR_FMTd__ "ld" +// PPC64-AIX:#define __INTPTR_FMTi__ "li" +// PPC64-AIX:#define __INTPTR_MAX__ 9223372036854775807L +// PPC64-AIX:#define __INTPTR_TYPE__ long int +// PPC64-AIX:#define __INTPTR_WIDTH__ 64 +// PPC64-AIX:#define __INT_FAST16_FMTd__ "hd" +// PPC64-AIX:#define __INT_FAST16_FMTi__ "hi" +// PPC64-AIX:#define __INT_FAST16_MAX__ 32767 +// PPC64-AIX:#define __INT_FAST16_TYPE__ short +// PPC64-AIX:#define __INT_FAST32_FMTd__ "d" +// PPC64-AIX:#define __INT_FAST32_FMTi__ "i" +// PPC64-AIX:#define __INT_FAST32_MAX__ 2147483647 +// PPC64-AIX:#define __INT_FAST32_TYPE__ int +// PPC64
[PATCH] D59048: Add AIX Target Info
jasonliu added a comment. I'm seeing some build bot failures because of the newly added test case: max_align.c. So I reverted the previous committed change. Please fix the test case and let me know so that I can help you commit again. Repository: rC Clang 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] D68340: Add AIX toolchain and basic linker functionality
jasonliu added a comment.
Any reason we are testing library search path using libgcc.a? We could use any
dummy path there, and upload a dummy archive library for the testing purpose.
Comment at: clang/lib/Driver/ToolChains/AIX.cpp:112
+
+AIX::~AIX() {}
+
We are not doing anything with the destructor. Do we need to declare and define
it?
Comment at: clang/lib/Driver/ToolChains/AIX.h:1
+//===--- AIX.h - AIX ToolChain Implementations --*- C++ -*-===//
+//
The length of this line should match with line 7.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68340/new/
https://reviews.llvm.org/D68340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D68340: Add AIX toolchain and basic linker functionality
jasonliu added a comment. nit: We might want to add period consistently for all the comments in the patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68340/new/ https://reviews.llvm.org/D68340 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D68340: Add AIX toolchain and basic linker functionality
jasonliu added inline comments.
Comment at: clang/lib/Driver/ToolChains/AIX.cpp:35
+ // Only support 32 and 64 bit
+ if (!IsArch32Bit && !IsArch64Bit)
+llvm_unreachable("Unsupported bit width value");
Xiangling_L wrote:
> Is there any reason to use llvm_unreachable here? I think we should use
> 'assertion' instead here:
>
> ```
> assert((IsArch32Bit || IsArch64Bit) && "...");
> ```
IsArch64Bit used only in the assertion could cause warning when the assertion
is turned off.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68340/new/
https://reviews.llvm.org/D68340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D68340: Add AIX toolchain and basic linker functionality
jasonliu added inline comments. Comment at: clang/lib/Driver/ToolChains/AIX.h:19 + +/// aix -- Directly call system default and linker. +// TODO: Enable direct call to system default assembler. remove "and". Comment at: clang/lib/Driver/ToolChains/AIX.h:36 + +} // end namespace aix. + Sorry, I don't think we need '.' for any of the "end namespace ..." comment. Comment at: clang/lib/Driver/ToolChains/AIX.h:63 + +#endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_AIX_H. We don't need '.' here as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68340/new/ https://reviews.llvm.org/D68340 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D68340: Add AIX toolchain and basic linker functionality
jasonliu added inline comments. Comment at: clang/lib/Driver/CMakeLists.txt:33 ToolChains/Arch/X86.cpp + ToolChains/AIX.cpp ToolChains/Ananas.cpp Looks like this list is following alphabetical order here, which means we should probably put "ToolChains/AIX.cpp" right after "ToolChain.cpp". Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68340/new/ https://reviews.llvm.org/D68340 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D68340: Add AIX toolchain and basic linker functionality
jasonliu accepted this revision.
jasonliu added a comment.
This revision is now accepted and ready to land.
Aside from the nit comment that can be addressed when checkin.
LGTM.
Comment at: clang/lib/Driver/ToolChains/AIX.cpp:59
+
+ auto getCrt0Basename = [&IsArch32Bit, &Args] {
+// Enable gprofiling when "-pg" is specified.
nit: There is no need to capture IsArch32Bit by reference.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68340/new/
https://reviews.llvm.org/D68340
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double
jasonliu added inline comments. Comment at: clang/include/clang/AST/RecordLayout.h:74 + /// The maximum allowed field alignment. This is set by #pragma pack. + CharUnits MaxFieldAlignment; + efriedma wrote: > If we have to keep around extra data anyway, probably better to add a field > for the preferred alignment, so we can keep the preferred alignment handling > together. We could get the MaxFieldAlignment in the same way ItaniumRecordLayoutBuilder::InitializeLayout get. Do we need to keep an extra data? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79719/new/ https://reviews.llvm.org/D79719 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double
jasonliu added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:2533 + hasFloatingPointAsFirstMember(RD, MaxFieldAlignment)) +return std::max(ABIAlign, (unsigned)toBits(CharUnits::fromQuantity(8))); +} I guess another thing that worth considering is how we support "pragma align(nature)" in the future. It requires us to get a normal alignment for AIX which means double will be aligned 8 byte in the structure. Right now, we always set the double to be aligned 4 byte in PPC.h, but it's not necessary the case if that pragma is in effect. How double should be aligned in a structure is really position dependent (is the structure comes after `pragma align(nature)` or `pragma align(power)`?) I'm not very clear on how we would handle that with the current architect. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79719/new/ https://reviews.llvm.org/D79719 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D83974: [AIX] report_fatal_error on `-fregister_global_dtors_with_atexit` for static init
jasonliu accepted this revision.
jasonliu added a comment.
This revision is now accepted and ready to land.
LGTM with minor nit.
Comment at:
clang/test/CodeGenCXX/aix-sinit-register-global-dtors-with-atexit.cpp:10
+struct T{
+T();
+~T();
nit: fix clang-format warning.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83974/new/
https://reviews.llvm.org/D83974
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double
jasonliu added inline comments.
Comment at: clang/lib/AST/ASTContext.cpp:2418
+
if (!Target->allowsLargerPreferedTypeAlignment())
return ABIAlign;
Should this if statement go above the `if (const auto *RT =
T->getAs()) ` ?
When Target does not allow larger prefered type alignment, then we should
return ABIAlign immediately without going through the RecordType query?
Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1234
+
+ bool DefaultsToAIXPowerAlignment =
+ Context.getTargetInfo().defaultsToAIXPowerAlignment();
Add const?
Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1801
+
+ bool DefaultsToAIXPowerAlignment =
+ Context.getTargetInfo().defaultsToAIXPowerAlignment();
const ?
Comment at: clang/lib/Basic/Targets/PPC.h:425
+} else if (Triple.isOSAIX()) {
+ SuitableAlign = 64;
+ LongDoubleWidth = 64;
SuitableAlign is set in line 412 as well. Please consider combining the two
`if` statements if grouping things together makes code easier to parse.
Comment at: clang/test/Layout/aix-double-struct-member.cpp:2
+// RUN: %clang_cc1 -emit-llvm-only -triple powerpc-ibm-aix-xcoff \
+// RUN: -fdump-record-layouts -fsyntax-only -x c++ %s | \
+// RUN: FileCheck %s
You are not using ` < %s` here. So `-x c++` is redundant?
Comment at: clang/test/Layout/aix-double-struct-member.cpp:305
+struct A { double x; };
+struct B : A {} b;
+
`b` is not necessary when you take the `sizeof` of B below?
Comment at: clang/test/Layout/aix-no-unique-address-with-double.cpp:138
+// CHECK-NEXT:| nvsize=8, nvalign=4, preferrednvalign=4]
+
+int a = sizeof(Empty);
I think this case is interesting and may worth adding too:
```
struct F {
[[no_unique_address]] Empty emp, emp2;
double d;
};
```
Comment at: clang/test/Layout/aix-power-alignment-typedef.cpp:12
+ double x;
+} c;
+typedef struct C __attribute__((__aligned__(2))) CC;
remove `c` ?
Comment at: clang/test/Layout/aix-power-alignment-typedef.cpp:25
+ Dbl x;
+} a;
+
remove `a`?
Comment at: clang/test/Layout/aix-power-alignment-typedef.cpp:43
+ char x;
+} u;
+
remove `u`?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79719/new/
https://reviews.llvm.org/D79719
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double
jasonliu added a comment. Thanks. No further comments from me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79719/new/ https://reviews.llvm.org/D79719 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D84356: [AIX] remove -u from the clang when invoke aix as assembler
jasonliu accepted this revision. jasonliu added a comment. This revision is now accepted and ready to land. Thanks. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84356/new/ https://reviews.llvm.org/D84356 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D84534: [AIX] Static init frontend recovery and backend support
jasonliu added inline comments.
Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4609
+// their own llvm.global_dtors entry.
+CGM.AddCXXStermFinalizerToGlobalDtor(StermFinalizer, 65535);
+ else
Handling template instantiation seems fairly orthogonal to "moving naming logic
from frontend to backend". Could we put it in a separate patch (which could be
a child of this one)?
Comment at: llvm/include/llvm/CodeGen/AsmPrinter.h:460
+ virtual void emitXXStructor(const DataLayout &DL, const int Priority,
+ const unsigned index, Constant *CV, bool isCtor)
{
+llvm_unreachable("Emit CXXStructor with priority is target-specific.");
Remove `const` from value type (e.g. Priority and index), as there is no real
need for it.
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:2165
+ emitXXStructor(DL, S.Priority, index, S.Func, isCtor);
+ ++index;
+ continue;
Maybe a naive quesiton, in what situation would we have name collision and need
`index` as suffix to differentiate?
Could we just report_fatal_error in those situation?
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1848
+// beforing emitting them.
+if (isSpecialLLVMGlobalArrayForStaticInit(&G)) {
+ if (GlobalUniqueModuleId.empty()) {
This would have conflict with D84363. You might want to rebase it later.
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1935
+ Function *Func = cast(CV);
+ Func->setLinkage(GlobalValue::ExternalLinkage);
+ Func->setName((isCtor ? llvm::Twine("__sinit") : llvm::Twine("__sterm")) +
Changing Function name and linkage underneath looks a bit scary. People would
have a hard time to map IR to final assembly that gets produced. Have you
thought about creating an alias (with the correct linkage and name) to the
original function instead?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84534/new/
https://reviews.llvm.org/D84534
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D76932: [AIX] emit .extern and .weak directive linkage
jasonliu added a comment.
Thanks for splitting the test case. I think it helps a lot for reading and
maintainability.
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1490
continue;
GlobalValue::VisibilityTypes V = F.getVisibility();
+
This line is not used before XCOFF specialized code, we should move it down.
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1495
+ if (F.isIntrinsic())
+continue;
+
If I understand correctly, we do not want to touch how we interact with
visibility in this patch.
If that's the case, we don't want to `continue` here and in line 1512 since the
early `continue` will change the code flow for visibility.
Suggestion:
if (TM.getTargetTriple().isOSBinFormatXCOFF() && !F.isIntrinsic()) {
and remove continue in line 1512.
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1498
+ // Get the function entry point symbol.
+ Name = OutContext.getOrCreateSymbol("." + Name->getName());
+ if (cast(Name)->hasContainingCsect())
nit: I don't think it's a good idea here to assign back to `Name` here, as
`Name` will also get used in `emitVisibility` below and we want to keep it as
it is. Let's create a new MCSymbol* here.
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1500
+ if (cast(Name)->hasContainingCsect())
+emitLinkage(&F, Name);
+
1. We need to rebase here, as it is called `hasRepresentedCsectSet()` instead
of `hasContainingCsect()` now.
2. I'm slightly worried here to rely on `hasRepresentedCsectSet()` to check if
a linkage should be emitted. This is based on the assumption that we will not
ever change our implementation for `.bl foo` to `.bl foo[PR]`. But in
https://reviews.llvm.org/D77080#inline-706207, we discussed about this
possibility. So this assumption might not be true in the future. However, I'm
not sure if there is another way to check if this function have been called
directly.
So if there is another way to check, we should pursue the alternative instead.
If there is not, then we need to add an assert here, like
`assert(Name->getName().equals(cast(Name)->getUnqualifiedName())`
to make sure we don't get a qualname here.
3.
Have a comment here and tell people what we are doing here.
For example,
// If there is a direct call to external function, then we need to emit linkage
for its function entry point.
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1503
+ if (F.hasAddressTaken()) {
+// Indirect call reference of the extern function.
+// for example c source code as :
Comment need to mention what we are trying to do here:
// If address is taken from an extern function, we need to emit linkage for its
function descriptor symbol.
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1504
+// Indirect call reference of the extern function.
+// for example c source code as :
+// extern int bar_ext();
nit: for -> For,
and we don't need space between `as` and `:`
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1510
+Name = Csect->getQualNameSymbol();
+emitLinkage(&F, Name);
+ }
nit: We don't need to assign it back to `Name`.
emitLinkage(&F, Csect->getQualNameSymbol());
Comment at: llvm/test/CodeGen/PowerPC/aix-WeakODRLinkage.ll:10
+ ret void
+}
+
Do we also want to test WeakAnyLinkage?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76932/new/
https://reviews.llvm.org/D76932
___
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
jasonliu accepted this revision. jasonliu added a comment. This revision is now accepted and ready to land. Not sure if you want to wait for the feedback from @sfertile and @cebowleratibm , but I'm good with the current revision. Repository: rG LLVM Github Monorepo 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] D76932: [AIX] emit .extern and .weak directive linkage
jasonliu added inline comments. Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1500 + if (cast(Name)->hasContainingCsect()) +emitLinkage(&F, Name); + DiggerLin wrote: > jasonliu wrote: > > 1. We need to rebase here, as it is called `hasRepresentedCsectSet()` > > instead of `hasContainingCsect()` now. > > 2. I'm slightly worried here to rely on `hasRepresentedCsectSet()` to check > > if a linkage should be emitted. This is based on the assumption that we > > will not ever change our implementation for `.bl foo` to `.bl foo[PR]`. But > > in https://reviews.llvm.org/D77080#inline-706207, we discussed about this > > possibility. So this assumption might not be true in the future. However, > > I'm not sure if there is another way to check if this function have been > > called directly. > > So if there is another way to check, we should pursue the alternative > > instead. If there is not, then we need to add an assert here, like > > `assert(Name->getName().equals(cast(Name)->getUnqualifiedName())` > > to make sure we don't get a qualname here. > > 3. > > Have a comment here and tell people what we are doing here. > > For example, > > // If there is a direct call to external function, then we need to emit > > linkage for its function entry point. > when we implement .bl foo to .bl foo[PR] > the SymbolName will change from .bl[SMC] and check the > .bl[SMC]->hasRepresentedCsectSet() Yes, but foo[PR]->hasRepresentedCsectSet() will always return true, because whenever we created a qualname will always have csect set. How will we know if foo() function is called directly then? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76932/new/ https://reviews.llvm.org/D76932 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D76932: [AIX] emit .extern and .weak directive linkage
jasonliu added inline comments. Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1500 + if (cast(Name)->hasContainingCsect()) +emitLinkage(&F, Name); + DiggerLin wrote: > jasonliu wrote: > > DiggerLin wrote: > > > jasonliu wrote: > > > > 1. We need to rebase here, as it is called `hasRepresentedCsectSet()` > > > > instead of `hasContainingCsect()` now. > > > > 2. I'm slightly worried here to rely on `hasRepresentedCsectSet()` to > > > > check if a linkage should be emitted. This is based on the assumption > > > > that we will not ever change our implementation for `.bl foo` to `.bl > > > > foo[PR]`. But in https://reviews.llvm.org/D77080#inline-706207, we > > > > discussed about this possibility. So this assumption might not be true > > > > in the future. However, I'm not sure if there is another way to check > > > > if this function have been called directly. > > > > So if there is another way to check, we should pursue the alternative > > > > instead. If there is not, then we need to add an assert here, like > > > > `assert(Name->getName().equals(cast(Name)->getUnqualifiedName())` > > > > to make sure we don't get a qualname here. > > > > 3. > > > > Have a comment here and tell people what we are doing here. > > > > For example, > > > > // If there is a direct call to external function, then we need to emit > > > > linkage for its function entry point. > > > when we implement .bl foo to .bl foo[PR] > > > the SymbolName will change from .bl[SMC] and check the > > > .bl[SMC]->hasRepresentedCsectSet() > > Yes, but foo[PR]->hasRepresentedCsectSet() will always return true, because > > whenever we created a qualname will always have csect set. How will we know > > if foo() function is called directly then? > we only deal with extern function(we do not deal with definition function) > here, for extern function, it not always has MCSectionXCOFF, it only create > the extern function be called directly in the > llvm/lib/CodeGen/MachineModuleInfo.cpp line 108~116, unless we delete the > code later. if the code is not changed. for extern function foo , the > .foo[PR] -> hasRepresentedCsectSet() when directly call. otherwise false. Okk. I think I got what you mean. If .foo[PR] is created then it means there is a direct call. Otherwise, we will not have a .foo[PR], and of course that newly created .foo[PR] will not have a RepresentedCsect. Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76932/new/ https://reviews.llvm.org/D76932 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D76932: [AIX] emit .extern and .weak directive linkage
jasonliu added inline comments.
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1497
+ OutContext.getOrCreateSymbol("." + Name->getName());
+ assert(FnEntryPointSym->getName().equals(
+ cast(FnEntryPointSym)->getUnqualifiedName()) &&
Based on our discussion, this assertion could be removed.
And please add a comment saying
// If there is a direct call to this extern function, we need to emit linkage
for its function entry point symbol.
Comment at: llvm/test/CodeGen/PowerPC/aix-extern-weak.ll:11
+
+@foo_ext_weak_p = global void (...)* bitcast (void ()* @foo_ext_weak to void
(...)*)
+
hubert.reinterpretcast wrote:
> I would prefer to test the function call and the take-address-of-function
> cases separately here.
I think this is to test when the function is both taken address, and get called
directly, will we emit linkage for both function entry point and function
descriptor symbol? The original implementation only emit one of them.
Maybe it makes sense to have a separate test case like this to test the linkage
emit in all circumstance:
```
void foo_called();
void foo_refed();
void foo_called_refed();
void (*foo_refed_p)() = foo_refed;
void (*foo_called_refed_p)() = foo_called_refed;
void bar(){
foo_called();
foo_called_refed();
}
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76932/new/
https://reviews.llvm.org/D76932
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D76932: [AIX] emit .extern and .weak directive linkage
jasonliu added inline comments.
Comment at: llvm/test/CodeGen/PowerPC/aix-extern.ll:14
+
+; Function Attrs: noinline nounwind optnone
+define void @foo() #0 {
nit: Function Attrs and `#0` could be removed
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76932/new/
https://reviews.llvm.org/D76932
___
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
jasonliu added a comment. This is landed in https://github.com/llvm/llvm-project/commit/085689d44cb95604072d0f2b130167d9410ea155. Not sure why it does not close automatically. Repository: rG LLVM Github Monorepo 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] D76130: [PPC][AIX] Implement variadic function handling in LowerFormalArguments_AIX
This revision was automatically updated to reflect the committed changes.
Closed by commit rG085689d44cb9: [PPC][AIX] Implement variadic function
handling in LowerFormalArguments_AIX (authored by jasonliu).
Changed prior to commit:
https://reviews.llvm.org/D76130?vs=255321&id=256367#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76130/new/
https://reviews.llvm.org/D76130
Files:
llvm/lib/Target/PowerPC/PPCISelLowering.cpp
llvm/test/CodeGen/PowerPC/aix32-cc-abi-vaarg.ll
llvm/test/CodeGen/PowerPC/aix64-cc-abi-vaarg.ll
Index: llvm/test/CodeGen/PowerPC/aix64-cc-abi-vaarg.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix64-cc-abi-vaarg.ll
@@ -0,0 +1,357 @@
+; RUN: llc -O2 -mtriple powerpc64-ibm-aix-xcoff -stop-after=machine-cp -verify-machineinstrs < %s | \
+; RUN: FileCheck --check-prefixes=CHECK,64BIT %s
+
+; RUN: llc -O2 -verify-machineinstrs -mcpu=pwr4 -mattr=-altivec \
+; RUN: -mtriple powerpc64-ibm-aix-xcoff < %s | \
+; RUN: FileCheck --check-prefixes=CHECKASM,ASM64 %s
+
+ define i32 @int_va_arg(i32 %a, ...) local_unnamed_addr {
+ entry:
+%arg1 = alloca i8*, align 8
+%arg2 = alloca i8*, align 8
+%0 = bitcast i8** %arg1 to i8*
+call void @llvm.lifetime.start.p0i8(i64 8, i8* nonnull %0)
+%1 = bitcast i8** %arg2 to i8*
+call void @llvm.lifetime.start.p0i8(i64 8, i8* nonnull %1)
+call void @llvm.va_start(i8* nonnull %0)
+call void @llvm.va_copy(i8* nonnull %1, i8* nonnull %0)
+%2 = va_arg i8** %arg1, i32
+%add = add nsw i32 %2, %a
+%3 = va_arg i8** %arg2, i32
+%mul = shl i32 %3, 1
+%add3 = add nsw i32 %add, %mul
+call void @llvm.va_end(i8* nonnull %0)
+call void @llvm.va_end(i8* nonnull %1)
+call void @llvm.lifetime.end.p0i8(i64 8, i8* nonnull %1)
+call void @llvm.lifetime.end.p0i8(i64 8, i8* nonnull %0)
+ret i32 %add3
+ }
+
+ declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture)
+ declare void @llvm.va_start(i8*)
+ declare void @llvm.va_copy(i8*, i8*)
+ declare void @llvm.va_end(i8*)
+ declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture)
+
+; 64BIT-LABEL: name:int_va_arg
+; 64BIT-LABEL: liveins:
+; 64BIT-DAG: - { reg: '$x3', virtual-reg: '' }
+; 64BIT-DAG: - { reg: '$x4', virtual-reg: '' }
+; 64BIT-DAG: - { reg: '$x5', virtual-reg: '' }
+; 64BIT-DAG: - { reg: '$x6', virtual-reg: '' }
+; 64BIT-DAG: - { reg: '$x7', virtual-reg: '' }
+; 64BIT-DAG: - { reg: '$x8', virtual-reg: '' }
+; 64BIT-DAG: - { reg: '$x9', virtual-reg: '' }
+; 64BIT-DAG: - { reg: '$x10', virtual-reg: '' }
+
+; 64BIT-LABEL: fixedStack:
+; 64BIT-DAG: - { id: 0, type: default, offset: 56, size: 8
+
+; 64BIT-LABEL: stack:
+; 64BIT-DAG: - { id: 0, name: arg1, type: default, offset: 0, size: 8
+; 64BIT-DAG: - { id: 1, name: arg2, type: default, offset: 0, size: 8
+
+; 64BIT-LABEL: body: |
+; 64BIT-DAG: bb.0.entry:
+; 64BIT-DAG: liveins: $x3, $x4, $x5, $x6, $x7, $x8, $x9, $x10
+; 64BIT-DAG: STD killed renamable $x4, 0, %fixed-stack.0 :: (store 8 into %fixed-stack.0)
+; 64BIT-DAG: STD killed renamable $x5, 8, %fixed-stack.0 :: (store 8 into %fixed-stack.0 + 8)
+; 64BIT-DAG: STD killed renamable $x6, 16, %fixed-stack.0 :: (store 8)
+; 64BIT-DAG: STD killed renamable $x7, 24, %fixed-stack.0 :: (store 8)
+; 64BIT-DAG: STD killed renamable $x8, 32, %fixed-stack.0 :: (store 8)
+; 64BIT-DAG: STD killed renamable $x9, 40, %fixed-stack.0 :: (store 8)
+; 64BIT-DAG: STD killed renamable $x10, 48, %fixed-stack.0 :: (store 8)
+; 64BIT-DAG: renamable $x11 = ADDI8 %fixed-stack.0, 0
+; 64BIT-DAG: STD renamable $x11, 0, %stack.1.arg2 :: (store 8 into %ir.1)
+; 64BIT-DAG: renamable $x4 = LD 0, %stack.1.arg2 :: (load 8 from %ir.arg2)
+; 64BIT-DAG: renamable $x7 = ADDI8 renamable $x4, 4
+; 64BIT-DAG: renamable $x5 = ADDI8 %fixed-stack.0, 4
+; 64BIT-DAG: renamable $r6 = LWZ 0, %fixed-stack.0 :: (load 4 from %fixed-stack.0, align 8)
+; 64BIT-DAG: STD killed renamable $x11, 0, %stack.0.arg1 :: (store 8 into %ir.0)
+; 64BIT-DAG: STD killed renamable $x5, 0, %stack.0.arg1 :: (store 8 into %ir.arg1)
+; 64BIT-DAG: STD killed renamable $x7, 0, %stack.1.arg2 :: (store 8 into %ir.arg2)
+; 64BIT-DAG: renamable $r4 = LWZ 0, killed renamable $x4 :: (load 4)
+; 64BIT-DAG: renamable $r3 = nsw ADD4 killed renamable $r6, renamable $r3, implicit killed $x3
+; 64BIT-DAG: renamable $r4 = RLWINM killed renamable $r4, 1, 0, 30
+; 64BIT-DAG: renamable $r3 = nsw ADD4 killed renamable $r3, killed renamable $r4, implicit-def $x3
+; 64BIT-DAG: BLR8 implicit $lr8, implicit $rm, implicit $x3
+
+; ASM64-LABEL: .int_va_arg:
+; ASM64-DAG: std 4, 56(1)
+; ASM64-DAG: addi 4, 1, 56
+; ASM64-DAG: std 4, -16(1)
+; ASM64-DAG: std 4, -8(1)
+; ASM64-DAG: ld 4, -16(1)
+; ASM64-DAG: s
[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang
jasonliu 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 + 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. 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
jasonliu 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: > sfertile wrote: > > 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. > It wasn't clear for me from the code that this is not a permanent thing. In > that case, does it make sense to leave a TODO here and say we need to > re-evaluate this in the future to decide if we want to support it or not on > AIX? > On another note, I'm not sure if this is really needed on AIX target though. > But I guess we could discuss it down the road. Just to avoid ambiguity, I meant I'm not sure if we really need this *option* on AIX. 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
jasonliu 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 + sfertile wrote: > 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. It wasn't clear for me from the code that this is not a permanent thing. In that case, does it make sense to leave a TODO here and say we need to re-evaluate this in the future to decide if we want to support it or not on AIX? On another note, I'm not sure if this is really needed on AIX target though. But I guess we could discuss it down the road. 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] D76932: [AIX] emit .extern and .weak directive linkage
jasonliu added inline comments.
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:441
+ case GlobalValue::ExternalWeakLinkage:
+if (TM.getTargetTriple().isOSBinFormatXCOFF()) {
+ OutStreamer->emitSymbolAttribute(GVSym, MCSA_Weak);
Maybe an assert on isOSBinFormatXCOFF is better?
If not, could we remove the else and llvm_unreachable to let it fall through?
Will it have warnings?
Or we could just only remove `else` here.
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1496
+
+MCSymbol *Name = getSymbol(&F);
+if (TM.getTargetTriple().isOSBinFormatXCOFF() && !F.isIntrinsic()) {
This block of code and D78045 will have conflict. One of us will need to rebase.
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1641
// External global variables are already handled.
+ if (GV->isDeclaration()) {
This comment does not apply any more.
Comment at: llvm/test/CodeGen/PowerPC/aix-extern-weak.ll:10
+; RUN: llvm-readobj --symbols %t.o | FileCheck --check-prefix=CHECKSYM %s
+
+@foo_ext_weak_p = global void (...)* bitcast (void ()* @foo_ext_weak_ref to
void (...)*)
Do we care to check the 64 bit object generation error here and for other test
cases that are applicable?
Comment at: llvm/test/CodeGen/PowerPC/aix-extern-weak.ll:29
+; COMMON-NEXT:.globl .main
+; COMMON-NEXT: .align4
+; COMMON-NEXT: .csect main[DS]
Could we align the assembly output in aix-extern-weak.ll, aix-extern.ll,
aix-weak.ll?
Comment at: llvm/test/CodeGen/PowerPC/aix-extern-weak.ll:48
+; COMMON-NEXT:.weak b_w[UA]
+; COMMON-NEXT:.weak foo_ext_weak_ref[DS]
+; COMMON-NEXT:.weak .foo_ext_weak
If the plan is not emit function entry point if it's not necessary, let's
CHECK-NOT that.
Comment at: llvm/test/CodeGen/PowerPC/aix-extern.ll:14
+
+; Function Attrs: noinline nounwind optnone
+define void @foo() #0 {
jasonliu wrote:
> nit: Function Attrs and `#0` could be removed
This comment is not addressed.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76932/new/
https://reviews.llvm.org/D76932
___
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
jasonliu added inline comments.
Comment at: clang/lib/AST/ItaniumMangle.cpp:5217
+ else
+Mangler.getStream() << D->getName();
+}
If I understand correctly, this function will come in pair with
`__cxx_global_var_init`.
`__cxx_global_var_init` use number as suffix in the end to avoid duplication
when there is more than one of those, but we are using the variable name as
suffix here instead.
Do we want to use number as suffix here to match what `__cxx_global_var_init`
does? It would help people to recognize the pairs and make them more symmetric.
Comment at: clang/lib/CodeGen/CGCXXABI.h:113
+
+ virtual bool isCXXGlobalInitAndDtorFuncInternal() const { return true; }
+
Do we need this isCXXGlobalInitAndDtorFuncInternal? Looks like it's always
going to return ! useSinitAndSterm() result.
Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:586
+ bool UseSinitAndSterm = getCXXABI().useSinitAndSterm();
+ if (UseSinitAndSterm) {
`const` for UseSinitAndSterm variable?
Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:638
+ // Create our global initialization function.
+ if (!CXXGlobalInits.empty()) {
+// Include the filename in the symbol name. When not using sinit and sterm
flip this for an early return?
Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:691
+ if (UseSinitAndSterm) {
+std::string FuncSuffix = GlobalUniqueModuleId;
+Fn = CreateGlobalInitOrDestructFunction(
We might want to avoid this string copy construction if possible.
Comment at: clang/lib/CodeGen/CodeGenModule.h:20
#include "SanitizerMetadata.h"
+#include "clang/AST/Attr.h"
#include "clang/AST/DeclCXX.h"
Is this Attr.h needed somewhere in this file?
Comment at: clang/lib/CodeGen/CodeGenModule.h:401
+ /// A unique trailing identifier as a part of sinit/sterm function when
+ /// UserSinitAndSterm set as true.
+ std::string GlobalUniqueModuleId;
nit: UserSinitAndSterm -> UseSinitAndSterm?
and we do not have that property here.
Suggestion:
when getCXXABI().useSinitAndSterm() return true.
Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4446
+
+ // Create __srterm function for the var decl.
+ llvm::Function *dtorStub = CGF.createAtExitStub(D, dtor, addr);
In this implementation we do not seem to have `__srterm` functions. I think we
should refer to `__dtor` instead.
Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4479
+
+ llvm::Value *NeedsDestruct = CGF.Builder.CreateIsNull(V, "guard.hasSrterm");
+
Srterm is not used in this implementation.
Suggestion:
guard.hasDtorCalled
Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4481
+
+ llvm::BasicBlock *DestructCheckBlock =
CGF.createBasicBlock("destruct.check");
+ llvm::BasicBlock *EndBlock = CGF.createBasicBlock("destruct.end");
I think we need a better naming for this and make it consistent with the end
block below.
As it is for now, `destruct.check` is confusing, as we are not checking
anything here and we are just going to call destructor directly in this block.
Comment at: clang/test/CodeGen/static-init.cpp:10
~test();
} t;
Would it be better to test static init for at least two global variable?
Testing only one global variable does not show the whole picture of AIX static
init.
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
jasonliu added inline comments. Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:305 + if (llvm::Function *unatexitFn = + dyn_cast(unatexit.getCallee())) +unatexitFn->setDoesNotThrow(); Is there a valid case that unatexit.getCallee() returns a type which could not be cast to llvm::Function? i.e. Could we use cast instead of dyn_cast? 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
jasonliu added a comment. -fregister_global_dtors_with_atexit does not seem to work properly in current implementation. We should consider somehow disabling/report_fatal_error it instead of letting it generate invalid code on AIX. Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:594 + "Cannot produce a unique identifier for this module based on strong " + "external symbols."); +GlobalUniqueModuleId = GlobalUniqueModuleId.substr(1); I think report_fatal_error is more appropriate here? Since we know some case will trigger this, and we do not want those case to silently pass in non-assertion mode? Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4452 + + // Emit __sterm function to unregister __srterm and call __srterm. + emitCXXGlobalVarDeclDestructFunc(D, dtorStub, addr); This comment is confusing. This is not emitting `__sterm` function, this is emitting `__cxx_global_var_destruct` functions. And again, we do not have `__srterm` function in this implementation. Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4468 + const CGFunctionInfo &FI = CGM.getTypes().arrangeNullaryFunction(); + llvm::Function *StermFn = CGM.CreateGlobalInitOrDestructFunction( + FTy, FnName.str(), FI, D.getLocation()); nit: this is not sterm function. 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] D79719: [AIX] Implement AIX special alignment rule about double/long double
jasonliu added inline comments.
Comment at: clang/include/clang/AST/RecordLayout.h:75
+ // can be different than Alignment in cases where it is beneficial for
+ // performance.
+ CharUnits PreferredAlignment;
nit for comment: I don't think it's related to performance nowadays, and it's
more for ABI-compat reason.
Comment at: clang/lib/AST/ASTContext.cpp:2409
+return std::max(
+ABIAlign, (unsigned)toBits(getASTRecordLayout(RD).PreferredAlignment));
+ }
static_cast instead of c cast?
Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:826
+static bool isAIXLayout(const ASTContext &Context) {
+ return Context.getTargetInfo().getTriple().getOS() == llvm::Triple::AIX;
+}
We added supportsAIXPowerAlignment, so let's make use of that.
We could replace every call to isAIXLayout with supportsAIXPowerAlignment. This
name is more expressive as well.
Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1225
+ CharUnits PreferredAlign =
+ (Packed && ((Context.getLangOpts().getClangABICompat() <=
+ LangOptions::ClangABI::Ver6) ||
Use a lambda for this query would be more preferable if same logic get used
twice.
Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1881
+ if (isAIXLayout(Context) && FieldOffset == CharUnits::Zero() &&
+ (IsUnion || NonOverlappingEmptyFieldFound)) {
+FirstNonOverlappingEmptyFieldHandled = true;
Maybe it's a naive thought, but is it possible to replace
`NonOverlappingEmptyFieldFound` with `IsOverlappingEmptyField &&
FieldOffsets.size() == 0`?
Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1943
getDataSize() != CharUnits::Zero())
FieldOffset = getDataSize().alignTo(FieldAlign);
else
hmm... Are we sure we should use FieldAlign instead of PreferredAlign here?
Same for the else below.
Comment at: clang/lib/Basic/Targets/PPC.h:377
+if (Triple.isOSAIX()) {
+ LongDoubleWidth = 64;
nit: use "else if" with the above if statement?
If it enters one of the Triples above, it's not necessary to check if it is AIX
any more.
Comment at: clang/lib/Basic/Targets/PPC.h:411
if (Triple.getOS() == llvm::Triple::AIX)
SuitableAlign = 64;
This could get combined with the new if for AIX below.
Comment at: clang/lib/Basic/Targets/PPC.h:419
+if (Triple.isOSAIX()) {
+ LongDoubleWidth = 64;
nit: use "else if" with the above if statement?
Comment at: clang/test/Layout/aix-no-unique-address-with-double.cpp:62
+
+ [[no_unique_address]] Empty emp;
+ A a;
Not an action item, but just an interesting note that, in some cases(like this
one) power alignment rule could reverse the benefit of having
[[no_unique_address]] at the first place.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79719/new/
https://reviews.llvm.org/D79719
___
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
jasonliu added inline comments.
Comment at: clang/lib/CodeGen/CGCXXABI.h:113
+
+ virtual bool isCXXGlobalInitAndDtorFuncInternal() const { return true; }
+
Xiangling_L wrote:
> jasonliu wrote:
> > Do we need this isCXXGlobalInitAndDtorFuncInternal? Looks like it's always
> > going to return ! useSinitAndSterm() result.
> AFAIK, not all platforms which use sinit and sterm have external linkage
> sinit/sterm functions. And also since for 7.2 AIX we are considering change
> sinit/sterm to internal linkage symbols, I seperate this query out.
I would prefer to create the query in the patch when we actually need it. With
what we have right now, it seems redundant.
Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:650
+ FTy,
+ (UseSinitAndSterm ? "__sinit8000_clang_" : "_GLOBAL__sub_I_") +
+ FuncSuffix,
It looks like one of the UseSinitAndSterm is redundant. We could have something
like:
```
UseSinitAndSterm ? llvm::twine("__sinit8000_clang_") + GlobalUniqueModuleId
: llvm::twine("_GLOBAL__sub_I_") + getTransformedFileName(getModule())
```
which minimize the use of std::string?
If this is too long, then we could separate them into if statement just like
what we have in EmitCXXGlobalDtorFunc, or use a lambda.
Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:692
+ if (UseSinitAndSterm) {
+Fn = CreateGlobalInitOrDestructFunction(
+FTy, llvm::Twine("__sterm8000_clang_") + GlobalUniqueModuleId, FI,
Could we assert !GlobalUniqueModuleId.empty() here?
Right now, it solely relies on `EmitCXXGlobalInitFunc` to run before
`EmitCXXGlobalDtorFunc` to get `GlobalUniqueModuleId` initialized properly. I
think it makes sense to put an assert here to make sure it stays in that case
in the future.
Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:699
+
+ if (!getCXXABI().isCXXGlobalInitAndDtorFuncInternal())
+Fn->setLinkage(llvm::Function::ExternalLinkage);
This could go inside of the `if (UseSinitAndSterm)`.
Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4479
+
+ llvm::Value *NeedsDestruct = CGF.Builder.CreateIsNull(V, "guard.hasSrterm");
+
jasonliu wrote:
> Srterm is not used in this implementation.
> Suggestion:
> guard.hasDtorCalled
"guard.hasDtor" does not reflect what the meaning. Maybe matching with the
variable name would make sense: "guard.needsDestruct" or just "needsDestruct"?
Comment at: clang/test/CodeGen/static-init.cpp:15
+
+// CHECK: define internal void @__cxx_global_var_init() #0 {
+// CHECK: entry:
#0 could be removed.
Repository:
rL LLVM
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
jasonliu added inline comments.
Comment at: clang/test/CodeGen/static-init.cpp:8
+// RUN: FileCheck %s
struct test {
Looks like the non-inline comments are easier to get ignored and missed, so I
will copy paste the non-inlined comment I previously had:
```
-fregister_global_dtors_with_atexit does not seem to work properly in current
implementation.
We should consider somehow disabling/report_fatal_error it instead of letting
it generate invalid code on AIX.
```
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] D81972: [NFC] Cleanup of EmitCXXGlobalInitFunc() and EmitCXXGlobalDtorFunc()
jasonliu added inline comments. Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:596 } + // Include the filename in the symbol name. Including "sub_" matches gcc I think this patch is missing what @hubert.reinterpretcast mentioned in https://reviews.llvm.org/D74166?id=269900#inline-751064 which is an early return like this: ``` if (CXXGlobalInits.empty()) return; ``` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81972/new/ https://reviews.llvm.org/D81972 ___ 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
jasonliu added inline comments.
Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:708
+" based on strong external symbols");
+ GlobalUniqueModuleId = GlobalUniqueModuleId.substr(1);
+}
Correct me if I'm wrong...
`GlobalUniqueModuleId` will always get “initialized" in
`EmitCXXGlobalInitFunc`. And `EmitCXXGlobalInitFunc` will always run before
`EmitCXXGlobalCleanUpFunc` in `CodeGenModule::Release()`. So in theory, we do
not need to initialize it again in here.
If we could not `assert (!GlobalUniqueModuleId.empty())` here, that tells me in
`EmitCXXGlobalInitFunc` we effectively get an empty string after
`GlobalUniqueModuleId = GlobalUniqueModuleId.substr(1);`. So something is not
right?
Comment at: clang/lib/CodeGen/CodeGenModule.h:471
- /// Global destructor functions and arguments that need to run on
termination.
+ /// Global destructor functions and arguments that need to run on
termination;
+ /// When UseSinitAndSterm is set, it instead contains sterm finalizer
Word after `;` should not be Capitalize. We should use small case, or change
this to `.`.
I would prefer changing it to `.`.
Comment at: clang/test/CodeGen/static-init.cpp:142
+
+// CHECK: ; Function Attrs: noinline nounwind
+// CHECK: define internal void @__finalize__ZZN5test41fEvE11staticLocal() #0 {
Is checking this Attrs necessary?
Comment at: clang/test/CodeGen/static-init.cpp:157
+
+// CHECK: define void
@__sinit8000_clang_1145401da454a7baad10bfe313c46638() #5 {
+// CHECK: entry:
I think we used to have dso_local marked on sinit and sterm function in
previous diff. Now they are gone. What's the reason for removing them?
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
jasonliu added inline comments.
Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:835
+
+ llvm::CallInst *CI = nullptr;
+ if (Arg == nullptr) {
nit: trailing whitespace
Comment at: clang/test/CodeGen/static-init.cpp:31
+namespace test4 {
+ struct Test4 {
+Test4();
nit: trailing whitespace in this line.
struct Test4 {
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] D81972: [NFC] Cleanup of EmitCXXGlobalInitFunc() and EmitCXXGlobalDtorFunc()
jasonliu added inline comments. Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:596 } + // Include the filename in the symbol name. Including "sub_" matches gcc jasonliu wrote: > I think this patch is missing what @hubert.reinterpretcast mentioned in > https://reviews.llvm.org/D74166?id=269900#inline-751064 > which is an early return like this: > > ``` > if (CXXGlobalInits.empty()) > return; > ``` Please double check the above early return is desired though. It seems even when CXXGlobalInits is empty, `GenerateCXXGlobalInitFunc` is trying to do a lot of things with `Fn` passed in. Later, we also called `AddGlobalCtor(Fn)`. So a lot of behavior changes here, we want to make sure it's really 'NFC'. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81972/new/ https://reviews.llvm.org/D81972 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D81972: [NFC] Cleanup of EmitCXXGlobalInitFunc() and EmitCXXGlobalDtorFunc()
jasonliu added inline comments.
Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:596
}
+ // Include the filename in the symbol name. Including "sub_" matches gcc
jasonliu wrote:
> jasonliu wrote:
> > I think this patch is missing what @hubert.reinterpretcast mentioned in
> > https://reviews.llvm.org/D74166?id=269900#inline-751064
> > which is an early return like this:
> >
> > ```
> > if (CXXGlobalInits.empty())
> > return;
> > ```
> Please double check the above early return is desired though. It seems even
> when CXXGlobalInits is empty, `GenerateCXXGlobalInitFunc` is trying to do a
> lot of things with `Fn` passed in. Later, we also called `AddGlobalCtor(Fn)`.
> So a lot of behavior changes here, we want to make sure it's really 'NFC'.
FYI,
```
class test {
public:
test();
};
test t1 __attribute__((init_priority (300)));
```
Compile with `clang -target x86_64-apple-darwin10 -emit-llvm `
With this change, I guess we will not generate @_GLOBAL__sub_I_d.cpp(), and
also that function will not get added into @llvm.global_ctors.
I'm not sure if we really need @_GLOBAL__sub_I_d.cpp() on that platform in this
case. But this change does not really qualify as NFC.
If we need to do this change, a test case is necessary, and we better ask
people who familiar with the platform to confirm it's a desired change.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81972/new/
https://reviews.llvm.org/D81972
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D81972: [NFC] Cleanup of EmitCXXGlobalInitFunc() and EmitCXXGlobalDtorFunc()
jasonliu added a comment. New diff does not have context available. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81972/new/ https://reviews.llvm.org/D81972 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D81972: [NFC] Cleanup of EmitCXXGlobalInitFunc() and EmitCXXGlobalDtorFunc()
jasonliu accepted this revision.
jasonliu added a comment.
This revision is now accepted and ready to land.
LGTM.
Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:596
}
+ // Include the filename in the symbol name. Including "sub_" matches gcc
Xiangling_L wrote:
> jasonliu wrote:
> > jasonliu wrote:
> > > jasonliu wrote:
> > > > I think this patch is missing what @hubert.reinterpretcast mentioned in
> > > > https://reviews.llvm.org/D74166?id=269900#inline-751064
> > > > which is an early return like this:
> > > >
> > > > ```
> > > > if (CXXGlobalInits.empty())
> > > > return;
> > > > ```
> > > Please double check the above early return is desired though. It seems
> > > even when CXXGlobalInits is empty, `GenerateCXXGlobalInitFunc` is trying
> > > to do a lot of things with `Fn` passed in. Later, we also called
> > > `AddGlobalCtor(Fn)`. So a lot of behavior changes here, we want to make
> > > sure it's really 'NFC'.
> > FYI,
> > ```
> > class test {
> > public:
> > test();
> > };
> > test t1 __attribute__((init_priority (300)));
> > ```
> > Compile with `clang -target x86_64-apple-darwin10 -emit-llvm `
> > With this change, I guess we will not generate @_GLOBAL__sub_I_d.cpp(), and
> > also that function will not get added into @llvm.global_ctors.
> > I'm not sure if we really need @_GLOBAL__sub_I_d.cpp() on that platform in
> > this case. But this change does not really qualify as NFC.
> > If we need to do this change, a test case is necessary, and we better ask
> > people who familiar with the platform to confirm it's a desired change.
> Thanks, I am gonna remove this part and keep it as a NFC patch.
Sure. Please update D74166 accordingly.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81972/new/
https://reviews.llvm.org/D81972
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D81270: [XCOFF][AIX] Use 'L..' instead of '.L' for getPrivateGlobalPrefix in DataLayout
This revision was automatically updated to reflect the committed changes.
Closed by commit rG572dde55eebb: [XCOFF][AIX] Use 'L..' instead of
'.L' for getPrivateGlobalPrefix in DataLayout (authored by jasonliu).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81270/new/
https://reviews.llvm.org/D81270
Files:
clang/lib/Basic/Targets/PPC.h
llvm/docs/LangRef.rst
llvm/include/llvm/IR/DataLayout.h
llvm/lib/IR/DataLayout.cpp
llvm/test/CodeGen/PowerPC/aix-lower-constant-pool-index.ll
llvm/test/CodeGen/PowerPC/aix-lower-jump-table.ll
llvm/test/CodeGen/PowerPC/aix-xcoff-mergeable-const.ll
llvm/test/CodeGen/PowerPC/aix-xcoff-mergeable-str.ll
llvm/unittests/IR/ManglerTest.cpp
Index: llvm/unittests/IR/ManglerTest.cpp
===
--- llvm/unittests/IR/ManglerTest.cpp
+++ llvm/unittests/IR/ManglerTest.cpp
@@ -136,4 +136,24 @@
"?vectorcall");
}
+TEST(ManglerTest, XCOFF) {
+ LLVMContext Ctx;
+ DataLayout DL("m:a"); // XCOFF/AIX
+ Module Mod("test", Ctx);
+ Mod.setDataLayout(DL);
+ Mangler Mang;
+ EXPECT_EQ(mangleStr("foo", Mang, DL), "foo");
+ EXPECT_EQ(mangleStr("\01foo", Mang, DL), "foo");
+ EXPECT_EQ(mangleStr("?foo", Mang, DL), "?foo");
+ EXPECT_EQ(mangleFunc("foo", llvm::GlobalValue::ExternalLinkage,
+ llvm::CallingConv::C, Mod, Mang),
+"foo");
+ EXPECT_EQ(mangleFunc("?foo", llvm::GlobalValue::ExternalLinkage,
+ llvm::CallingConv::C, Mod, Mang),
+"?foo");
+ EXPECT_EQ(mangleFunc("foo", llvm::GlobalValue::PrivateLinkage,
+ llvm::CallingConv::C, Mod, Mang),
+"L..foo");
+}
+
} // end anonymous namespace
Index: llvm/test/CodeGen/PowerPC/aix-xcoff-mergeable-str.ll
===
--- llvm/test/CodeGen/PowerPC/aix-xcoff-mergeable-str.ll
+++ llvm/test/CodeGen/PowerPC/aix-xcoff-mergeable-str.ll
@@ -27,20 +27,20 @@
; CHECK: .csect .rodata.str2.2[RO],2
; CHECK-NEXT: .align 1
-; CHECK-NEXT: .Lmagic16:
+; CHECK-NEXT: L..magic16:
; CHECK-NEXT: .vbyte 2, 264 # 0x108
; CHECK-NEXT: .vbyte 2, 272 # 0x110
; CHECK-NEXT: .vbyte 2, 213 # 0xd5
; CHECK-NEXT: .vbyte 2, 0 # 0x0
; CHECK-NEXT: .csect .rodata.str4.4[RO],2
; CHECK-NEXT: .align 2
-; CHECK-NEXT: .Lmagic32:
+; CHECK-NEXT: L..magic32:
; CHECK-NEXT: .vbyte 4, 464 # 0x1d0
; CHECK-NEXT: .vbyte 4, 472 # 0x1d8
; CHECK-NEXT: .vbyte 4, 413 # 0x19d
; CHECK-NEXT: .vbyte 4, 0 # 0x0
; CHECK-NEXT: .csect .rodata.str1.1[RO],2
-; CHECK-NEXT: .LstrA:
+; CHECK-NEXT: L..strA:
; CHECK-NEXT: .byte 104
; CHECK-NEXT: .byte 101
; CHECK-NEXT: .byte 108
@@ -55,7 +55,7 @@
; CHECK-NEXT: .byte 33
; CHECK-NEXT: .byte 10
; CHECK-NEXT: .byte 0
-; CHECK-NEXT: .L.str:
+; CHECK-NEXT: L...str:
; CHECK-NEXT: .byte 97
; CHECK-NEXT: .byte 98
; CHECK-NEXT: .byte 99
Index: llvm/test/CodeGen/PowerPC/aix-xcoff-mergeable-const.ll
===
--- llvm/test/CodeGen/PowerPC/aix-xcoff-mergeable-const.ll
+++ llvm/test/CodeGen/PowerPC/aix-xcoff-mergeable-const.ll
@@ -25,7 +25,7 @@
;CHECK: .csect .rodata[RO],4
;CHECK-NEXT: .align 4
-;CHECK-NEXT: .L__const.main.cnst32:
+;CHECK-NEXT: L..__const.main.cnst32:
;CHECK32-NEXT: .vbyte 4, 1073741824
;CHECK32-NEXT: .vbyte 4, 50
;CHECK64-NEXT: .vbyte 8, 4611686018427387954
@@ -38,7 +38,7 @@
;CHECK-NEXT: .space 4
;CHECK-NEXT: .align 3
-;CHECK-NEXT: .L__const.main.cnst16:
+;CHECK-NEXT: L..__const.main.cnst16:
;CHECK32-NEXT: .vbyte 4, 1073741824
;CHECK32-NEXT: .vbyte 4, 22
;CHECK64-NEXT: .vbyte 8, 4611686018427387926
@@ -46,12 +46,12 @@
;CHECK-NEXT: .space 4
;CHECK-NEXT: .align 3
-;CHECK-NEXT: .L__const.main.cnst8:
+;CHECK-NEXT: L..__const.main.cnst8:
;CHECK-NEXT: .vbyte 4, 1073741832 # 0x4008
;CHECK-NEXT: .vbyte 4, 0 # 0x0
;CHECK-NEXT: .align 3
-;CHECK-NEXT: .L__const.main.cnst4:
+;CHECK-NEXT: L..__const.main.cnst4:
;CHECK-NEXT: .vbyte 2, 16392 # 0x4008
;CHECK-NEXT: .byte 0 # 0x0
;CHECK-NEXT: .space 1
Index: llvm/test/CodeGen/PowerPC/aix-lower-jump-table.ll
===
--- llvm/test/CodeGen/PowerPC/aix-lower-jump-table.ll
+++ llvm/test/CodeGen/PowerPC/aix-lower-jump-table.ll
@@ -98,11 +98,11 @@
; 32SMALL-ASM: blr
; 32SMALL-ASM: .csect .rodata[RO],2
; 32SMALL-ASM: .align 2
-; 32SMALL-ASM: .LJTI0_0:
-; 32SMALL-
[PATCH] D82476: [Clang][Driver] Recognize the AIX OBJECT_MODE environment setting
jasonliu accepted this revision. jasonliu 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/D82476/new/ https://reviews.llvm.org/D82476 ___ 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
jasonliu added inline comments.
Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:345
+// rarely.
+Weights = nullptr;
+ } else if (Kind == GuardKind::VariableGuard && !D->isLocalVarDecl()) {
Do we need to change/complicate the interface for this function, just to do a
call to Builder.CreateCondBr()?
Could we call that function directly from where it's needed?
Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:648
- // Include the filename in the symbol name. Including "sub_" matches gcc and
- // makes sure these symbols appear lexicographically behind the symbols with
- // priority emitted above.
- SmallString<128> FileName = llvm::sys::path::filename(getModule().getName());
- if (FileName.empty())
-FileName = "";
+ // Create our global initialization function.
+ if (UseSinitAndSterm && CXXGlobalInits.empty())
This comment does not apply well on top of this early return for some platform.
I think you could move it to current line 665?
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
jasonliu accepted this revision. jasonliu added a comment. This revision is now accepted and ready to land. LGTM. But I suggest to wait for @hubert.reinterpretcast to have another scan at this before landing. 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] D79719: [AIX] Implement AIX special alignment rule about double/long double
jasonliu added inline comments.
Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1881
+ if (isAIXLayout(Context) && FieldOffset == CharUnits::Zero() &&
+ (IsUnion || NonOverlappingEmptyFieldFound)) {
+FirstNonOverlappingEmptyFieldHandled = true;
Xiangling_L wrote:
> jasonliu wrote:
> > Maybe it's a naive thought, but is it possible to replace
> > `NonOverlappingEmptyFieldFound` with `IsOverlappingEmptyField &&
> > FieldOffsets.size() == 0`?
> I don't think these two work the same. `NonOverlappingEmptyFieldFound`
> represents the 1st non-empty and non-overlapping field in the record.
> `IsOverlappingEmptyField && FieldOffsets.size() == 0` represents something
> opposite.
You are right. I meant could we replace it with `!(IsOverlappingEmptyField &&
FieldOffsets.size() == 0)`?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79719/new/
https://reviews.llvm.org/D79719
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double
jasonliu added inline comments.
Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1881
+ if (isAIXLayout(Context) && FieldOffset == CharUnits::Zero() &&
+ (IsUnion || NonOverlappingEmptyFieldFound)) {
+FirstNonOverlappingEmptyFieldHandled = true;
Xiangling_L wrote:
> jasonliu wrote:
> > Xiangling_L wrote:
> > > jasonliu wrote:
> > > > Maybe it's a naive thought, but is it possible to replace
> > > > `NonOverlappingEmptyFieldFound` with `IsOverlappingEmptyField &&
> > > > FieldOffsets.size() == 0`?
> > > I don't think these two work the same. `NonOverlappingEmptyFieldFound`
> > > represents the 1st non-empty and non-overlapping field in the record.
> > > `IsOverlappingEmptyField && FieldOffsets.size() == 0` represents
> > > something opposite.
> > You are right. I meant could we replace it with `!(IsOverlappingEmptyField
> > && FieldOffsets.size() == 0)`?
> I don't think so. The replacement does not work for the case:
>
>
> ```
> struct A {
> int : 0;
> double d;
> };
> ```
>
> where __alignof(A) should be 4;
Got it. Thanks!
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79719/new/
https://reviews.llvm.org/D79719
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double
jasonliu accepted this revision. jasonliu added a comment. LGTM. I have no further comments on this patch. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79719/new/ https://reviews.llvm.org/D79719 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D76932: [AIX] emit .extern and .weak directive linkage
jasonliu accepted this revision. jasonliu added a comment. This revision is now accepted and ready to land. LGTM. Please wait a day or two to see if @hubert.reinterpretcast have further comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D76932/new/ https://reviews.llvm.org/D76932 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D78506: [NFC] Common up TargetCodeGenInfo for all PowerPC target.
jasonliu created this revision.
jasonliu added reviewers: sfertile, nemanjai, hubert.reinterpretcast,
cebowleratibm.
Herald added subscribers: steven.zhang, shchenz.
jasonliu added a reviewer: PowerPC.
jasonliu added a comment.
To reviewers:
Although the patch is showing changes on various ABIInfo classes. But those
classes are actually untouched.
This patch only touches PowerPC TargetCodeGenInfo related classes.
There is little differences across all power pc related target for
TargetCodeGenInfo implementation.
The biggest difference is the different ABIInfo classes it takes.
But we don't necessarily need so many TargetCodeGenInfo for different PowerPC
architect.
So it might be beneficial to common them up as one PPCTargetCodeGenInfo.
https://reviews.llvm.org/D78506
Files:
clang/lib/CodeGen/TargetInfo.cpp
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -4172,8 +4172,9 @@
/*allowHigherAlign*/ false);
}
-// PowerPC-32
+// PowerPC
namespace {
+// PowerPC-32
/// PPC32_SVR4_ABIInfo - The 32-bit PowerPC ELF (SVR4) ABI information.
class PPC32_SVR4_ABIInfo : public DefaultABIInfo {
bool IsSoftFloatABI;
@@ -4188,10 +4189,136 @@
QualType Ty) const override;
};
-class PPC32TargetCodeGenInfo : public TargetCodeGenInfo {
+// PowerPC-64
+/// PPC64_SVR4_ABIInfo - The 64-bit PowerPC ELF (SVR4) ABI information.
+class PPC64_SVR4_ABIInfo : public SwiftABIInfo {
+public:
+ enum ABIKind {
+ELFv1 = 0,
+ELFv2
+ };
+
+private:
+ static const unsigned GPRBits = 64;
+ ABIKind Kind;
+ bool HasQPX;
+ bool IsSoftFloatABI;
+
+ // A vector of float or double will be promoted to <4 x f32> or <4 x f64> and
+ // will be passed in a QPX register.
+ bool IsQPXVectorTy(const Type *Ty) const {
+if (!HasQPX)
+ return false;
+
+if (const VectorType *VT = Ty->getAs()) {
+ unsigned NumElements = VT->getNumElements();
+ if (NumElements == 1)
+return false;
+
+ if (VT->getElementType()->isSpecificBuiltinType(BuiltinType::Double)) {
+if (getContext().getTypeSize(Ty) <= 256)
+ return true;
+ } else if (VT->getElementType()->
+ isSpecificBuiltinType(BuiltinType::Float)) {
+if (getContext().getTypeSize(Ty) <= 128)
+ return true;
+ }
+}
+
+return false;
+ }
+
+ bool IsQPXVectorTy(QualType Ty) const {
+return IsQPXVectorTy(Ty.getTypePtr());
+ }
+
+public:
+ PPC64_SVR4_ABIInfo(CodeGen::CodeGenTypes &CGT, ABIKind Kind, bool HasQPX,
+ bool SoftFloatABI)
+ : SwiftABIInfo(CGT), Kind(Kind), HasQPX(HasQPX),
+IsSoftFloatABI(SoftFloatABI) {}
+
+ bool isPromotableTypeForABI(QualType Ty) const;
+ CharUnits getParamTypeAlignment(QualType Ty) const;
+
+ ABIArgInfo classifyReturnType(QualType RetTy) const;
+ ABIArgInfo classifyArgumentType(QualType Ty) const;
+
+ bool isHomogeneousAggregateBaseType(QualType Ty) const override;
+ bool isHomogeneousAggregateSmallEnough(const Type *Ty,
+ uint64_t Members) const override;
+
+ // TODO: We can add more logic to computeInfo to improve performance.
+ // Example: For aggregate arguments that fit in a register, we could
+ // use getDirectInReg (as is done below for structs containing a single
+ // floating-point value) to avoid pushing them to memory on function
+ // entry. This would require changing the logic in PPCISelLowering
+ // when lowering the parameters in the caller and args in the callee.
+ void computeInfo(CGFunctionInfo &FI) const override {
+if (!getCXXABI().classifyReturnType(FI))
+ FI.getReturnInfo() = classifyReturnType(FI.getReturnType());
+for (auto &I : FI.arguments()) {
+ // We rely on the default argument classification for the most part.
+ // One exception: An aggregate containing a single floating-point
+ // or vector item must be passed in a register if one is available.
+ const Type *T = isSingleElementStruct(I.type, getContext());
+ if (T) {
+const BuiltinType *BT = T->getAs();
+if (IsQPXVectorTy(T) ||
+(T->isVectorType() && getContext().getTypeSize(T) == 128) ||
+(BT && BT->isFloatingPoint())) {
+ QualType QT(T, 0);
+ I.info = ABIArgInfo::getDirectInReg(CGT.ConvertType(QT));
+ continue;
+}
+ }
+ I.info = classifyArgumentType(I.type);
+}
+ }
+
+ Address EmitVAArg(CodeGenFunction &CGF, Address VAListAddr,
+QualType Ty) const override;
+
+ bool shouldPassIndirectlyForSwift(ArrayRef scalars,
+bool asReturnValue) const override {
+return occupiesMoreThan(CGT, scalars, /*total*/ 4);
+ }
+
+ bool isSwiftErrorInRegister() const override {
+return false;
+ }
+};
+
+class PPCTargetCodeGenInf
[PATCH] D78506: [NFC] Common up TargetCodeGenInfo for all PowerPC target.
jasonliu added a comment. To reviewers: Although the patch is showing changes on various ABIInfo classes. But those classes are actually untouched. This patch only touches PowerPC TargetCodeGenInfo related classes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78506/new/ https://reviews.llvm.org/D78506 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D78563: [AIX] Port power alignment rules to clang
jasonliu added inline comments. Comment at: clang/test/Layout/aix-double-struct-member.cpp:136 +// CHECK-NEXT:| nvsize=12, nvalign=4] +}; // namespace test4 1. I think we also want to test empty base class with a derived class contains double as first. And also a non-empty base class with derived contains double as first to show that it has effect on basic inheritance as well. 2. We need one more test for seeing how pack align interact with AIX alignment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78563/new/ https://reviews.llvm.org/D78563 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D78563: [AIX] Port power alignment rules to clang
jasonliu added inline comments. Comment at: clang/include/clang/AST/RecordLayout.h:177 + /// getAIXOffsetAlignment - Get the record of aixOffset alignment in + /// characters. Not sure if aixOffset is a thing? Might be better to just say "aix offset alignment"? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78563/new/ https://reviews.llvm.org/D78563 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D78563: [AIX] Port power alignment rules to clang
jasonliu added inline comments. Comment at: clang/include/clang/AST/RecordLayout.h:81 + /// AIXOffsetAlignment - The special AIX Alignment for the object that + /// contains floating-point member or sub-member. This is for AIX-abi only. AIX Alignment -> AIX alignment Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1013 + UpdateAlignment(BaseAlign, UnpackedBaseAlign, + /*AIXOffsetAlignment : used by AIX-ABI*/ + BaseAlign); Consistent spelling as: AIX-abi? Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1853 + // AIX abi has this special rule that in aggregates, the first member of + // floating point data type(or aggregate type contains floating point data Consistent spelling as AIX-abi? Comment at: clang/test/Layout/aix-double-struct-member.cpp:136 +// CHECK-NEXT:| nvsize=12, nvalign=4] +}; // namespace test4 jasonliu wrote: > 1. I think we also want to test empty base class with a derived class > contains double as first. And also a non-empty base class with derived > contains double as first to show that it has effect on basic inheritance as > well. > 2. We need one more test for seeing how pack align interact with AIX > alignment. We also want to test if MaxFieldAlignment could override the AIX alignment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78563/new/ https://reviews.llvm.org/D78563 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D79033: [NFC][clang] Replace raw new/delete with unique_ptr to store ABIInfo in TargetCodeGenInfo
jasonliu created this revision.
jasonliu added reviewers: hubert.reinterpretcast, sfertile, nemanjai,
Xiangling_L.
Herald added subscribers: s.egerton, simoncook, fedor.sergeev, aheejin, dschuff.
Herald added a project: clang.
Use unique_ptr to manage the lifetime of ABIInfo member inside
TargetCodeGenInfo.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D79033
Files:
clang/lib/CodeGen/TargetInfo.cpp
clang/lib/CodeGen/TargetInfo.h
Index: clang/lib/CodeGen/TargetInfo.h
===
--- clang/lib/CodeGen/TargetInfo.h
+++ clang/lib/CodeGen/TargetInfo.h
@@ -43,12 +43,11 @@
/// codegeneration issues, like target-specific attributes, builtins and so
/// on.
class TargetCodeGenInfo {
- ABIInfo *Info;
+ std::unique_ptr Info = nullptr;
public:
- // WARNING: Acquires the ownership of ABIInfo.
- TargetCodeGenInfo(ABIInfo *info = nullptr) : Info(info) {}
- virtual ~TargetCodeGenInfo();
+ TargetCodeGenInfo(std::unique_ptr Info)
+ : Info(std::move(Info)) {}
/// getABIInfo() - Returns ABI info helper for the target.
const ABIInfo &getABIInfo() const { return *Info; }
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -385,8 +385,6 @@
return Address(PHI, Align);
}
-TargetCodeGenInfo::~TargetCodeGenInfo() { delete Info; }
-
// If someone can figure out a general rule for this, that would be great.
// It's probably just doomed to be platform-dependent, though.
unsigned TargetCodeGenInfo::getSizeOfUnwindException() const {
@@ -682,7 +680,7 @@
class DefaultTargetCodeGenInfo : public TargetCodeGenInfo {
public:
DefaultTargetCodeGenInfo(CodeGen::CodeGenTypes &CGT)
-: TargetCodeGenInfo(new DefaultABIInfo(CGT)) {}
+ : TargetCodeGenInfo(std::make_unique(CGT)) {}
};
ABIArgInfo DefaultABIInfo::classifyArgumentType(QualType Ty) const {
@@ -772,7 +770,7 @@
public:
explicit WebAssemblyTargetCodeGenInfo(CodeGen::CodeGenTypes &CGT,
WebAssemblyABIInfo::ABIKind K)
- : TargetCodeGenInfo(new WebAssemblyABIInfo(CGT, K)) {}
+ : TargetCodeGenInfo(std::make_unique(CGT, K)) {}
void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
CodeGen::CodeGenModule &CGM) const override {
@@ -898,8 +896,8 @@
class PNaClTargetCodeGenInfo : public TargetCodeGenInfo {
public:
- PNaClTargetCodeGenInfo(CodeGen::CodeGenTypes &CGT)
-: TargetCodeGenInfo(new PNaClABIInfo(CGT)) {}
+ PNaClTargetCodeGenInfo(CodeGen::CodeGenTypes &CGT)
+ : TargetCodeGenInfo(std::make_unique(CGT)) {}
};
void PNaClABIInfo::computeInfo(CGFunctionInfo &FI) const {
@@ -1140,7 +1138,7 @@
X86_32TargetCodeGenInfo(CodeGen::CodeGenTypes &CGT, bool DarwinVectorABI,
bool RetSmallStructInRegABI, bool Win32StructABI,
unsigned NumRegisterParameters, bool SoftFloatABI)
- : TargetCodeGenInfo(new X86_32ABIInfo(
+ : TargetCodeGenInfo(std::make_unique(
CGT, DarwinVectorABI, RetSmallStructInRegABI, Win32StructABI,
NumRegisterParameters, SoftFloatABI)) {}
@@ -2334,7 +2332,7 @@
class X86_64TargetCodeGenInfo : public TargetCodeGenInfo {
public:
X86_64TargetCodeGenInfo(CodeGen::CodeGenTypes &CGT, X86AVXABILevel AVXLevel)
- : TargetCodeGenInfo(new X86_64ABIInfo(CGT, AVXLevel)) {}
+ : TargetCodeGenInfo(std::make_unique(CGT, AVXLevel)) {}
const X86_64ABIInfo &getABIInfo() const {
return static_cast(TargetCodeGenInfo::getABIInfo());
@@ -2478,7 +2476,7 @@
public:
WinX86_64TargetCodeGenInfo(CodeGen::CodeGenTypes &CGT,
X86AVXABILevel AVXLevel)
- : TargetCodeGenInfo(new WinX86_64ABIInfo(CGT, AVXLevel)) {}
+ : TargetCodeGenInfo(std::make_unique(CGT, AVXLevel)) {}
void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
CodeGen::CodeGenModule &CGM) const override;
@@ -4204,8 +4202,8 @@
public:
PPC32TargetCodeGenInfo(CodeGenTypes &CGT, bool SoftFloatABI,
bool RetSmallStructInRegABI)
- : TargetCodeGenInfo(new PPC32_SVR4_ABIInfo(CGT, SoftFloatABI,
- RetSmallStructInRegABI)) {}
+ : TargetCodeGenInfo(std::make_unique(
+CGT, SoftFloatABI, RetSmallStructInRegABI)) {}
static bool isStructReturnInRegABI(const llvm::Triple &Triple,
const CodeGenOptions &Opts);
@@ -4595,8 +4593,8 @@
PPC64_SVR4_TargetCodeGenInfo(CodeGenTypes &CGT,
PPC64_SVR4_ABIInfo::ABIKind Kind, bool HasQPX,
bool SoftFloatABI)
- : TargetCodeGenInfo(new PPC64_SVR4_ABIInfo(CGT, Kind, HasQPX,
- SoftFloatABI)) {}
+ : TargetCodeGe
[PATCH] D79035: [clang][AIX] Implement ABIInfo and TargetCodeGenInfo for AIX
jasonliu created this revision.
jasonliu added reviewers: Xiangling_L, sfertile, hubert.reinterpretcast,
cebowleratibm, ZarkoCA.
Herald added subscribers: kbarton, nemanjai.
Herald added a project: clang.
Created AIXABIInfo and AIXTargetCodeGenInfo for AIX ABI.
Some investigation and FAQ on why we created AIXABIInfo:
Use or derive PPC32/PPC64 ABIInfo for AIX:
There are a lot of subtle differences between PPC32 and PPC64 variation. For
AIX we do not have a huge differences between 32 bit and 64 bit for the ABI
rules. Which means if we decide to use PPC32 for 32 bit on AIX and PPC64 for 64
bit on AIX, we will need to add in a lot of target check to make them
symmetric. The code flow will be really hard to follow and verify for every
target that is trying to use them. And it’s easy for us to take a lot of
unwanted change that really is meant for SVR targets.
Use one ABIInfo class for both 32 bit and 64 bit on AIX will make the code much
clear and easier to follow through. And that was one of the reason for us to
create LowerCall_AIX/LowerFormalArgument_AIX instead of just rename and use the
existing SVR4/Darwin one.
Derive from DefaultABIInfo:
We are going to override most of the methods in DefaultABIInfo anyway if we
look at the analysis below.
Derive from SwiftABIInfo:
I don't think we want to claim Swift support on AIX right now.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D79035
Files:
clang/lib/CodeGen/TargetInfo.cpp
clang/test/CodeGen/aix-complex.c
clang/test/CodeGen/aix-return.c
clang/test/CodeGen/aix-struct-arg.c
clang/test/CodeGen/aix-vaargs.c
clang/test/CodeGen/aix-vector.c
clang/test/CodeGen/ppc32-and-aix-struct-return.c
clang/test/CodeGen/ppc32-dwarf.c
clang/test/CodeGen/ppc64-dwarf.c
Index: clang/test/CodeGen/ppc64-dwarf.c
===
--- clang/test/CodeGen/ppc64-dwarf.c
+++ clang/test/CodeGen/ppc64-dwarf.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple powerpc64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,PPC64
static unsigned char dwarf_reg_size_table[1024];
int test() {
@@ -119,10 +120,10 @@
// CHECK-NEXT: store i8 16, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 108), align 1
// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 109), align 1
// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 110), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 111), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 112), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 113), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 114), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 115), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 116), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 111), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 112), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 113), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 114), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 115), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 116), align 1
// CHECK-NEXT: ret i32 1
Index: clang/test/CodeGen/ppc32-dwarf.c
===
--- /dev/null
+++ clang/test/CodeGen/ppc32-dwarf.c
@@ -0,0 +1,126 @@
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,PPC32
+static unsigned char dwarf_reg_size_table[1024];
+
+int test() {
+ __builtin_init_dwarf_reg_size_table(dwarf_reg_size_table);
+
+ return __builtin_dwarf_sp_column();
+}
+
+// CHECK-LABEL: define i32 @test()
+// CHECK: store i8 4, i8* getelementptr inbounds ([1024 x i8], [1024 x
[PATCH] D79033: [NFC][clang] Replace raw new/delete with unique_ptr to store ABIInfo in TargetCodeGenInfo
jasonliu updated this revision to Diff 260746.
jasonliu added a comment.
Provide default virtual destructor.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79033/new/
https://reviews.llvm.org/D79033
Files:
clang/lib/CodeGen/TargetInfo.cpp
clang/lib/CodeGen/TargetInfo.h
Index: clang/lib/CodeGen/TargetInfo.h
===
--- clang/lib/CodeGen/TargetInfo.h
+++ clang/lib/CodeGen/TargetInfo.h
@@ -43,12 +43,11 @@
/// codegeneration issues, like target-specific attributes, builtins and so
/// on.
class TargetCodeGenInfo {
- ABIInfo *Info;
+ std::unique_ptr Info = nullptr;
public:
- // WARNING: Acquires the ownership of ABIInfo.
- TargetCodeGenInfo(ABIInfo *info = nullptr) : Info(info) {}
- virtual ~TargetCodeGenInfo();
+ TargetCodeGenInfo(std::unique_ptr Info) : Info(std::move(Info)) {}
+ virtual ~TargetCodeGenInfo() = default;
/// getABIInfo() - Returns ABI info helper for the target.
const ABIInfo &getABIInfo() const { return *Info; }
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -385,8 +385,6 @@
return Address(PHI, Align);
}
-TargetCodeGenInfo::~TargetCodeGenInfo() { delete Info; }
-
// If someone can figure out a general rule for this, that would be great.
// It's probably just doomed to be platform-dependent, though.
unsigned TargetCodeGenInfo::getSizeOfUnwindException() const {
@@ -682,7 +680,7 @@
class DefaultTargetCodeGenInfo : public TargetCodeGenInfo {
public:
DefaultTargetCodeGenInfo(CodeGen::CodeGenTypes &CGT)
-: TargetCodeGenInfo(new DefaultABIInfo(CGT)) {}
+ : TargetCodeGenInfo(std::make_unique(CGT)) {}
};
ABIArgInfo DefaultABIInfo::classifyArgumentType(QualType Ty) const {
@@ -772,7 +770,7 @@
public:
explicit WebAssemblyTargetCodeGenInfo(CodeGen::CodeGenTypes &CGT,
WebAssemblyABIInfo::ABIKind K)
- : TargetCodeGenInfo(new WebAssemblyABIInfo(CGT, K)) {}
+ : TargetCodeGenInfo(std::make_unique(CGT, K)) {}
void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
CodeGen::CodeGenModule &CGM) const override {
@@ -898,8 +896,8 @@
class PNaClTargetCodeGenInfo : public TargetCodeGenInfo {
public:
- PNaClTargetCodeGenInfo(CodeGen::CodeGenTypes &CGT)
-: TargetCodeGenInfo(new PNaClABIInfo(CGT)) {}
+ PNaClTargetCodeGenInfo(CodeGen::CodeGenTypes &CGT)
+ : TargetCodeGenInfo(std::make_unique(CGT)) {}
};
void PNaClABIInfo::computeInfo(CGFunctionInfo &FI) const {
@@ -1140,7 +1138,7 @@
X86_32TargetCodeGenInfo(CodeGen::CodeGenTypes &CGT, bool DarwinVectorABI,
bool RetSmallStructInRegABI, bool Win32StructABI,
unsigned NumRegisterParameters, bool SoftFloatABI)
- : TargetCodeGenInfo(new X86_32ABIInfo(
+ : TargetCodeGenInfo(std::make_unique(
CGT, DarwinVectorABI, RetSmallStructInRegABI, Win32StructABI,
NumRegisterParameters, SoftFloatABI)) {}
@@ -2334,7 +2332,7 @@
class X86_64TargetCodeGenInfo : public TargetCodeGenInfo {
public:
X86_64TargetCodeGenInfo(CodeGen::CodeGenTypes &CGT, X86AVXABILevel AVXLevel)
- : TargetCodeGenInfo(new X86_64ABIInfo(CGT, AVXLevel)) {}
+ : TargetCodeGenInfo(std::make_unique(CGT, AVXLevel)) {}
const X86_64ABIInfo &getABIInfo() const {
return static_cast(TargetCodeGenInfo::getABIInfo());
@@ -2478,7 +2476,7 @@
public:
WinX86_64TargetCodeGenInfo(CodeGen::CodeGenTypes &CGT,
X86AVXABILevel AVXLevel)
- : TargetCodeGenInfo(new WinX86_64ABIInfo(CGT, AVXLevel)) {}
+ : TargetCodeGenInfo(std::make_unique(CGT, AVXLevel)) {}
void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
CodeGen::CodeGenModule &CGM) const override;
@@ -4204,8 +4202,8 @@
public:
PPC32TargetCodeGenInfo(CodeGenTypes &CGT, bool SoftFloatABI,
bool RetSmallStructInRegABI)
- : TargetCodeGenInfo(new PPC32_SVR4_ABIInfo(CGT, SoftFloatABI,
- RetSmallStructInRegABI)) {}
+ : TargetCodeGenInfo(std::make_unique(
+CGT, SoftFloatABI, RetSmallStructInRegABI)) {}
static bool isStructReturnInRegABI(const llvm::Triple &Triple,
const CodeGenOptions &Opts);
@@ -4595,8 +4593,8 @@
PPC64_SVR4_TargetCodeGenInfo(CodeGenTypes &CGT,
PPC64_SVR4_ABIInfo::ABIKind Kind, bool HasQPX,
bool SoftFloatABI)
- : TargetCodeGenInfo(new PPC64_SVR4_ABIInfo(CGT, Kind, HasQPX,
- SoftFloatABI)) {}
+ : TargetCodeGenInfo(std::make_unique(
+CGT, Kind, HasQPX, SoftFloatABI)) {}
int getDwarfEHStackPointer(CodeGen::CodeGenModule &M) const
[PATCH] D79033: [NFC][clang] Replace raw new/delete with unique_ptr to store ABIInfo in TargetCodeGenInfo
jasonliu updated this revision to Diff 260766.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79033/new/
https://reviews.llvm.org/D79033
Files:
clang/lib/CodeGen/TargetInfo.cpp
clang/lib/CodeGen/TargetInfo.h
Index: clang/lib/CodeGen/TargetInfo.h
===
--- clang/lib/CodeGen/TargetInfo.h
+++ clang/lib/CodeGen/TargetInfo.h
@@ -43,11 +43,10 @@
/// codegeneration issues, like target-specific attributes, builtins and so
/// on.
class TargetCodeGenInfo {
- ABIInfo *Info;
+ std::unique_ptr Info = nullptr;
public:
- // WARNING: Acquires the ownership of ABIInfo.
- TargetCodeGenInfo(ABIInfo *info = nullptr) : Info(info) {}
+ TargetCodeGenInfo(std::unique_ptr Info) : Info(std::move(Info)) {}
virtual ~TargetCodeGenInfo();
/// getABIInfo() - Returns ABI info helper for the target.
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -385,7 +385,7 @@
return Address(PHI, Align);
}
-TargetCodeGenInfo::~TargetCodeGenInfo() { delete Info; }
+TargetCodeGenInfo::~TargetCodeGenInfo() = default;
// If someone can figure out a general rule for this, that would be great.
// It's probably just doomed to be platform-dependent, though.
@@ -682,7 +682,7 @@
class DefaultTargetCodeGenInfo : public TargetCodeGenInfo {
public:
DefaultTargetCodeGenInfo(CodeGen::CodeGenTypes &CGT)
-: TargetCodeGenInfo(new DefaultABIInfo(CGT)) {}
+ : TargetCodeGenInfo(std::make_unique(CGT)) {}
};
ABIArgInfo DefaultABIInfo::classifyArgumentType(QualType Ty) const {
@@ -772,7 +772,7 @@
public:
explicit WebAssemblyTargetCodeGenInfo(CodeGen::CodeGenTypes &CGT,
WebAssemblyABIInfo::ABIKind K)
- : TargetCodeGenInfo(new WebAssemblyABIInfo(CGT, K)) {}
+ : TargetCodeGenInfo(std::make_unique(CGT, K)) {}
void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
CodeGen::CodeGenModule &CGM) const override {
@@ -898,8 +898,8 @@
class PNaClTargetCodeGenInfo : public TargetCodeGenInfo {
public:
- PNaClTargetCodeGenInfo(CodeGen::CodeGenTypes &CGT)
-: TargetCodeGenInfo(new PNaClABIInfo(CGT)) {}
+ PNaClTargetCodeGenInfo(CodeGen::CodeGenTypes &CGT)
+ : TargetCodeGenInfo(std::make_unique(CGT)) {}
};
void PNaClABIInfo::computeInfo(CGFunctionInfo &FI) const {
@@ -1140,7 +1140,7 @@
X86_32TargetCodeGenInfo(CodeGen::CodeGenTypes &CGT, bool DarwinVectorABI,
bool RetSmallStructInRegABI, bool Win32StructABI,
unsigned NumRegisterParameters, bool SoftFloatABI)
- : TargetCodeGenInfo(new X86_32ABIInfo(
+ : TargetCodeGenInfo(std::make_unique(
CGT, DarwinVectorABI, RetSmallStructInRegABI, Win32StructABI,
NumRegisterParameters, SoftFloatABI)) {}
@@ -2334,7 +2334,7 @@
class X86_64TargetCodeGenInfo : public TargetCodeGenInfo {
public:
X86_64TargetCodeGenInfo(CodeGen::CodeGenTypes &CGT, X86AVXABILevel AVXLevel)
- : TargetCodeGenInfo(new X86_64ABIInfo(CGT, AVXLevel)) {}
+ : TargetCodeGenInfo(std::make_unique(CGT, AVXLevel)) {}
const X86_64ABIInfo &getABIInfo() const {
return static_cast(TargetCodeGenInfo::getABIInfo());
@@ -2478,7 +2478,7 @@
public:
WinX86_64TargetCodeGenInfo(CodeGen::CodeGenTypes &CGT,
X86AVXABILevel AVXLevel)
- : TargetCodeGenInfo(new WinX86_64ABIInfo(CGT, AVXLevel)) {}
+ : TargetCodeGenInfo(std::make_unique(CGT, AVXLevel)) {}
void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
CodeGen::CodeGenModule &CGM) const override;
@@ -4204,8 +4204,8 @@
public:
PPC32TargetCodeGenInfo(CodeGenTypes &CGT, bool SoftFloatABI,
bool RetSmallStructInRegABI)
- : TargetCodeGenInfo(new PPC32_SVR4_ABIInfo(CGT, SoftFloatABI,
- RetSmallStructInRegABI)) {}
+ : TargetCodeGenInfo(std::make_unique(
+CGT, SoftFloatABI, RetSmallStructInRegABI)) {}
static bool isStructReturnInRegABI(const llvm::Triple &Triple,
const CodeGenOptions &Opts);
@@ -4595,8 +4595,8 @@
PPC64_SVR4_TargetCodeGenInfo(CodeGenTypes &CGT,
PPC64_SVR4_ABIInfo::ABIKind Kind, bool HasQPX,
bool SoftFloatABI)
- : TargetCodeGenInfo(new PPC64_SVR4_ABIInfo(CGT, Kind, HasQPX,
- SoftFloatABI)) {}
+ : TargetCodeGenInfo(std::make_unique(
+CGT, Kind, HasQPX, SoftFloatABI)) {}
int getDwarfEHStackPointer(CodeGen::CodeGenModule &M) const override {
// This is recovered from gcc output.
@@ -5154,7 +5154,7 @@
class AArch64TargetCodeGenInfo : public TargetCodeGenInfo {
public:
AArch64TargetCodeGenInfo(
[PATCH] D79033: [NFC][clang] Replace raw new/delete with unique_ptr to store ABIInfo in TargetCodeGenInfo
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe0c356582d2f: [NFC][clang] Replace raw new/delete with
unique_ptr to store ABIInfo in… (authored by jasonliu).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79033/new/
https://reviews.llvm.org/D79033
Files:
clang/lib/CodeGen/TargetInfo.cpp
clang/lib/CodeGen/TargetInfo.h
Index: clang/lib/CodeGen/TargetInfo.h
===
--- clang/lib/CodeGen/TargetInfo.h
+++ clang/lib/CodeGen/TargetInfo.h
@@ -43,11 +43,10 @@
/// codegeneration issues, like target-specific attributes, builtins and so
/// on.
class TargetCodeGenInfo {
- ABIInfo *Info;
+ std::unique_ptr Info = nullptr;
public:
- // WARNING: Acquires the ownership of ABIInfo.
- TargetCodeGenInfo(ABIInfo *info = nullptr) : Info(info) {}
+ TargetCodeGenInfo(std::unique_ptr Info) : Info(std::move(Info)) {}
virtual ~TargetCodeGenInfo();
/// getABIInfo() - Returns ABI info helper for the target.
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -385,7 +385,7 @@
return Address(PHI, Align);
}
-TargetCodeGenInfo::~TargetCodeGenInfo() { delete Info; }
+TargetCodeGenInfo::~TargetCodeGenInfo() = default;
// If someone can figure out a general rule for this, that would be great.
// It's probably just doomed to be platform-dependent, though.
@@ -682,7 +682,7 @@
class DefaultTargetCodeGenInfo : public TargetCodeGenInfo {
public:
DefaultTargetCodeGenInfo(CodeGen::CodeGenTypes &CGT)
-: TargetCodeGenInfo(new DefaultABIInfo(CGT)) {}
+ : TargetCodeGenInfo(std::make_unique(CGT)) {}
};
ABIArgInfo DefaultABIInfo::classifyArgumentType(QualType Ty) const {
@@ -772,7 +772,7 @@
public:
explicit WebAssemblyTargetCodeGenInfo(CodeGen::CodeGenTypes &CGT,
WebAssemblyABIInfo::ABIKind K)
- : TargetCodeGenInfo(new WebAssemblyABIInfo(CGT, K)) {}
+ : TargetCodeGenInfo(std::make_unique(CGT, K)) {}
void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
CodeGen::CodeGenModule &CGM) const override {
@@ -898,8 +898,8 @@
class PNaClTargetCodeGenInfo : public TargetCodeGenInfo {
public:
- PNaClTargetCodeGenInfo(CodeGen::CodeGenTypes &CGT)
-: TargetCodeGenInfo(new PNaClABIInfo(CGT)) {}
+ PNaClTargetCodeGenInfo(CodeGen::CodeGenTypes &CGT)
+ : TargetCodeGenInfo(std::make_unique(CGT)) {}
};
void PNaClABIInfo::computeInfo(CGFunctionInfo &FI) const {
@@ -1140,7 +1140,7 @@
X86_32TargetCodeGenInfo(CodeGen::CodeGenTypes &CGT, bool DarwinVectorABI,
bool RetSmallStructInRegABI, bool Win32StructABI,
unsigned NumRegisterParameters, bool SoftFloatABI)
- : TargetCodeGenInfo(new X86_32ABIInfo(
+ : TargetCodeGenInfo(std::make_unique(
CGT, DarwinVectorABI, RetSmallStructInRegABI, Win32StructABI,
NumRegisterParameters, SoftFloatABI)) {}
@@ -2343,7 +2343,7 @@
class X86_64TargetCodeGenInfo : public TargetCodeGenInfo {
public:
X86_64TargetCodeGenInfo(CodeGen::CodeGenTypes &CGT, X86AVXABILevel AVXLevel)
- : TargetCodeGenInfo(new X86_64ABIInfo(CGT, AVXLevel)) {}
+ : TargetCodeGenInfo(std::make_unique(CGT, AVXLevel)) {}
const X86_64ABIInfo &getABIInfo() const {
return static_cast(TargetCodeGenInfo::getABIInfo());
@@ -2487,7 +2487,7 @@
public:
WinX86_64TargetCodeGenInfo(CodeGen::CodeGenTypes &CGT,
X86AVXABILevel AVXLevel)
- : TargetCodeGenInfo(new WinX86_64ABIInfo(CGT, AVXLevel)) {}
+ : TargetCodeGenInfo(std::make_unique(CGT, AVXLevel)) {}
void setTargetAttributes(const Decl *D, llvm::GlobalValue *GV,
CodeGen::CodeGenModule &CGM) const override;
@@ -4235,8 +4235,8 @@
public:
PPC32TargetCodeGenInfo(CodeGenTypes &CGT, bool SoftFloatABI,
bool RetSmallStructInRegABI)
- : TargetCodeGenInfo(new PPC32_SVR4_ABIInfo(CGT, SoftFloatABI,
- RetSmallStructInRegABI)) {}
+ : TargetCodeGenInfo(std::make_unique(
+CGT, SoftFloatABI, RetSmallStructInRegABI)) {}
static bool isStructReturnInRegABI(const llvm::Triple &Triple,
const CodeGenOptions &Opts);
@@ -4626,8 +4626,8 @@
PPC64_SVR4_TargetCodeGenInfo(CodeGenTypes &CGT,
PPC64_SVR4_ABIInfo::ABIKind Kind, bool HasQPX,
bool SoftFloatABI)
- : TargetCodeGenInfo(new PPC64_SVR4_ABIInfo(CGT, Kind, HasQPX,
- SoftFloatABI)) {}
+ : TargetCodeGenInfo(std::make_unique(
+CGT, Kind, HasQPX, SoftFloatABI)) {}
int getDwarfEHStackPointer(CodeGen::C
[PATCH] D79044: [AIX] Avoid structor alias; die before bad alias codegen
jasonliu accepted this revision. jasonliu 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/D79044/new/ https://reviews.llvm.org/D79044 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D84534: [AIX] Static init frontend recovery and backend support
jasonliu added inline comments.
Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:24
#include "llvm/Support/Path.h"
#include "llvm/Transforms/Utils/ModuleUtils.h"
We are removing the usage of "getUniqueModuleId" in this file, So I assume this
include could get removed as well.
Comment at: clang/lib/CodeGen/CodeGenModule.h:1058
+ /// Add an sterm finalizer to its own llvm.global_dtors entry.
+ void AddCXXStermFinalizerToGlobalDtor(llvm::Function *StermFinalizer,
+int Priority) {
This wrapper seems redundant. Calling ` AddGlobalDtor(StermFinalizer,
Priority);` directly from the callee side already convey what we are trying to
achieve here.
Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4602
+llvm::report_fatal_error(
+"prioritized __sinit and __sterm functions are not yet supported");
+ else if (isTemplateInstantiation(D.getTemplateSpecializationKind()) ||
Could we trigger this error? If so, could we have a test for it?
Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:2119
-unsigned Priority, const MCSymbol *KeySym) const {
- report_fatal_error("XCOFF dtor section not yet implemented.");
-}
I think it's still useful to keep these functions around to prevent
accidentally calling to these functions on AIX. We could rephrase error message
to "static constructor/destructor section is not needed on AIX."
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1874
+ }
+ emitSpecialLLVMGlobal(&G);
+ continue;
Have some suggestion on structure backend code change:
1. Remove `isSpecialLLVMGlobalArrayForStaticInit` and
`isSpecialLLVMGlobalArrayToSkip`, and call `emitSpecialLLVMGlobal` directly
instead. This would handle all the `llvm.` variable for us. We would need do
early return for names start with `llvm.` in emitGlobalVariable as well, since
they are all handled here.
2. We could override emitXXStructorList because how XCOFF handle the list is
vastly different than the other target. We could make the common part(i.e.
processing and sorting) a utility function, say "preprocessStructorList".
3. It's not necessary to use the same name `emitXXStructor` if the interface is
different. It might not provide much help when we kinda know no other target is
going to use this interface. So we could turn it into a lambda inside of the
overrided emitXXStructorList, or simply no need for a new function because the
logic is fairly straightforward.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84534/new/
https://reviews.llvm.org/D84534
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D84534: [AIX] Static init frontend recovery and backend support
jasonliu added inline comments.
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1865
+if (isSpecialLLVMGlobalArrayForStaticInit(&G)) {
+ if (GlobalUniqueModuleId.empty()) {
+GlobalUniqueModuleId = getUniqueModuleId(&M);
We will need to move this part of the if statement to the overrided
`emitXXStructorList` as well. (If we think overriding `emitXXStructorList` and
calling `emitSpecialLLVMGlobal ` directly is a good idea.)
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84534/new/
https://reviews.llvm.org/D84534
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D84534: [AIX] Static init frontend recovery and backend support
jasonliu added inline comments.
Comment at: format:1
+//===-- PPCAsmPrinter.cpp - Print machine instrs to PowerPC assembly
--===//
+//
Redundant file?
Comment at: llvm/include/llvm/CodeGen/AsmPrinter.h:455
}
+ struct Structor {
The new block is not all `Overridable Hooks`, seems better belong in `Coarse
grained IR lowering routines`.
Comment at: llvm/include/llvm/CodeGen/AsmPrinter.h:456
+ struct Structor {
+int Priority = 0;
This struct got moved to header, means it's not an implementation detail any
more.
We would need doxygen comment on it.
Comment at: llvm/include/llvm/CodeGen/AsmPrinter.h:460
+// In most cases, `Key` represents a `ComdatKey`. Except on AIX, a `Key` is
+// used to identify a csect where we should emit `.ref` when needed.
+GlobalValue *Key = nullptr;
I have a slight preference to leave it as ComdatKey, and change the name when
we actually need to handle `.ref` because without seeing the actual
implementation for `.ref` we don't know if this is the way we want to pursue.
Comment at: llvm/include/llvm/CodeGen/AsmPrinter.h:466
+
+ bool preprocessStructorList(const DataLayout &DL, const Constant *List,
+ SmallVector &Structors);
A doxygen comment describe what this function does, and what its return value
means, and mention `Structors` is an output argument.
By looking at what this function does, it seems `buildStructorList` is a better
name.
Comment at: llvm/include/llvm/CodeGen/AsmPrinter.h:468
+ SmallVector &Structors);
+ /// Targets can override this to change how `llvm.global_ctors` and
+ /// `llvm.global_dtors` lists get handled.
Add a blank line above this.
Comment at: llvm/test/CodeGen/PowerPC/aix-static-init-key-object.ll:6
+
+@llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32,
void ()*, i8* } { i32 65535, void ()* @foo, i8* @v}]
+
Adding this test case would looks like we already decided how to handle .ref in
clang and llvm. But in fact, we haven't. I would prefer not having this test.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84534/new/
https://reviews.llvm.org/D84534
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D84534: [AIX] Static init frontend recovery and backend support
jasonliu added inline comments.
Comment at: llvm/include/llvm/CodeGen/AsmPrinter.h:391
+ /// @param[out] Structors Sorted Structor structs by Priority.
+ /// @return false if List is not an array of '{ i32, void ()*, i8* }'
structs.
+ bool preprocessXXStructorList(const DataLayout &DL, const Constant *List,
This description is not entirely true. We only see if the array is a
ConstantArray and returning false. We are not returning false if the array's
element is not `{ i32, void ()*, i8* }`.
Comment at: llvm/include/llvm/CodeGen/AsmPrinter.h:466
+
+ bool preprocessStructorList(const DataLayout &DL, const Constant *List,
+ SmallVector &Structors);
Xiangling_L wrote:
> jasonliu wrote:
> > A doxygen comment describe what this function does, and what its return
> > value means, and mention `Structors` is an output argument.
> > By looking at what this function does, it seems `buildStructorList` is a
> > better name.
> I meant to and should've named this function to `preprocessXXStructorList`,
> let me know if you still prefer `buildStructorList`. And if you do, since the
> underneath of `SmallVector` is a variable-sized array, maybe we should try
> `buildSortedStructorArray`?
`preprocess` sounds like we are already having a XXStructorList and now we try
to do something on it.
But right now, we are actually passing in an empty StructorList/Array and build
it from scratch. So I would still prefer the name of `build` in it.
I don't mind changing to a more accurate name as you suggested.
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:2107
+ if (!isa(List))
+return false;
Return of boolean seems unnecessary.
Callee could check the size of the Structors to decide if they want an early
return or not (or in this particular case, the for loop would just do nothing
and no need for extra condition if you don't mind the call to
getPointerPrefAlignment or assign to 0 to Index)?
So we could just return void for this function?
Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:2129
}
-
// Emit the function pointers in the target-specific order
llvm::stable_sort(Structors, [](const Structor &L, const Structor &R) {
nit: Missing blank line.
Comment at: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp:1966
+
+Index++;
+ }
nit: Use `++Index` instead.
https://llvm.org/docs/CodingStandards.html#prefer-preincrement
Or use `Index++` at line 1963 so that we don't need this line.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84534/new/
https://reviews.llvm.org/D84534
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D84534: [AIX] Static init frontend recovery and backend support
jasonliu added inline comments. Comment at: llvm/include/llvm/CodeGen/AsmPrinter.h:466 + + bool preprocessStructorList(const DataLayout &DL, const Constant *List, + SmallVector &Structors); Xiangling_L wrote: > jasonliu wrote: > > Xiangling_L wrote: > > > jasonliu wrote: > > > > A doxygen comment describe what this function does, and what its return > > > > value means, and mention `Structors` is an output argument. > > > > By looking at what this function does, it seems `buildStructorList` is > > > > a better name. > > > I meant to and should've named this function to > > > `preprocessXXStructorList`, let me know if you still prefer > > > `buildStructorList`. And if you do, since the underneath of `SmallVector` > > > is a variable-sized array, maybe we should try `buildSortedStructorArray`? > > `preprocess` sounds like we are already having a XXStructorList and now we > > try to do something on it. > > But right now, we are actually passing in an empty StructorList/Array and > > build it from scratch. So I would still prefer the name of `build` in it. > > I don't mind changing to a more accurate name as you suggested. > I think we do have a `XXStructorList` here which is the initializer of > `llvm.gloal_ctors`or `llvm.gloal_dtors` array? The usage of this term is > consistent with other spots. My understanding is that before we enter this `preprocessXXStructorList`, we do not have a list of XXStructor. We only have a list of `Constant`. This functions turns a list of `Constant` to a list of `XXStructor`. Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:2107 + if (!isa(List)) +return false; Xiangling_L wrote: > jasonliu wrote: > > Return of boolean seems unnecessary. > > Callee could check the size of the Structors to decide if they want an > > early return or not (or in this particular case, the for loop would just do > > nothing and no need for extra condition if you don't mind the call to > > getPointerPrefAlignment or assign to 0 to Index)? > > So we could just return void for this function? > Yeah, we could do that. But it looks a boolean return value makes the code > flow natural. And if any target does want to control the early return in the > future, it's flexbile for them to do that. I am wondering is there any big > difference between bool and void return value for this function? If target need to control early return in the future, they could still do so by querying if the output `Structors` is empty or not. I just don't want unnecessary returns, as it's one more thing that user of the function need to care about when they examine this function. And the user of this function would have the same question of why we need to return a boolean here. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84534/new/ https://reviews.llvm.org/D84534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D84534: [AIX] Static init frontend recovery and backend support
jasonliu accepted this revision. jasonliu added a comment. This revision is now accepted and ready to land. Thanks. LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84534/new/ https://reviews.llvm.org/D84534 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D79035: [clang][AIX] Implement ABIInfo and TargetCodeGenInfo for AIX
jasonliu marked 2 inline comments as done. jasonliu added inline comments. Comment at: clang/test/CodeGen/ppc32-dwarf.c:2 +// RUN: %clang_cc1 -triple powerpc-unknown-aix -emit-llvm %s -o - | FileCheck %s +// RUN: %clang_cc1 -triple powerpc-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,PPC32 +static unsigned char dwarf_reg_size_table[1024]; Xiangling_L wrote: > Minor comment: > Would `PPC32SVR4` compared to `PPC32` make the checking content clearer since > PPC32 actually includes AIX target? Technically, it's PPC32 target except AIX (not restrict to SVR4). So PPC32SVR4 is not that accurate either. Comment at: clang/test/CodeGen/ppc64-dwarf.c:2 +// RUN: %clang_cc1 -triple powerpc64-unknown-aix -emit-llvm %s -o - | FileCheck %s +// RUN: %clang_cc1 -triple powerpc64-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,PPC64 static unsigned char dwarf_reg_size_table[1024]; Xiangling_L wrote: > Same comment as above. > s/PPC64/PPC64SVR4? Same above, and for PPC64 we have Darwin that's actually not SVR4. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79035/new/ https://reviews.llvm.org/D79035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D79035: [clang][AIX] Implement ABIInfo and TargetCodeGenInfo for AIX
jasonliu marked 2 inline comments as done. jasonliu added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:4317 + if (isAggregateTypeForABI(RetTy)) +return getNaturalAlignIndirect(RetTy); + Xiangling_L wrote: > This method uses the ABI alignment of the given aggregate type which I think > is not ideal due to our AIX special alignment rule. We need to use preferred > alignment in this case. > Btw also I think it's not necessary for you to rebase your patch on the power > alignment patch, I can refresh the testcase when I am dealing with that one. As it is right now in master, there is no difference between natural alignment and preferred alignment for AIX. The tentative direction is to use preferred alignment to record the actual alignment on AIX, but it is not finalized yet. I would rather leave this part of the work for the patch that's going to implement the power alignment rule for AIX. Comment at: clang/test/CodeGen/aix-vaargs.c:3 +// REQUIRES: asserts +// RUN: %clang_cc1 -triple powerpc-unknown-aix -emit-llvm -o - %s | FileCheck %s --check-prefixes=AIX-COM,AIX-M32 +// RUN: %clang_cc1 -triple powerpc64-unknown-aix -emit-llvm -o - %s | FileCheck %s --check-prefixes=AIX-COM,AIX-M64 Xiangling_L wrote: > Consistent with other testcases to use `AIX32/AIX64`? I chose AIX-M32/AIX-M64 mainly because the length is the same as AIX-COM so we don't need to worry about aligning the space. I would prefer to keep it that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79035/new/ https://reviews.llvm.org/D79035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D79035: [clang][AIX] Implement ABIInfo and TargetCodeGenInfo for AIX
jasonliu updated this revision to Diff 262250.
jasonliu marked 12 inline comments as done.
jasonliu added a comment.
Rebase and address comments.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79035/new/
https://reviews.llvm.org/D79035
Files:
clang/lib/CodeGen/TargetInfo.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/CodeGen/aix-complex.c
clang/test/CodeGen/aix-return.c
clang/test/CodeGen/aix-struct-arg.c
clang/test/CodeGen/aix-vaargs.c
clang/test/CodeGen/aix-vector.c
clang/test/CodeGen/ppc32-and-aix-struct-return.c
clang/test/CodeGen/ppc32-dwarf.c
clang/test/CodeGen/ppc32-struct-return.c
clang/test/CodeGen/ppc64-dwarf.c
clang/test/Driver/ppc-unsupported.c
Index: clang/test/Driver/ppc-unsupported.c
===
--- clang/test/Driver/ppc-unsupported.c
+++ clang/test/Driver/ppc-unsupported.c
@@ -7,4 +7,12 @@
// RUN: -c %s 2>&1 | FileCheck %s
// RUN: not %clang -target powerpc64le-unknown-linux -msvr4-struct-return \
// RUN: -c %s 2>&1 | FileCheck %s
+// RUN: not %clang -target powerpc-unknown-aix -maix-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
+// RUN: not %clang -target powerpc-unknown-aix -msvr4-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
+// RUN: not %clang -target powerpc64-unknown-aix -maix-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
+// RUN: not %clang -target powerpc64-unknown-aix -msvr4-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
// CHECK: unsupported option
Index: clang/test/CodeGen/ppc64-dwarf.c
===
--- clang/test/CodeGen/ppc64-dwarf.c
+++ clang/test/CodeGen/ppc64-dwarf.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple powerpc64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,PPC64
static unsigned char dwarf_reg_size_table[1024];
int test() {
@@ -119,10 +120,10 @@
// CHECK-NEXT: store i8 16, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 108), align 1
// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 109), align 1
// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 110), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 111), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 112), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 113), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 114), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 115), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 116), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 111), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 112), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 113), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 114), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 115), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 116), align 1
// CHECK-NEXT: ret i32 1
Index: clang/test/CodeGen/ppc32-struct-return.c
===
--- clang/test/CodeGen/ppc32-struct-return.c
+++ /dev/null
@@ -1,88 +0,0 @@
-// REQUIRES: powerpc-registered-target
-// RUN: %clang_cc1 -triple powerpc-unknown-freebsd \
-// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-SVR4
-// RUN: %clang_cc1 -triple powerpc-unknown-linux \
-// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-AIX
-// RUN: %clang_cc1 -triple powerpc-unknown-linux -maix-struct-return \
-// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-AIX
-// RUN: %clang_cc1 -triple powerpc-unknown-linux -msvr4-struct-return \
-// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-SVR4
-// RUN: %clang_cc1 -triple powerpc-unknown-netbsd \
-// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-SVR4
-// RUN: %clang_cc1 -triple
[PATCH] D79035: [clang][AIX] Implement ABIInfo and TargetCodeGenInfo for AIX
jasonliu updated this revision to Diff 262252.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79035/new/
https://reviews.llvm.org/D79035
Files:
clang/lib/CodeGen/TargetInfo.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/CodeGen/aix-complex.c
clang/test/CodeGen/aix-return.c
clang/test/CodeGen/aix-struct-arg.c
clang/test/CodeGen/aix-vaargs.c
clang/test/CodeGen/aix-vector.c
clang/test/CodeGen/ppc32-and-aix-struct-return.c
clang/test/CodeGen/ppc32-dwarf.c
clang/test/CodeGen/ppc32-struct-return.c
clang/test/CodeGen/ppc64-dwarf.c
clang/test/Driver/ppc-unsupported.c
Index: clang/test/Driver/ppc-unsupported.c
===
--- clang/test/Driver/ppc-unsupported.c
+++ clang/test/Driver/ppc-unsupported.c
@@ -7,4 +7,12 @@
// RUN: -c %s 2>&1 | FileCheck %s
// RUN: not %clang -target powerpc64le-unknown-linux -msvr4-struct-return \
// RUN: -c %s 2>&1 | FileCheck %s
+// RUN: not %clang -target powerpc-unknown-aix -maix-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
+// RUN: not %clang -target powerpc-unknown-aix -msvr4-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
+// RUN: not %clang -target powerpc64-unknown-aix -maix-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
+// RUN: not %clang -target powerpc64-unknown-aix -msvr4-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
// CHECK: unsupported option
Index: clang/test/CodeGen/ppc64-dwarf.c
===
--- clang/test/CodeGen/ppc64-dwarf.c
+++ clang/test/CodeGen/ppc64-dwarf.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple powerpc64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,PPC64
static unsigned char dwarf_reg_size_table[1024];
int test() {
@@ -119,10 +120,10 @@
// CHECK-NEXT: store i8 16, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 108), align 1
// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 109), align 1
// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 110), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 111), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 112), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 113), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 114), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 115), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 116), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 111), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 112), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 113), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 114), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 115), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 116), align 1
// CHECK-NEXT: ret i32 1
Index: clang/test/CodeGen/ppc32-struct-return.c
===
--- clang/test/CodeGen/ppc32-struct-return.c
+++ /dev/null
@@ -1,88 +0,0 @@
-// REQUIRES: powerpc-registered-target
-// RUN: %clang_cc1 -triple powerpc-unknown-freebsd \
-// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-SVR4
-// RUN: %clang_cc1 -triple powerpc-unknown-linux \
-// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-AIX
-// RUN: %clang_cc1 -triple powerpc-unknown-linux -maix-struct-return \
-// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-AIX
-// RUN: %clang_cc1 -triple powerpc-unknown-linux -msvr4-struct-return \
-// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-SVR4
-// RUN: %clang_cc1 -triple powerpc-unknown-netbsd \
-// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-SVR4
-// RUN: %clang_cc1 -triple powerpc-unknown-openbsd \
-// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-SVR4
-//
[PATCH] D79035: [clang][AIX] Implement ABIInfo and TargetCodeGenInfo for AIX
jasonliu added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:4368 + + return emitVoidPtrVAArg(CGF, VAListAddr, Ty, /*Indirect*/ false, TypeInfo, + SlotSize, /*AllowHigher*/ true); ZarkoCA wrote: > Is there a reason why Indirect is set to `false` instead of querying for it > using `classifyArgumentType(Ty).isIndirect()`? For how we handle Vaarg in backend, we just expect everything on register(no ByVal) even when it's a structure. So the indirect should always be false unless there is change in backend implementation. Comment at: clang/lib/CodeGen/TargetInfo.cpp:4641 return false; case CodeGenOptions::SRCK_InRegs: // -msvr4-struct-return return true; ZarkoCA wrote: > Has this option been verified to work correctly on AIX? In > https://reviews.llvm.org/D76360 we added a defensive error because we weren't > sure whether padding was handled correctly as described in the code. Thanks. I think we should disable this option in this patch. Comment at: clang/test/CodeGen/ppc32-and-aix-struct-return.c:8 +// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-AIX +// RUN: %clang_cc1 -triple powerpc-unknown-linux \ +// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-AIX Xiangling_L wrote: > Do you mean to check AIX or SVR4? This file is actually from llvm-project/clang/test/CodeGen/ppc32-struct-return.c What I did is only adding the check for AIX triple. And apparently, the default behavior of powerpc-unknown-linux triple is CHECK-AIX. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79035/new/ https://reviews.llvm.org/D79035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D79035: [clang][AIX] Implement ABIInfo and TargetCodeGenInfo for AIX
jasonliu updated this revision to Diff 262869.
jasonliu added a comment.
Address comments.
Add in over aligned structure and structure containing vector type handling for
argument on AIX.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79035/new/
https://reviews.llvm.org/D79035
Files:
clang/lib/CodeGen/TargetInfo.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/CodeGen/aix-complex.c
clang/test/CodeGen/aix-return.c
clang/test/CodeGen/aix-struct-arg.c
clang/test/CodeGen/aix-vaargs.c
clang/test/CodeGen/aix-vector.c
clang/test/CodeGen/ppc32-and-aix-struct-return.c
clang/test/CodeGen/ppc32-dwarf.c
clang/test/CodeGen/ppc32-struct-return.c
clang/test/CodeGen/ppc64-dwarf.c
clang/test/Driver/ppc-unsupported.c
Index: clang/test/Driver/ppc-unsupported.c
===
--- clang/test/Driver/ppc-unsupported.c
+++ clang/test/Driver/ppc-unsupported.c
@@ -7,4 +7,12 @@
// RUN: -c %s 2>&1 | FileCheck %s
// RUN: not %clang -target powerpc64le-unknown-linux -msvr4-struct-return \
// RUN: -c %s 2>&1 | FileCheck %s
+// RUN: not %clang -target powerpc-unknown-aix -maix-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
+// RUN: not %clang -target powerpc-unknown-aix -msvr4-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
+// RUN: not %clang -target powerpc64-unknown-aix -maix-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
+// RUN: not %clang -target powerpc64-unknown-aix -msvr4-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
// CHECK: unsupported option
Index: clang/test/CodeGen/ppc64-dwarf.c
===
--- clang/test/CodeGen/ppc64-dwarf.c
+++ clang/test/CodeGen/ppc64-dwarf.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple powerpc64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,PPC64
static unsigned char dwarf_reg_size_table[1024];
int test() {
@@ -119,10 +120,10 @@
// CHECK-NEXT: store i8 16, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 108), align 1
// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 109), align 1
// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 110), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 111), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 112), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 113), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 114), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 115), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 116), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 111), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 112), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 113), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 114), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 115), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 116), align 1
// CHECK-NEXT: ret i32 1
Index: clang/test/CodeGen/ppc32-struct-return.c
===
--- clang/test/CodeGen/ppc32-struct-return.c
+++ /dev/null
@@ -1,88 +0,0 @@
-// REQUIRES: powerpc-registered-target
-// RUN: %clang_cc1 -triple powerpc-unknown-freebsd \
-// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-SVR4
-// RUN: %clang_cc1 -triple powerpc-unknown-linux \
-// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-AIX
-// RUN: %clang_cc1 -triple powerpc-unknown-linux -maix-struct-return \
-// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-AIX
-// RUN: %clang_cc1 -triple powerpc-unknown-linux -msvr4-struct-return \
-// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-SVR4
-// RUN: %clang_cc1 -triple powerpc-unknown-netbsd \
-// RUN: -emit-llvm -o - %s | FileCheck %s --check-pr
[PATCH] D79035: [clang][AIX] Implement ABIInfo and TargetCodeGenInfo for AIX
jasonliu updated this revision to Diff 262878.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79035/new/
https://reviews.llvm.org/D79035
Files:
clang/lib/CodeGen/TargetInfo.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/CodeGen/aix-complex.c
clang/test/CodeGen/aix-return.c
clang/test/CodeGen/aix-struct-arg.c
clang/test/CodeGen/aix-vaargs.c
clang/test/CodeGen/aix-vector.c
clang/test/CodeGen/ppc32-and-aix-struct-return.c
clang/test/CodeGen/ppc32-dwarf.c
clang/test/CodeGen/ppc32-struct-return.c
clang/test/CodeGen/ppc64-dwarf.c
clang/test/Driver/ppc-unsupported.c
Index: clang/test/Driver/ppc-unsupported.c
===
--- clang/test/Driver/ppc-unsupported.c
+++ clang/test/Driver/ppc-unsupported.c
@@ -7,4 +7,12 @@
// RUN: -c %s 2>&1 | FileCheck %s
// RUN: not %clang -target powerpc64le-unknown-linux -msvr4-struct-return \
// RUN: -c %s 2>&1 | FileCheck %s
+// RUN: not %clang -target powerpc-unknown-aix -maix-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
+// RUN: not %clang -target powerpc-unknown-aix -msvr4-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
+// RUN: not %clang -target powerpc64-unknown-aix -maix-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
+// RUN: not %clang -target powerpc64-unknown-aix -msvr4-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
// CHECK: unsupported option
Index: clang/test/CodeGen/ppc64-dwarf.c
===
--- clang/test/CodeGen/ppc64-dwarf.c
+++ clang/test/CodeGen/ppc64-dwarf.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple powerpc64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,PPC64
static unsigned char dwarf_reg_size_table[1024];
int test() {
@@ -119,10 +120,10 @@
// CHECK-NEXT: store i8 16, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 108), align 1
// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 109), align 1
// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 110), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 111), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 112), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 113), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 114), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 115), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 116), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 111), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 112), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 113), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 114), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 115), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 116), align 1
// CHECK-NEXT: ret i32 1
Index: clang/test/CodeGen/ppc32-struct-return.c
===
--- clang/test/CodeGen/ppc32-struct-return.c
+++ /dev/null
@@ -1,88 +0,0 @@
-// REQUIRES: powerpc-registered-target
-// RUN: %clang_cc1 -triple powerpc-unknown-freebsd \
-// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-SVR4
-// RUN: %clang_cc1 -triple powerpc-unknown-linux \
-// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-AIX
-// RUN: %clang_cc1 -triple powerpc-unknown-linux -maix-struct-return \
-// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-AIX
-// RUN: %clang_cc1 -triple powerpc-unknown-linux -msvr4-struct-return \
-// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-SVR4
-// RUN: %clang_cc1 -triple powerpc-unknown-netbsd \
-// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-SVR4
-// RUN: %clang_cc1 -triple powerpc-unknown-openbsd \
-// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-SVR4
-//
[PATCH] D79035: [clang][AIX] Implement ABIInfo and TargetCodeGenInfo for AIX
jasonliu updated this revision to Diff 262958.
jasonliu marked 2 inline comments as done.
jasonliu added a comment.
Address Zarko's comment.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79035/new/
https://reviews.llvm.org/D79035
Files:
clang/lib/CodeGen/TargetInfo.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/CodeGen/aix-complex.c
clang/test/CodeGen/aix-return.c
clang/test/CodeGen/aix-struct-arg.c
clang/test/CodeGen/aix-vaargs.c
clang/test/CodeGen/aix-vector.c
clang/test/CodeGen/ppc32-and-aix-struct-return.c
clang/test/CodeGen/ppc32-dwarf.c
clang/test/CodeGen/ppc32-struct-return.c
clang/test/CodeGen/ppc64-dwarf.c
clang/test/Driver/ppc-unsupported.c
Index: clang/test/Driver/ppc-unsupported.c
===
--- clang/test/Driver/ppc-unsupported.c
+++ clang/test/Driver/ppc-unsupported.c
@@ -7,4 +7,12 @@
// RUN: -c %s 2>&1 | FileCheck %s
// RUN: not %clang -target powerpc64le-unknown-linux -msvr4-struct-return \
// RUN: -c %s 2>&1 | FileCheck %s
+// RUN: not %clang -target powerpc-unknown-aix -maix-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
+// RUN: not %clang -target powerpc-unknown-aix -msvr4-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
+// RUN: not %clang -target powerpc64-unknown-aix -maix-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
+// RUN: not %clang -target powerpc64-unknown-aix -msvr4-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
// CHECK: unsupported option
Index: clang/test/CodeGen/ppc64-dwarf.c
===
--- clang/test/CodeGen/ppc64-dwarf.c
+++ clang/test/CodeGen/ppc64-dwarf.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple powerpc64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,PPC64
static unsigned char dwarf_reg_size_table[1024];
int test() {
@@ -119,10 +120,10 @@
// CHECK-NEXT: store i8 16, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 108), align 1
// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 109), align 1
// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 110), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 111), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 112), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 113), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 114), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 115), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 116), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 111), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 112), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 113), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 114), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 115), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 116), align 1
// CHECK-NEXT: ret i32 1
Index: clang/test/CodeGen/ppc32-struct-return.c
===
--- clang/test/CodeGen/ppc32-struct-return.c
+++ /dev/null
@@ -1,88 +0,0 @@
-// REQUIRES: powerpc-registered-target
-// RUN: %clang_cc1 -triple powerpc-unknown-freebsd \
-// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-SVR4
-// RUN: %clang_cc1 -triple powerpc-unknown-linux \
-// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-AIX
-// RUN: %clang_cc1 -triple powerpc-unknown-linux -maix-struct-return \
-// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-AIX
-// RUN: %clang_cc1 -triple powerpc-unknown-linux -msvr4-struct-return \
-// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-SVR4
-// RUN: %clang_cc1 -triple powerpc-unknown-netbsd \
-// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-SVR4
-// RUN: %clang_cc1 -triple powe
[PATCH] D79035: [clang][AIX] Implement ABIInfo and TargetCodeGenInfo for AIX
jasonliu updated this revision to Diff 262964.
jasonliu marked 5 inline comments as done.
jasonliu added a comment.
Address Xiangling's comment.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79035/new/
https://reviews.llvm.org/D79035
Files:
clang/lib/CodeGen/TargetInfo.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/CodeGen/aix-complex.c
clang/test/CodeGen/aix-return.c
clang/test/CodeGen/aix-struct-arg.c
clang/test/CodeGen/aix-vaargs.c
clang/test/CodeGen/aix-vector.c
clang/test/CodeGen/ppc32-and-aix-struct-return.c
clang/test/CodeGen/ppc32-dwarf.c
clang/test/CodeGen/ppc32-struct-return.c
clang/test/CodeGen/ppc64-dwarf.c
clang/test/Driver/ppc-unsupported.c
Index: clang/test/Driver/ppc-unsupported.c
===
--- clang/test/Driver/ppc-unsupported.c
+++ clang/test/Driver/ppc-unsupported.c
@@ -7,4 +7,12 @@
// RUN: -c %s 2>&1 | FileCheck %s
// RUN: not %clang -target powerpc64le-unknown-linux -msvr4-struct-return \
// RUN: -c %s 2>&1 | FileCheck %s
+// RUN: not %clang -target powerpc-unknown-aix -maix-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
+// RUN: not %clang -target powerpc-unknown-aix -msvr4-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
+// RUN: not %clang -target powerpc64-unknown-aix -maix-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
+// RUN: not %clang -target powerpc64-unknown-aix -msvr4-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
// CHECK: unsupported option
Index: clang/test/CodeGen/ppc64-dwarf.c
===
--- clang/test/CodeGen/ppc64-dwarf.c
+++ clang/test/CodeGen/ppc64-dwarf.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple powerpc64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,PPC64
static unsigned char dwarf_reg_size_table[1024];
int test() {
@@ -119,10 +120,10 @@
// CHECK-NEXT: store i8 16, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 108), align 1
// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 109), align 1
// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 110), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 111), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 112), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 113), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 114), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 115), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 116), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 111), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 112), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 113), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 114), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 115), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 116), align 1
// CHECK-NEXT: ret i32 1
Index: clang/test/CodeGen/ppc32-struct-return.c
===
--- clang/test/CodeGen/ppc32-struct-return.c
+++ /dev/null
@@ -1,88 +0,0 @@
-// REQUIRES: powerpc-registered-target
-// RUN: %clang_cc1 -triple powerpc-unknown-freebsd \
-// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-SVR4
-// RUN: %clang_cc1 -triple powerpc-unknown-linux \
-// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-AIX
-// RUN: %clang_cc1 -triple powerpc-unknown-linux -maix-struct-return \
-// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-AIX
-// RUN: %clang_cc1 -triple powerpc-unknown-linux -msvr4-struct-return \
-// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-SVR4
-// RUN: %clang_cc1 -triple powerpc-unknown-netbsd \
-// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-SVR4
-// RUN: %clang_cc1 -triple
[PATCH] D79035: [clang][AIX] Implement ABIInfo and TargetCodeGenInfo for AIX
jasonliu added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:1547 // Otherwise, if the type contains an SSE vector type, the alignment is 16. + if (Align >= 16 && (isSIMDVectorType(getContext(), Ty) || Xiangling_L wrote: > Also update the comment? I think the comment is fine. It's still SSE vector type even if we changed the query to SIMD. SSE is for x86 and altivec is for Power, but they are both SIMD vectors. Comment at: clang/lib/CodeGen/TargetInfo.cpp:4684 return false; case CodeGenOptions::SRCK_InRegs: // -msvr4-struct-return return true; Xiangling_L wrote: > I noticed that in patch https://reviews.llvm.org/D76360, Zarko added a check > to emit an error for using this option within cc1. But in your patch, this > option only emit error when invoked by the driver. Does that mean we are > pretty sure this option is doing what we want on AIX? Are you able to set this CodeGen option when it is disabled in the Frontend/CompilerInvocation.cpp? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79035/new/ https://reviews.llvm.org/D79035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D79035: [clang][AIX] Implement ABIInfo and TargetCodeGenInfo for AIX
jasonliu updated this revision to Diff 263502.
jasonliu added a comment.
Add error report in clang_cc1 for unsupported option on AIX.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79035/new/
https://reviews.llvm.org/D79035
Files:
clang/lib/CodeGen/TargetInfo.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/CodeGen/aix-complex.c
clang/test/CodeGen/aix-return.c
clang/test/CodeGen/aix-struct-arg.c
clang/test/CodeGen/aix-vaargs.c
clang/test/CodeGen/aix-vector.c
clang/test/CodeGen/ppc32-and-aix-struct-return.c
clang/test/CodeGen/ppc32-dwarf.c
clang/test/CodeGen/ppc32-struct-return.c
clang/test/CodeGen/ppc64-dwarf.c
clang/test/Driver/ppc-unsupported.c
clang/test/Frontend/aix-unsupported.c
Index: clang/test/Frontend/aix-unsupported.c
===
--- /dev/null
+++ clang/test/Frontend/aix-unsupported.c
@@ -0,0 +1,10 @@
+// REQUIRES: powerpc-registered-target
+// RUN: not %clang_cc1 -triple powerpc-unknown-aix -maix-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -triple powerpc-unknown-aix -msvr4-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -triple powerpc64-unknown-aix -maix-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -triple powerpc64-unknown-aix -msvr4-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
+// CHECK: unsupported option
Index: clang/test/Driver/ppc-unsupported.c
===
--- clang/test/Driver/ppc-unsupported.c
+++ clang/test/Driver/ppc-unsupported.c
@@ -7,4 +7,12 @@
// RUN: -c %s 2>&1 | FileCheck %s
// RUN: not %clang -target powerpc64le-unknown-linux -msvr4-struct-return \
// RUN: -c %s 2>&1 | FileCheck %s
+// RUN: not %clang -target powerpc-unknown-aix -maix-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
+// RUN: not %clang -target powerpc-unknown-aix -msvr4-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
+// RUN: not %clang -target powerpc64-unknown-aix -maix-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
+// RUN: not %clang -target powerpc64-unknown-aix -msvr4-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
// CHECK: unsupported option
Index: clang/test/CodeGen/ppc64-dwarf.c
===
--- clang/test/CodeGen/ppc64-dwarf.c
+++ clang/test/CodeGen/ppc64-dwarf.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple powerpc64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,PPC64
static unsigned char dwarf_reg_size_table[1024];
int test() {
@@ -119,10 +120,10 @@
// CHECK-NEXT: store i8 16, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 108), align 1
// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 109), align 1
// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 110), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 111), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 112), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 113), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 114), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 115), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 116), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 111), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 112), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 113), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 114), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 115), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 116), align 1
// CHECK-NEXT: ret i32 1
Index: clang/test/CodeGen/ppc32-struct-return.c
===
--- clang/test/CodeGen
[PATCH] D79035: [clang][AIX] Implement ABIInfo and TargetCodeGenInfo for AIX
jasonliu marked an inline comment as done. jasonliu added inline comments. Comment at: clang/test/Frontend/aix-unsupported.c:10 +// RUN: -c %s 2>&1 | FileCheck %s +// CHECK: unsupported option Xiangling_L wrote: > One thing I am not so sure about is that for these two options > `-maix-struct-return`, `-msvr4-struct-return`, do we need to update the > `ClangCommandLineReference.rst` since we emit diags `unsupported option > '-maix-struct-return' for target 'powerpc-unknown-aix'` I think the error message is clear enough in itself what user should expect. I don't think it's necessary to add to the doc. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79035/new/ https://reviews.llvm.org/D79035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D79035: [clang][AIX] Implement ABIInfo and TargetCodeGenInfo for AIX
jasonliu updated this revision to Diff 263864.
jasonliu added a comment.
Remove driver's error.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79035/new/
https://reviews.llvm.org/D79035
Files:
clang/lib/CodeGen/TargetInfo.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/CodeGen/aix-complex.c
clang/test/CodeGen/aix-return.c
clang/test/CodeGen/aix-struct-arg.c
clang/test/CodeGen/aix-vaargs.c
clang/test/CodeGen/aix-vector.c
clang/test/CodeGen/ppc32-and-aix-struct-return.c
clang/test/CodeGen/ppc32-dwarf.c
clang/test/CodeGen/ppc32-struct-return.c
clang/test/CodeGen/ppc64-dwarf.c
clang/test/Frontend/aix-unsupported.c
Index: clang/test/Frontend/aix-unsupported.c
===
--- /dev/null
+++ clang/test/Frontend/aix-unsupported.c
@@ -0,0 +1,10 @@
+// REQUIRES: powerpc-registered-target
+// RUN: not %clang_cc1 -triple powerpc-unknown-aix -maix-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -triple powerpc-unknown-aix -msvr4-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -triple powerpc64-unknown-aix -maix-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -triple powerpc64-unknown-aix -msvr4-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
+// CHECK: unsupported option
Index: clang/test/CodeGen/ppc64-dwarf.c
===
--- clang/test/CodeGen/ppc64-dwarf.c
+++ clang/test/CodeGen/ppc64-dwarf.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple powerpc64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,PPC64
static unsigned char dwarf_reg_size_table[1024];
int test() {
@@ -119,10 +120,10 @@
// CHECK-NEXT: store i8 16, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 108), align 1
// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 109), align 1
// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 110), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 111), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 112), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 113), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 114), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 115), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 116), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 111), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 112), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 113), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 114), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 115), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 116), align 1
// CHECK-NEXT: ret i32 1
Index: clang/test/CodeGen/ppc32-struct-return.c
===
--- clang/test/CodeGen/ppc32-struct-return.c
+++ /dev/null
@@ -1,88 +0,0 @@
-// REQUIRES: powerpc-registered-target
-// RUN: %clang_cc1 -triple powerpc-unknown-freebsd \
-// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-SVR4
-// RUN: %clang_cc1 -triple powerpc-unknown-linux \
-// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-AIX
-// RUN: %clang_cc1 -triple powerpc-unknown-linux -maix-struct-return \
-// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-AIX
-// RUN: %clang_cc1 -triple powerpc-unknown-linux -msvr4-struct-return \
-// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-SVR4
-// RUN: %clang_cc1 -triple powerpc-unknown-netbsd \
-// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-SVR4
-// RUN: %clang_cc1 -triple powerpc-unknown-openbsd \
-// RUN: -emit-llvm -o - %s | FileCheck %s --check-prefix=CHECK-SVR4
-// RUN: %clang_cc1 -triple powerpc-unknown-openbsd -maix-struc
[PATCH] D79035: [clang][AIX] Implement ABIInfo and TargetCodeGenInfo for AIX
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7f5d91d3ffed: [clang][AIX] Implement ABIInfo and
TargetCodeGenInfo for AIX (authored by jasonliu).
Changed prior to commit:
https://reviews.llvm.org/D79035?vs=263864&id=264922#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79035/new/
https://reviews.llvm.org/D79035
Files:
clang/lib/CodeGen/TargetInfo.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/test/CodeGen/aix-complex.c
clang/test/CodeGen/aix-return.c
clang/test/CodeGen/aix-struct-arg.c
clang/test/CodeGen/aix-vaargs.c
clang/test/CodeGen/aix-vector.c
clang/test/CodeGen/ppc32-and-aix-struct-return.c
clang/test/CodeGen/ppc32-dwarf.c
clang/test/CodeGen/ppc32-struct-return.c
clang/test/CodeGen/ppc64-dwarf.c
clang/test/Frontend/aix-unsupported.c
Index: clang/test/Frontend/aix-unsupported.c
===
--- /dev/null
+++ clang/test/Frontend/aix-unsupported.c
@@ -0,0 +1,10 @@
+// REQUIRES: powerpc-registered-target
+// RUN: not %clang_cc1 -triple powerpc-unknown-aix -maix-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -triple powerpc-unknown-aix -msvr4-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -triple powerpc64-unknown-aix -maix-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -triple powerpc64-unknown-aix -msvr4-struct-return \
+// RUN: -c %s 2>&1 | FileCheck %s
+// CHECK: unsupported option
Index: clang/test/CodeGen/ppc64-dwarf.c
===
--- clang/test/CodeGen/ppc64-dwarf.c
+++ clang/test/CodeGen/ppc64-dwarf.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -triple powerpc64-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-unknown-aix -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,PPC64
static unsigned char dwarf_reg_size_table[1024];
int test() {
@@ -119,10 +120,10 @@
// CHECK-NEXT: store i8 16, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 108), align 1
// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 109), align 1
// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 110), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 111), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 112), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 113), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 114), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 115), align 1
-// CHECK-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 116), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 111), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 112), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 113), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 114), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 115), align 1
+// PPC64-NEXT: store i8 8, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i64 0, i64 116), align 1
// CHECK-NEXT: ret i32 1
Index: clang/test/CodeGen/ppc32-dwarf.c
===
--- /dev/null
+++ clang/test/CodeGen/ppc32-dwarf.c
@@ -0,0 +1,126 @@
+// RUN: %clang_cc1 -triple powerpc-unknown-aix -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc-unknown-unknown -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,PPC32
+static unsigned char dwarf_reg_size_table[1024];
+
+int test() {
+ __builtin_init_dwarf_reg_size_table(dwarf_reg_size_table);
+
+ return __builtin_dwarf_sp_column();
+}
+
+// CHECK-LABEL: define i32 @test()
+// CHECK: store i8 4, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i32 0, i32 0), align 1
+// CHECK-NEXT:store i8 4, i8* getelementptr inbounds ([1024 x i8], [1024 x i8]* @dwarf_reg_size_table, i32 0, i32 1), align 1
+// CHECK-NEX
[PATCH] D87451: add new clang option -mignore-xcoff-visibility
jasonliu added inline comments.
Comment at: clang/docs/ClangCommandLineReference.rst:2310
+.. option:: -mignore-xcoff-visibility
+
We should move this option to where all the other -m options resides.
Comment at: clang/docs/ClangCommandLineReference.rst:2312
+
+Ignore all the visibility pragmas and attributes in source code for aix xcoff.
+
Comment at: clang/include/clang/Basic/LangOptions.def:300
ENUM_LANGOPT(GC, GCMode, 2, NonGC, "Objective-C Garbage Collection mode")
+BENIGN_LANGOPT(IgnoreXCOFFVisibility, 1, 0, "All the visibility pragmas and
attributes that are specified in the source are ignored in aix.")
ENUM_LANGOPT(ValueVisibilityMode, Visibility, 3, DefaultVisibility,
nit: AIX XCOFF target
Comment at: clang/include/clang/Driver/Options.td:1939
def dA : Flag<["-"], "dA">, Alias;
+def mignore_xcoff_visibility : Flag<["-"], "mignore-xcoff-visibility">,
Group,
+ HelpText<"Ignore all the visibility pragmas and attributes in source code">,
We should move this option to where the m groups belong.
Comment at: clang/lib/AST/Decl.cpp:1487
+ ? CK.forLinkageOnly()
+ : CK);
}
Question: When I look for `visibility` in CodeGen, I could find some of the
visibility settings there that does not depend on the query of existing
visibility settings. For example, some of the vtable generation, which means we
could still get visibility settings out from IR in some situation?
Does those visibility settings in the CodeGen matters?
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2768
// The type-visibility mode defaults to the value-visibility mode.
if (Arg *typeVisOpt = Args.getLastArg(OPT_ftype_visibility)) {
Opts.setTypeVisibilityMode(parseVisibility(typeVisOpt, Args, Diags));
Question: how should -mignore-xcoff-visiblity interact with -ftype-visibility?
Comment at: clang/test/Driver/ignore-xcoff-visibility.cpp:24
+// RUN: FileCheck -check-prefix=ERROR %s
+
+__attribute__((visibility("hidden"))) void foo_h(int *p) {
I don't feel like this test belongs in the Driver directory.
There is driver test needed, which is to pass in `-###` and specify
-mignore-xcoff-visibility to see if it gets passed by the driver.
But the majority of what this test tests seems belong to CodeGen directory.
Comment at: clang/test/Driver/ignore-xcoff-visibility.cpp:62
+#pragma GCC visibility pop
+
+// IGNOREVISIBILITY:@b = global i32 0
Do we also want to test attribute visibility on a type?
For example:
```
class __attribute__((__visibility__("hidden"))) TestClass {
int foo();
int bar();
}
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87451/new/
https://reviews.llvm.org/D87451
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86790: [FE] Use preferred alignment instead of ABI alignment for complete object when applicable
jasonliu added inline comments. Comment at: clang/include/clang/AST/ASTContext.h:259 mutable TypeInfoMap MemoizedTypeInfo; + /// /// A cache from types to size and preferred alignment information. + mutable TypeInfoMap MemoizedPreferredTypeInfo; nit: Extra /// at the beginning of the comment could be removed. Comment at: clang/include/clang/AST/ASTContext.h:2133 + /// 'arm_sve_vector_bits'. Should only be called if T->isVLST(). + unsigned getBitwidthForAttributedSveType(const Type *T) const; + Maybe you want to move up to a newer base? I don't see how this change belong to this patch. Comment at: clang/lib/AST/ASTContext.cpp:1872 - // This call can invalidate MemoizedTypeInfo[T], so we need a second lookup. - TypeInfo TI = getTypeInfoImpl(T); - MemoizedTypeInfo[T] = TI; - return TI; +// This call can invalidate MemoizedTypeInfo[T], so we need a second lookup. +TypeInfo TI = getTypeInfoImpl(T, NeedsPreferredAlignment); nit: This comment needs an update, as it could invalidate MemoizedPreferredTypeInfo depending on the flag. Comment at: clang/lib/AST/ASTContext.cpp:2106 Width = Target->getDoubleWidth(); - Align = Target->getDoubleAlign(); + setAlign(Width, Target->getDoubleAlign()); break; I have two concerns here: 1. I feel like we are having duplicate logic between here and ASTContext::getPreferredTypeAlign for how to get preferred alignment. 2. These logic are too AIX-centric and only modified the places for where types are affected by preferred alignment on AIX. What if on some platforms, BuiltinType::Float have different preferred alignments? Or Width is not the preferred alignment on certain platform for double/long double. Comment at: clang/lib/CodeGen/TargetInfo.cpp:4539 CharUnits CCAlign = getParamTypeAlignment(Ty); CharUnits TyAlign = getContext().getTypeAlignInChars(Ty); Question: It looks like getNaturalAlignIndirect and getTypeAlignInChars here are all returning ABI alignment. But according to the comments, we should use a preferred alignment when it's a complete object. Isn't this complete object? Or I'm missing something? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86790/new/ https://reviews.llvm.org/D86790 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D87702: [Frontend] Add pragma align natural and sort out pragma pack stack effect
jasonliu added inline comments.
Comment at: clang/include/clang/Sema/Sema.h:66
#include "llvm/Frontend/OpenMP/OMPConstants.h"
+#include
#include
Do we need cmath?
Comment at: clang/include/clang/Sema/Sema.h:488
+AlignPackInfo(AlignPackInfo::Mode M, int Num, bool IsAIX)
+: PackAttr(true), AlignMode(M), PackNumber(Num), AIXStack(IsAIX) {}
+
I noticed PackNumber is an unsigned char and we are passing an int type into it.
Could we add an assertion in the constructor to make sure Num would never be
something that's going to get truncated when it converts to an unsigned char?
Comment at: clang/include/clang/Sema/Sema.h:504
+unsigned getPackNumber() const { return PackNumber; }
+void setPackNumber(int Num) { PackNumber = Num; }
+
I don't see this function gets referenced in this patch.
Comment at: clang/include/clang/Sema/Sema.h:514
+
+bool operator==(AlignPackInfo Info) const {
+ return AlignMode == Info.AlignMode && PackNumber == Info.PackNumber;
I think a normal operator== signature would look like this:
bool operator==(const AlignPackInfo &Info) const;
Comment at: clang/include/clang/Sema/Sema.h:515
+bool operator==(AlignPackInfo Info) const {
+ return AlignMode == Info.AlignMode && PackNumber == Info.PackNumber;
+}
This could return true when `PackAttr` in AlignPackInfo are not the same.
Wouldn't that cause an issue?
Comment at: clang/include/clang/Sema/Sema.h:519
+bool operator!=(AlignPackInfo Info) const {
+ return AlignMode != Info.AlignMode || PackNumber != Info.PackNumber;
+}
You should be able to implement this using "operator==" right?
e.g. return !(*this == Info);
Comment at: clang/include/clang/Sema/Sema.h:524
+/// \brief True if this is a pragma pack attribute,
+// not a pragma align attribute.
+bool PackAttr;
Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:697
InferAlignment(false), Packed(false), IsUnion(false),
-IsMac68kAlign(false), IsMsStruct(false), UnfilledBitsInLastUnit(0),
-LastBitfieldTypeSize(0), MaxFieldAlignment(CharUnits::Zero()),
-DataSize(0), NonVirtualSize(CharUnits::Zero()),
+IsMac68kAlign(false), IsNaturalAlign(false), IsMsStruct(false),
+UnfilledBitsInLastUnit(0), LastBitfieldTypeSize(0),
This could probably confuse people, as natural alignment is the default on most
target except AIX.
Comment at: clang/lib/Sema/SemaAttr.cpp:403
// Warn about modified alignment after #includes.
if (PrevPackState.CurrentValue != PackStack.CurrentValue) {
Diag(IncludeLoc, diag::warn_pragma_pack_modified_after_include);
Since we changed the PackStack for it to contain AlignPackInfo instead of
unsigned.
This stack no longer only contains Pack information. So we need to rethink
about how this diagnostic and the one follows should work.
i.e. What's the purpose of these diagnostic? Is it still only for pragma pack
report? If so, what we are doing here is not correct, since the `CurrentValue`
could be different, not because of the pragma pack change, but because of the
pragma align change.
If it's not only for pragma pack any more, but also intend to detect the pragma
align interaction, then possibly function name and diagnostic needs some
modification, as they don't match the intent any more.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87702/new/
https://reviews.llvm.org/D87702
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D87904: [AIX][Clang][Driver] Add handling of nostartfiles option
jasonliu accepted this revision. jasonliu 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/D87904/new/ https://reviews.llvm.org/D87904 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D87914: [AIX][Clang][Driver] Add handling of shared option
jasonliu accepted this revision. jasonliu 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/D87914/new/ https://reviews.llvm.org/D87914 ___ 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
jasonliu added a comment. I think it would help the review if we could put the NFC portion(e.g. TypeSize -> StorageUnitSize) to a new patch, and give some rationale about the NFC change. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:624 /// a byte, but larger units are used if IsMsStruct. unsigned char UnfilledBitsInLastUnit; + /// LastBitfieldStorageUnitSize - If IsMsStruct, represents the size of the 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] D87451: add new clang option -mno-xcoff-visibility
jasonliu added inline comments. Comment at: clang/docs/ClangCommandLineReference.rst:2310 +.. option:: -mno-xcoff-visibility + It's rare to see an option with only the negative form. Could we rename and make it a positive form somehow? Also we would need to move this option to where the m group belongs. Comment at: clang/test/CodeGen/aix-no-xcoff-visibility.cpp:1 +// RUN: %clang -target powerpc-unknown-aix -emit-llvm -o - -S %s |\ +// RUN: FileCheck --check-prefix=VISIBILITY-IR %s I don't think we should call the driver directly in here. We should have a separate driver test where we invoke `clang`, and we should invoke the front end `clang_cc1` here. Comment at: clang/test/CodeGen/aix-no-xcoff-visibility.cpp:75 + +// VISIBILITY-IR:@b = protected global i32 0 +// VISIBILITY-IR:@pramb = hidden global i32 0 Not sure if the IR check is really necessary, since we haven't made any IR change here. It's going to be all the same with or without the new -m option. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87451/new/ https://reviews.llvm.org/D87451 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D87451: add new clang option -mno-xcoff-visibility
jasonliu added inline comments.
Comment at: llvm/include/llvm/Target/TargetMachine.h:265
+ /// corresponding to -mno-xcoff-visibility.
+ bool getNoXCOFFVisibility() const { return Options.NoXCOFFVisibility; }
+
DiggerLin wrote:
> daltenty wrote:
> > This seems like it needs the corresponding comand-line option for llc added
> > and an llc test.
> I think it will be in another separate patch.
I would actually prefer to have that in the same patch, as that would give us a
full picture. It's not a huge patch even if we combine them.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87451/new/
https://reviews.llvm.org/D87451
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D86790: [FE] Use preferred alignment instead of ABI alignment for complete object when applicable
jasonliu added inline comments.
Comment at: clang/test/CodeGenCXX/aix-alignment.cpp:16
+// AIX64: %call = call noalias nonnull i8* @_Znam(i64 8)
+B *allocBp() { return new B[0]; }
+
I believe we would also want to add a test for the delete call to verify the
change we made:
```
void bar() {
delete[] allocBp();
}
```
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86790/new/
https://reviews.llvm.org/D86790
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D87702: [Frontend] Add pragma align natural and sort out pragma pack stack effect
jasonliu added inline comments.
Comment at: clang/include/clang/Sema/Sema.h:505
+// AlignPackInfo::getFromRawEncoding, it should not be inspected directly.
+uint32_t getRawEncoding(const AlignPackInfo &Info) const {
+ std::uint32_t Encoding{};
this getter do not need to take any parameters. You could just use `*this`.
Or, to make it consistent with the getFromRawEncoding, we could make this a
static function as well and keep the parameter.
Comment at: clang/include/clang/Sema/Sema.h:506
+uint32_t getRawEncoding(const AlignPackInfo &Info) const {
+ std::uint32_t Encoding{};
+ if (Info.IsAIXStack())
Since this new structure have the same size as the uint32_t, have you consider
using memcpy to burn the data directly into the uint32_t, and vice versa?
Comment at: clang/include/clang/Sema/Sema.h:570
+/// \brief True if it is a AIX #pragma align/pack stack.
+bool AIXStack;
+
It might be better to rename it XLStack, as this is the XL compiler behavior on
AIX. Other compilers on AIX, like gcc, could have different behavior.
Comment at: clang/include/clang/Sema/Sema.h:637
PragmaStack VtorDispStack;
- // #pragma pack.
- // Sentinel to represent when the stack is set to mac68k alignment.
- static const unsigned kMac68kAlignmentSentinel = ~0U;
- PragmaStack PackStack;
+ PragmaStack PackStack;
// The current #pragma pack values and locations at each #include.
Xiangling_L wrote:
> jasonliu wrote:
> > We should consider renaming PackStack to AlignPackStack across Clang. Maybe
> > even as a NFC first. As it is right now, clang already uses one stack to
> > record those two informations from `Pragma align` and `Pragma pack`. Leave
> > it as PackStack is too confusing, and people could actually ignore the
> > pragma Align when developing with PackStack.
> That's a good point. I will create a NFC accordingly.
Please post this NFC when you have time.
Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:699-700
+IsMac68kAlign(false),
+IsNaturalAlign(!Context.getTargetInfo().getTriple().isOSAIX() ? true
+ : false),
+IsMsStruct(false), UnfilledBitsInLastUnit(0),
Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1322
HandledFirstNonOverlappingEmptyField =
- !Context.getTargetInfo().defaultsToAIXPowerAlignment();
+ !Context.getTargetInfo().defaultsToAIXPowerAlignment() || IsNaturalAlign;
Not sure if this would have a real effect, but reading this code concerns me a
little bit:
We would set `HandledFirstNonOverlappingEmptyField` based on `IsNaturalAlign`,
but for AIX, that bit might only get set at line 1343.
It would look like we have a read before write to the variable here.
Comment at: clang/lib/Sema/SemaAttr.cpp:232
Action = Sema::PSK_Push_Set;
-Alignment = 0;
+if (IsAIX)
+ ModeVal = AlignPackInfo::Natural;
XLPragmaPack is used to control pack and align stack semantic behavior.
Someone on AIX might want to turn off this compatible pack and align with XL
behavior (to get the compatible behavior with clang on other platform for
easier porting purpose), but they would still prefer to have the natural
alignment to work correctly.
So it might not be desirable to query this option for this natural alignment
behavior.
Comment at: clang/lib/Sema/SemaAttr.cpp:413
+ // FIXME: PackStack may contain both #pragma align and #pragma pack
+ // information, we should warn about both modified align mode and alignment.
if (PrevPackState.CurrentValue != PackStack.CurrentValue) {
I don't think there is a preferable solution yet. Pointing out what could go
wrong is good enough here.
Same for the FIXME below.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87702/new/
https://reviews.llvm.org/D87702
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D91455: [XCOFF][AIX] Generate LSDA data and compact unwind section on AIX
jasonliu created this revision.
jasonliu added reviewers: xingxue, hubert.reinterpretcast, cebowleratibm,
DiggerLin, daltenty.
Herald added subscribers: llvm-commits, kbarton, hiraditya, mgorny, nemanjai.
Herald added a project: LLVM.
jasonliu requested review of this revision.
Herald added a subscriber: aheejin.
AIX uses the existing EH infrastructure in clang and llvm.
The major differences would be
1. AIX do not have CFI instructions.
2. AIX uses a new personality routine, named __xlcxx_personality_v1. It doesn't
use the GCC personality rountine, because the intractability is not there yet
on AIX.
3. AIX do not use eh_frame sections. Instead, it would use a eh_info section
(compat unwind section) to store the information about personality routine and
LSDA data address.
https://reviews.llvm.org/D91455
Files:
clang/lib/CodeGen/CGCleanup.h
clang/lib/CodeGen/CGException.cpp
clang/test/CodeGenCXX/personality.cpp
llvm/include/llvm/Analysis/EHPersonalities.h
llvm/include/llvm/CodeGen/AsmPrinter.h
llvm/include/llvm/MC/MCTargetOptions.h
llvm/lib/Analysis/EHPersonalities.cpp
llvm/lib/CodeGen/AsmPrinter/AIXException.cpp
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
llvm/lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp
llvm/lib/CodeGen/AsmPrinter/CMakeLists.txt
llvm/lib/CodeGen/AsmPrinter/DwarfException.h
llvm/lib/CodeGen/AsmPrinter/EHStreamer.cpp
llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
llvm/lib/CodeGen/TargetPassConfig.cpp
llvm/lib/MC/MCAsmInfoXCOFF.cpp
llvm/lib/MC/MCObjectFileInfo.cpp
llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
llvm/test/CodeGen/PowerPC/aix-exception.ll
Index: llvm/test/CodeGen/PowerPC/aix-exception.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/aix-exception.ll
@@ -0,0 +1,152 @@
+; RUN: llc -verify-machineinstrs -mtriple powerpc-ibm-aix-xcoff -mcpu=pwr4 \
+; RUN: -mattr=-altivec < %s | \
+; RUN: FileCheck --check-prefixes=ASM,ASM32 %s
+
+; RUN: llc -verify-machineinstrs -mtriple powerpc64-ibm-aix-xcoff -mcpu=pwr4 \
+; RUN: -mattr=-altivec < %s | \
+; RUN: FileCheck --check-prefixes=ASM,ASM64 %s
+
+@_ZTIi = external constant i8*
+
+define void @_Z9throwFuncv() {
+entry:
+ %exception = call i8* @__cxa_allocate_exception(i32 4) #2
+ %0 = bitcast i8* %exception to i32*
+ store i32 1, i32* %0, align 16
+ call void @__cxa_throw(i8* %exception, i8* bitcast (i8** @_ZTIi to i8*), i8* null) #3
+ unreachable
+}
+
+; ASM:._Z9throwFuncv:
+; ASM: bl .__cxa_allocate_exception[PR]
+; ASM: nop
+; ASM32:lwz 4, L..C0(2)
+; ASM64:ld 4, L..C0(2)
+; ASM: bl .__cxa_throw[PR]
+; ASM: nop
+
+define i32 @_Z9catchFuncv() personality i8* bitcast (i32 (...)* @__xlcxx_personality_v1 to i8*) {
+entry:
+ %retval = alloca i32, align 4
+ %exn.slot = alloca i8*, align 4
+ %ehselector.slot = alloca i32, align 4
+ %0 = alloca i32, align 4
+ invoke void @_Z9throwFuncv()
+ to label %invoke.cont unwind label %lpad
+
+invoke.cont: ; preds = %entry
+ br label %try.cont
+
+lpad: ; preds = %entry
+ %1 = landingpad { i8*, i32 }
+ catch i8* bitcast (i8** @_ZTIi to i8*)
+ %2 = extractvalue { i8*, i32 } %1, 0
+ store i8* %2, i8** %exn.slot, align 4
+ %3 = extractvalue { i8*, i32 } %1, 1
+ store i32 %3, i32* %ehselector.slot, align 4
+ br label %catch.dispatch
+
+catch.dispatch: ; preds = %lpad
+ %sel = load i32, i32* %ehselector.slot, align 4
+ %4 = call i32 @llvm.eh.typeid.for(i8* bitcast (i8** @_ZTIi to i8*)) #2
+ %matches = icmp eq i32 %sel, %4
+ br i1 %matches, label %catch, label %eh.resume
+
+catch:; preds = %catch.dispatch
+ %exn = load i8*, i8** %exn.slot, align 4
+ %5 = call i8* @__cxa_begin_catch(i8* %exn) #2
+ %6 = bitcast i8* %5 to i32*
+ %7 = load i32, i32* %6, align 4
+ store i32 %7, i32* %0, align 4
+ store i32 2, i32* %retval, align 4
+ call void @__cxa_end_catch() #2
+ br label %return
+
+try.cont: ; preds = %invoke.cont
+ store i32 1, i32* %retval, align 4
+ br label %return
+
+return: ; preds = %try.cont, %catch
+ %8 = load i32, i32* %retval, align 4
+ ret i32 %8
+
+eh.resume:; preds = %catch.dispatch
+ %exn1 = load i8*, i8** %exn.slot, align 4
+ %sel2 = load i32, i32* %ehselector.slot, align 4
+ %lpad.val = insertvalue { i8*, i32 } undef, i8* %exn1, 0
+ %lpad.val3 = insertvalue { i8*, i32 } %lpad.val, i32 %sel2, 1
+ resume { i8*, i32 } %lpad.val3
+}
+
+; ASM: ._Z9catchFuncv:
+; ASM: L..func_begin0:
+; ASM: # %bb.0:# %entry
+; ASM: mflr 0
+; ASM: L..tmp0:
+; ASM: bl ._Z9throwFuncv
+; ASM: nop
+; ASM: L..tmp1:
+; ASM: # %bb.1:
