[macppc] devel/llvm: add clang -msvr4-struct-return

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[macppc] devel/llvm: add clang -msvr4-struct-return

George Koehler-2
Hello ports!

In OpenBSD macppc, clang and gcc use incompatible ABIs to return small
structs: gcc defaults to -msvr4-struct-return, but clang always acts
like gcc -maix-struct-return.  This causes crashes at runtime: for
example, clang code can't call libxcb (in Xenocara built by gcc).

This diff for ports-clang adds clang -msvr4-struct-return, and uses it
by default for OpenBSD powerpc.  I proposed the diff upstream at
https://reviews.llvm.org/D73290 (where Mark Millard reported that the
diff seems to work so far on FreeBSD).

My iMac G3 (1x 400 MHz PowerPC 750, 512 M RAM) with OpenBSD macppc is
trying to build devel/llvm with this diff, but after 45 hours, has built
less than half of 4470 targets.  (If I had not broken my PowerBook G4,
it might have completed the build by now.)

My amd64 desktop built devel/llvm with this diff days ago.  I can now
cross-compile, where I run 'clang -c' on amd64, scp the *.o to my iMac,
then use base-gcc to link.

The attachment "sret-examples.tar.gz" contains my tiny example programs
and a Makefile set to use ports-clang as CLANG and base-gcc as GCC.  Run
'make' to build smain-[cg][cg] and xcbtest.  The 4 smain-* try different
combinations of clang and gcc; they should show 12 ok tests.  The
xcbtest requires an X server and should show terminal output like,

$ ./xcbtest                                                          
conn = 0xb16f3033000
cookie.sequence = 1
reply = 0xb173fd873e0

where conn and reply should not be zero.  (This output is from amd64;
I can't run xcbtest on my iMac because X11 isn't working.)

On my iMac, 'make CLANG=/usr/bin/clang' uses a base-clang without this
diff: smain-cg (clang calling gcc) fails most tests and smain-gc (gcc
calling clang) crashes.

To cross-compile the clang parts from amd64, I do
$ make clang CLANG='/usr/local/bin/clang -target ppc-openbsd'
and scp the *.o files to my iMac.  This fixes smain-cg and smain-gc.

If you want to build devel/llvm on macppc, I suggest a G4 or G5 with at
least 1024M of RAM.  (My iMac with 512M RAM took over 600M swap to
compile ASTDumper; the build seemed to freeze for a few hours.)  If you
build it, you might try 'make COMPILER=ports-clang' on some ports.  If
a port so built does crash at runtime, it might be hard to know whether
a struct return or some other clang bug caused the crash.

I'm still waiting for my iMac to build this.  Later, I might try to
apply this diff to base-clang.  I won't commit this diff to ports-clang
before I know what to do with base-clang.

--George

Index: Makefile
===================================================================
RCS file: /cvs/ports/devel/llvm/Makefile,v
retrieving revision 1.240
diff -u -p -r1.240 Makefile
--- Makefile 7 Jan 2020 22:59:43 -0000 1.240
+++ Makefile 24 Jan 2020 17:37:47 -0000
@@ -18,7 +18,7 @@ PKGSPEC-main = llvm-=${LLVM_V}
 PKGNAME-main = llvm-${LLVM_V}
 PKGNAME-python = py-llvm-${LLVM_V}
 PKGNAME-lldb = lldb-${LLVM_V}
-REVISION-main = 5
+REVISION-main = 6
 REVISION-lldb= 0
 
 CATEGORIES = devel
Index: patches/patch-tools_clang_include_clang_Driver_Options_td
===================================================================
RCS file: /cvs/ports/devel/llvm/patches/patch-tools_clang_include_clang_Driver_Options_td,v
retrieving revision 1.19
diff -u -p -r1.19 patch-tools_clang_include_clang_Driver_Options_td
--- patches/patch-tools_clang_include_clang_Driver_Options_td 6 Jul 2019 15:06:36 -0000 1.19
+++ patches/patch-tools_clang_include_clang_Driver_Options_td 24 Jan 2020 17:37:47 -0000
@@ -2,6 +2,7 @@ $OpenBSD: patch-tools_clang_include_clan
 
 - Add ret protctor options as no-ops.
 - Improve the X86FixupGadgets pass
+- Add -msvr4-struct-return for powerpc.
 - Alias the command line parameter -p to -pg.
 - implement -msave-args in clang/llvm, like the sun did for gcc
 
@@ -23,7 +24,20 @@ Index: tools/clang/include/clang/Driver/
  def fstandalone_debug : Flag<["-"], "fstandalone-debug">, Group<f_Group>, Flags<[CoreOption]>,
    HelpText<"Emit full debug info for all types used by the program">;
  def fno_standalone_debug : Flag<["-"], "fno-standalone-debug">, Group<f_Group>, Flags<[CoreOption]>,
-@@ -2500,7 +2508,7 @@ def pthreads : Flag<["-"], "pthreads">;
+@@ -2230,6 +2238,12 @@ def mlongcall: Flag<["-"], "mlongcall">,
+     Group<m_ppc_Features_Group>;
+ def mno_longcall : Flag<["-"], "mno-longcall">,
+     Group<m_ppc_Features_Group>;
++def maix_struct_return : Flag<["-"], "maix-struct-return">,
++  Group<m_Group>, Flags<[CC1Option]>,
++  HelpText<"Return all structs in memory (PPC32 only)">;
++def msvr4_struct_return : Flag<["-"], "msvr4-struct-return">,
++  Group<m_Group>, Flags<[CC1Option]>,
++  HelpText<"Return small structs in registers (PPC32 only)">;
+
+ def mvx : Flag<["-"], "mvx">, Group<m_Group>;
+ def mno_vx : Flag<["-"], "mno-vx">, Group<m_Group>;
+@@ -2500,7 +2514,7 @@ def pthreads : Flag<["-"], "pthreads">;
  def pthread : Flag<["-"], "pthread">, Flags<[CC1Option]>,
    HelpText<"Support POSIX threads in generated code">;
  def no_pthread : Flag<["-"], "no-pthread">, Flags<[CC1Option]>;
@@ -32,7 +46,7 @@ Index: tools/clang/include/clang/Driver/
  def pie : Flag<["-"], "pie">;
  def read__only__relocs : Separate<["-"], "read_only_relocs">;
  def remap : Flag<["-"], "remap">;
-@@ -2949,6 +2957,8 @@ def mshstk : Flag<["-"], "mshstk">, Group<m_x86_Featur
+@@ -2949,6 +2963,8 @@ def mshstk : Flag<["-"], "mshstk">, Group<m_x86_Featur
  def mno_shstk : Flag<["-"], "mno-shstk">, Group<m_x86_Features_Group>;
  def mretpoline_external_thunk : Flag<["-"], "mretpoline-external-thunk">, Group<m_x86_Features_Group>;
  def mno_retpoline_external_thunk : Flag<["-"], "mno-retpoline-external-thunk">, Group<m_x86_Features_Group>;
Index: patches/patch-tools_clang_lib_CodeGen_TargetInfo_cpp
===================================================================
RCS file: patches/patch-tools_clang_lib_CodeGen_TargetInfo_cpp
diff -N patches/patch-tools_clang_lib_CodeGen_TargetInfo_cpp
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-tools_clang_lib_CodeGen_TargetInfo_cpp 24 Jan 2020 17:37:47 -0000
@@ -0,0 +1,128 @@
+$OpenBSD$
+
+Add -msvr4-struct-return for powerpc.
+
+Index: tools/clang/lib/CodeGen/TargetInfo.cpp
+--- tools/clang/lib/CodeGen/TargetInfo.cpp.orig
++++ tools/clang/lib/CodeGen/TargetInfo.cpp
+@@ -4092,22 +4092,39 @@ namespace {
+ /// PPC32_SVR4_ABIInfo - The 32-bit PowerPC ELF (SVR4) ABI information.
+ class PPC32_SVR4_ABIInfo : public DefaultABIInfo {
+   bool IsSoftFloatABI;
++  bool IsRetSmallStructInRegABI;
+
+   CharUnits getParamTypeAlignment(QualType Ty) const;
+
+ public:
+-  PPC32_SVR4_ABIInfo(CodeGen::CodeGenTypes &CGT, bool SoftFloatABI)
+-      : DefaultABIInfo(CGT), IsSoftFloatABI(SoftFloatABI) {}
++  PPC32_SVR4_ABIInfo(CodeGen::CodeGenTypes &CGT, bool SoftFloatABI,
++                     bool RetSmallStructInRegABI)
++      : DefaultABIInfo(CGT), IsSoftFloatABI(SoftFloatABI),
++        IsRetSmallStructInRegABI(RetSmallStructInRegABI) {}
+
++  ABIArgInfo classifyReturnType(QualType RetTy) const;
++
++  void computeInfo(CGFunctionInfo &FI) const override {
++    if (!getCXXABI().classifyReturnType(FI))
++      FI.getReturnInfo() = classifyReturnType(FI.getReturnType());
++    for (auto &I : FI.arguments())
++      I.info = classifyArgumentType(I.type);
++  }
++
+   Address EmitVAArg(CodeGenFunction &CGF, Address VAListAddr,
+                     QualType Ty) const override;
+ };
+
+ class PPC32TargetCodeGenInfo : public TargetCodeGenInfo {
+ public:
+-  PPC32TargetCodeGenInfo(CodeGenTypes &CGT, bool SoftFloatABI)
+-      : TargetCodeGenInfo(new PPC32_SVR4_ABIInfo(CGT, SoftFloatABI)) {}
++  PPC32TargetCodeGenInfo(CodeGenTypes &CGT, bool SoftFloatABI,
++                         bool RetSmallStructInRegABI)
++      : TargetCodeGenInfo(new PPC32_SVR4_ABIInfo(CGT, SoftFloatABI,
++                                                 RetSmallStructInRegABI)) {}
+
++  static bool isStructReturnInRegABI(const llvm::Triple &Triple,
++                                     const CodeGenOptions &Opts);
++
+   int getDwarfEHStackPointer(CodeGen::CodeGenModule &M) const override {
+     // This is recovered from gcc output.
+     return 1; // r1 is the dedicated stack pointer
+@@ -4142,6 +4159,34 @@ CharUnits PPC32_SVR4_ABIInfo::getParamTypeAlignment(Qu
+   return CharUnits::fromQuantity(4);
+ }
+
++ABIArgInfo PPC32_SVR4_ABIInfo::classifyReturnType(QualType RetTy) const {
++  uint64_t Size;
++
++  // -msvr4-struct-return puts small aggregates in GPR3 and GPR4.
++  if (isAggregateTypeForABI(RetTy) && IsRetSmallStructInRegABI &&
++      (Size = getContext().getTypeSize(RetTy)) <= 64) {
++    // System V ABI (1995), page 3-22, specified:
++    // > A structure or union whose size is less than or equal to 8 bytes
++    // > shall be returned in r3 and r4, as if it were first stored in the
++    // > 8-byte aligned memory area and then the low addressed word were
++    // > loaded into r3 and the high-addressed word into r4.  Bits beyond
++    // > the last member of the structure or union are not defined.
++    //
++    // GCC for big-endian PPC32 inserts the pad before the first member,
++    // not "beyond the last member" of the struct.  To stay compatible
++    // with GCC, we coerce the struct to an integer of the same size.
++    // LLVM will extend it and return i32 in r3, or i64 in r3:r4.
++    if (Size == 0)
++      return ABIArgInfo::getIgnore();
++    else {
++      llvm::Type *CoerceTy = llvm::Type::getIntNTy(getVMContext(), Size);
++      return ABIArgInfo::getDirect(CoerceTy);
++    }
++  }
++
++  return DefaultABIInfo::classifyReturnType(RetTy);
++}
++
+ // TODO: this implementation is now likely redundant with
+ // DefaultABIInfo::EmitVAArg.
+ Address PPC32_SVR4_ABIInfo::EmitVAArg(CodeGenFunction &CGF, Address VAList,
+@@ -4299,6 +4344,25 @@ Address PPC32_SVR4_ABIInfo::EmitVAArg(CodeGenFunction
+   return Result;
+ }
+
++bool PPC32TargetCodeGenInfo::isStructReturnInRegABI(
++    const llvm::Triple &Triple, const CodeGenOptions &Opts) {
++  assert(Triple.getArch() == llvm::Triple::ppc);
++
++  switch (Opts.getStructReturnConvention()) {
++  case CodeGenOptions::SRCK_Default:
++    break;
++  case CodeGenOptions::SRCK_OnStack: // -maix-struct-return
++    return false;
++  case CodeGenOptions::SRCK_InRegs: // -msvr4-struct-return
++    return true;
++  }
++
++  if (Triple.isOSBinFormatELF() && !Triple.isOSLinux())
++    return true;
++
++  return false;
++}
++
+ bool
+ PPC32TargetCodeGenInfo::initDwarfEHRegSizeTable(CodeGen::CodeGenFunction &CGF,
+                                                 llvm::Value *Address) const {
+@@ -9330,9 +9394,13 @@ const TargetCodeGenInfo &CodeGenModule::getTargetCodeG
+     return SetCGInfo(new ARMTargetCodeGenInfo(Types, Kind));
+   }
+
+-  case llvm::Triple::ppc:
++  case llvm::Triple::ppc: {
++    bool RetSmallStructInRegABI =
++        PPC32TargetCodeGenInfo::isStructReturnInRegABI(Triple, CodeGenOpts);
+     return SetCGInfo(
+-        new PPC32TargetCodeGenInfo(Types, CodeGenOpts.FloatABI == "soft"));
++        new PPC32TargetCodeGenInfo(Types, CodeGenOpts.FloatABI == "soft",
++                                   RetSmallStructInRegABI));
++  }
+   case llvm::Triple::ppc64:
+     if (Triple.isOSBinFormatELF()) {
+       PPC64_SVR4_ABIInfo::ABIKind Kind = PPC64_SVR4_ABIInfo::ELFv1;
Index: patches/patch-tools_clang_lib_Driver_ToolChains_Clang_cpp
===================================================================
RCS file: /cvs/ports/devel/llvm/patches/patch-tools_clang_lib_Driver_ToolChains_Clang_cpp,v
retrieving revision 1.11
diff -u -p -r1.11 patch-tools_clang_lib_Driver_ToolChains_Clang_cpp
--- patches/patch-tools_clang_lib_Driver_ToolChains_Clang_cpp 7 Jan 2020 22:59:43 -0000 1.11
+++ patches/patch-tools_clang_lib_Driver_ToolChains_Clang_cpp 24 Jan 2020 17:37:47 -0000
@@ -1,5 +1,6 @@
 $OpenBSD: patch-tools_clang_lib_Driver_ToolChains_Clang_cpp,v 1.11 2020/01/07 22:59:43 jca Exp $
 
+- Add -msvr4-struct-return for powerpc.
 - Make LLVM create strict aligned code for OpenBSD/arm64.
 - Disable -fstrict-aliasing per default on OpenBSD.
 - Enable -fwrapv by default
@@ -33,7 +34,27 @@ $OpenBSD: patch-tools_clang_lib_Driver_T
 Index: tools/clang/lib/Driver/ToolChains/Clang.cpp
 --- tools/clang/lib/Driver/ToolChains/Clang.cpp.orig
 +++ tools/clang/lib/Driver/ToolChains/Clang.cpp
-@@ -3899,9 +3899,12 @@ void Clang::ConstructJob(Compilation &C, const JobActi
+@@ -3870,6 +3870,19 @@ void Clang::ConstructJob(Compilation &C, const JobActi
+     CmdArgs.push_back(A->getValue());
+   }
+
++  if (Arg *A = Args.getLastArg(options::OPT_maix_struct_return,
++                               options::OPT_msvr4_struct_return)) {
++    if (TC.getArch() != llvm::Triple::ppc) {
++      D.Diag(diag::err_drv_unsupported_opt_for_target)
++          << A->getSpelling() << RawTriple.str();
++    } else if (A->getOption().matches(options::OPT_maix_struct_return)) {
++      CmdArgs.push_back("-maix-struct-return");
++    } else {
++      assert(A->getOption().matches(options::OPT_msvr4_struct_return));
++      CmdArgs.push_back("-msvr4-struct-return");
++    }
++  }
++
+   if (Arg *A = Args.getLastArg(options::OPT_fpcc_struct_return,
+                                options::OPT_freg_struct_return)) {
+     if (TC.getArch() != llvm::Triple::x86) {
+@@ -3899,9 +3912,12 @@ void Clang::ConstructJob(Compilation &C, const JobActi
        OFastEnabled ? options::OPT_Ofast : options::OPT_fstrict_aliasing;
    // We turn strict aliasing off by default if we're in CL mode, since MSVC
    // doesn't do any TBAA.
@@ -48,7 +69,7 @@ Index: tools/clang/lib/Driver/ToolChains
      CmdArgs.push_back("-relaxed-aliasing");
    if (!Args.hasFlag(options::OPT_fstruct_path_tbaa,
                      options::OPT_fno_struct_path_tbaa))
-@@ -4527,7 +4530,8 @@ void Clang::ConstructJob(Compilation &C, const JobActi
+@@ -4527,7 +4543,8 @@ void Clang::ConstructJob(Compilation &C, const JobActi
                                        options::OPT_fno_strict_overflow)) {
      if (A->getOption().matches(options::OPT_fno_strict_overflow))
        CmdArgs.push_back("-fwrapv");
@@ -58,7 +79,7 @@ Index: tools/clang/lib/Driver/ToolChains
 
    if (Arg *A = Args.getLastArg(options::OPT_freroll_loops,
                                 options::OPT_fno_reroll_loops))
-@@ -4544,9 +4548,46 @@ void Clang::ConstructJob(Compilation &C, const JobActi
+@@ -4544,9 +4561,46 @@ void Clang::ConstructJob(Compilation &C, const JobActi
                     false))
      CmdArgs.push_back(Args.MakeArgString("-mspeculative-load-hardening"));
 
@@ -106,7 +127,7 @@ Index: tools/clang/lib/Driver/ToolChains
    // Translate -mstackrealign
    if (Args.hasFlag(options::OPT_mstackrealign, options::OPT_mno_stackrealign,
                     false))
-@@ -5029,6 +5070,18 @@ void Clang::ConstructJob(Compilation &C, const JobActi
+@@ -5029,6 +5083,18 @@ void Clang::ConstructJob(Compilation &C, const JobActi
                                       options::OPT_fno_rewrite_imports, false);
    if (RewriteImports)
      CmdArgs.push_back("-frewrite-imports");
Index: patches/patch-tools_clang_lib_Frontend_CompilerInvocation_cpp
===================================================================
RCS file: /cvs/ports/devel/llvm/patches/patch-tools_clang_lib_Frontend_CompilerInvocation_cpp,v
retrieving revision 1.3
diff -u -p -r1.3 patch-tools_clang_lib_Frontend_CompilerInvocation_cpp
--- patches/patch-tools_clang_lib_Frontend_CompilerInvocation_cpp 6 Jul 2019 15:06:36 -0000 1.3
+++ patches/patch-tools_clang_lib_Frontend_CompilerInvocation_cpp 24 Jan 2020 17:37:47 -0000
@@ -18,6 +18,8 @@ essentially gadget free, leaving only th
 jumping into the instruction stream partway through other instructions. Work to
 remove these gadgets will continue through other mechanisms.
 
+Add -msvr4-struct-return for powerpc.
+
 Index: tools/clang/lib/Frontend/CompilerInvocation.cpp
 --- tools/clang/lib/Frontend/CompilerInvocation.cpp.orig
 +++ tools/clang/lib/Frontend/CompilerInvocation.cpp
@@ -30,3 +32,25 @@ Index: tools/clang/lib/Frontend/Compiler
    if (Arg *A = Args.getLastArg(OPT_mstack_probe_size)) {
      StringRef Val = A->getValue();
      unsigned StackProbeSize = Opts.StackProbeSize;
+@@ -1197,11 +1199,18 @@ static bool ParseCodeGenArgs(CodeGenOptions &Opts, Arg
+       Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << Val;
+   }
+
+-  if (Arg *A = Args.getLastArg(OPT_fpcc_struct_return, OPT_freg_struct_return)) {
+-    if (A->getOption().matches(OPT_fpcc_struct_return)) {
++  // X86_32 has -fppc-struct-return and -freg-struct-return.
++  // PPC32 has -maix-struct-return and -msvr4-struct-return.
++  if (Arg *A =
++          Args.getLastArg(OPT_fpcc_struct_return, OPT_freg_struct_return,
++                          OPT_maix_struct_return, OPT_msvr4_struct_return)) {
++    const Option &O = A->getOption();
++    if (O.matches(OPT_fpcc_struct_return) ||
++        O.matches(OPT_maix_struct_return)) {
+       Opts.setStructReturnConvention(CodeGenOptions::SRCK_OnStack);
+     } else {
+-      assert(A->getOption().matches(OPT_freg_struct_return));
++      assert(O.matches(OPT_freg_struct_return) ||
++             O.matches(OPT_msvr4_struct_return));
+       Opts.setStructReturnConvention(CodeGenOptions::SRCK_InRegs);
+     }
+   }

sret-examples.tar.gz (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [macppc] devel/llvm: add clang -msvr4-struct-return

Jeremie Courreges-Anglas-2
On Tue, Jan 28 2020, George Koehler <[hidden email]> wrote:
> Hello ports!

Hi George :)

[...]

I'm happy to see you move on with this diff.  I don't have a powerpc
machine at hand so I'll trust you regarding the correctness of the
resulting object code.  If you want double checks here I'd suggest
talking to kettenis@ (maybe others).

Two things came to my mind regarding your diff:

- on reviews.llvm.org you say this affects OpenBSD, NetBSD and FreeBSD,
  but your code changes the default for anything ELF+non-Linux:

> ++  if (Triple.isOSBinFormatELF() && !Triple.isOSLinux())
> ++    return true;

  Are there only four platforms supported?  Would it be safer to
  explicitely change the default on *BSD?

- I guess you did not choose the name of the options randomly.  Lazy
  question: were SVR4 and AIX the first users of those ABI variants?
  Or is there another reason?

> I'm still waiting for my iMac to build this.  Later, I might try to
> apply this diff to base-clang.  I won't commit this diff to ports-clang
> before I know what to do with base-clang.

Ack.

--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Reply | Threaded
Open this post in threaded view
|

Re: [macppc] devel/llvm: add clang -msvr4-struct-return

Charlene Wendling
In reply to this post by George Koehler-2
Hi :)

On Tue, 28 Jan 2020 22:27:22 -0500
George Koehler wrote:

> Hello ports!
>
> In OpenBSD macppc, clang and gcc use incompatible ABIs to return small
> structs: gcc defaults to -msvr4-struct-return, but clang always acts
> like gcc -maix-struct-return.  This causes crashes at runtime: for
> example, clang code can't call libxcb (in Xenocara built by gcc).
>
> This diff for ports-clang adds clang -msvr4-struct-return, and uses it
> by default for OpenBSD powerpc.  I proposed the diff upstream at
> https://reviews.llvm.org/D73290 (where Mark Millard reported that the
> diff seems to work so far on FreeBSD).
>
> My iMac G3 (1x 400 MHz PowerPC 750, 512 M RAM) with OpenBSD macppc is
> trying to build devel/llvm with this diff, but after 45 hours, has
> built less than half of 4470 targets.  (If I had not broken my
> PowerBook G4, it might have completed the build by now.)

I've finished the build with your diff on my PowerBook G4 A1138, where
it took 29 hours, and so i tested:

> My amd64 desktop built devel/llvm with this diff days ago.  I can now
> cross-compile, where I run 'clang -c' on amd64, scp the *.o to my
> iMac, then use base-gcc to link.
>
> The attachment "sret-examples.tar.gz" contains my tiny example
> programs and a Makefile set to use ports-clang as CLANG and base-gcc
> as GCC.  Run 'make' to build smain-[cg][cg] and xcbtest.  The 4
> smain-* try different combinations of clang and gcc; they should show
> 12 ok tests.  

The 4 smain-* return all ok.

> The xcbtest requires an X server and should show terminal output like,

> $ ./xcbtest                                                          
> conn = 0xb16f3033000
> cookie.sequence = 1
> reply = 0xb173fd873e0

$ DISPLAY=:0 ./xcbtest
conn = 0xcb5ee000
cookie.sequence = 1
reply = 0xaccbbd20

> where conn and reply should not be zero.  (This output is from amd64;
> I can't run xcbtest on my iMac because X11 isn't working.)
>
> On my iMac, 'make CLANG=/usr/bin/clang' uses a base-clang without this
> diff: smain-cg (clang calling gcc) fails most tests and smain-gc (gcc
> calling clang) crashes.

I can confirm that.
 

> To cross-compile the clang parts from amd64, I do
> $ make clang CLANG='/usr/local/bin/clang -target ppc-openbsd'
> and scp the *.o files to my iMac.  This fixes smain-cg and smain-gc.
>
> If you want to build devel/llvm on macppc, I suggest a G4 or G5 with
> at least 1024M of RAM.  (My iMac with 512M RAM took over 600M swap to
> compile ASTDumper; the build seemed to freeze for a few hours.)  If
> you build it, you might try 'make COMPILER=ports-clang' on some
> ports.  If a port so built does crash at runtime, it might be hard to
> know whether a struct return or some other clang bug caused the crash.

I had to bring back games/galois to ports-gcc because it was broken at
startup with ports-clang, so i tried to build it with your clang; it's
playable, there are still segfaults when messing with options, and i
forgot to build it with debug infos on. I could if asked to, but this
is already an improvement!

I'm not knowledgeable enough to comment about the technical aspects of
this diff. This will be my clang until next bulk, so i'll report
back if i see issues.

> I'm still waiting for my iMac to build this.  Later, I might try to
> apply this diff to base-clang.  I won't commit this diff to
> ports-clang before I know what to do with base-clang.
>
> --George
>
> Index: Makefile
> ===================================================================
> RCS file: /cvs/ports/devel/llvm/Makefile,v
> retrieving revision 1.240
> diff -u -p -r1.240 Makefile
> --- Makefile 7 Jan 2020 22:59:43 -0000 1.240
> +++ Makefile 24 Jan 2020 17:37:47 -0000
> @@ -18,7 +18,7 @@ PKGSPEC-main = llvm-=${LLVM_V}
>  PKGNAME-main = llvm-${LLVM_V}
>  PKGNAME-python = py-llvm-${LLVM_V}
>  PKGNAME-lldb = lldb-${LLVM_V}
> -REVISION-main = 5
> +REVISION-main = 6
>  REVISION-lldb= 0
>  
>  CATEGORIES = devel
> Index: patches/patch-tools_clang_include_clang_Driver_Options_td
> ===================================================================
> RCS
> file: /cvs/ports/devel/llvm/patches/patch-tools_clang_include_clang_Driver_Options_td,v
> retrieving revision 1.19 diff -u -p -r1.19
> patch-tools_clang_include_clang_Driver_Options_td
> --- patches/patch-tools_clang_include_clang_Driver_Options_td
> 6 Jul 2019 15:06:36 -0000 1.19 +++
> patches/patch-tools_clang_include_clang_Driver_Options_td 24
> Jan 2020 17:37:47 -0000 @@ -2,6 +2,7 @@ $OpenBSD:
> patch-tools_clang_include_clan
>  - Add ret protctor options as no-ops.
>  - Improve the X86FixupGadgets pass
> +- Add -msvr4-struct-return for powerpc.
>  - Alias the command line parameter -p to -pg.
>  - implement -msave-args in clang/llvm, like the sun did for gcc
>  
> @@ -23,7 +24,20 @@ Index: tools/clang/include/clang/Driver/
>   def fstandalone_debug : Flag<["-"], "fstandalone-debug">,
> Group<f_Group>, Flags<[CoreOption]>, HelpText<"Emit full debug info
> for all types used by the program">; def fno_standalone_debug :
> Flag<["-"], "fno-standalone-debug">, Group<f_Group>,
> Flags<[CoreOption]>, -@@ -2500,7 +2508,7 @@ def pthreads :
> Flag<["-"], "pthreads">; +@@ -2230,6 +2238,12 @@ def mlongcall:
> Flag<["-"], "mlongcall">,
> +     Group<m_ppc_Features_Group>;
> + def mno_longcall : Flag<["-"], "mno-longcall">,
> +     Group<m_ppc_Features_Group>;
> ++def maix_struct_return : Flag<["-"], "maix-struct-return">,
> ++  Group<m_Group>, Flags<[CC1Option]>,
> ++  HelpText<"Return all structs in memory (PPC32 only)">;
> ++def msvr4_struct_return : Flag<["-"], "msvr4-struct-return">,
> ++  Group<m_Group>, Flags<[CC1Option]>,
> ++  HelpText<"Return small structs in registers (PPC32 only)">;
> +
> + def mvx : Flag<["-"], "mvx">, Group<m_Group>;
> + def mno_vx : Flag<["-"], "mno-vx">, Group<m_Group>;
> +@@ -2500,7 +2514,7 @@ def pthreads : Flag<["-"], "pthreads">;
>   def pthread : Flag<["-"], "pthread">, Flags<[CC1Option]>,
>     HelpText<"Support POSIX threads in generated code">;
>   def no_pthread : Flag<["-"], "no-pthread">, Flags<[CC1Option]>;
> @@ -32,7 +46,7 @@ Index: tools/clang/include/clang/Driver/
>   def pie : Flag<["-"], "pie">;
>   def read__only__relocs : Separate<["-"], "read_only_relocs">;
>   def remap : Flag<["-"], "remap">;
> -@@ -2949,6 +2957,8 @@ def mshstk : Flag<["-"], "mshstk">,
> Group<m_x86_Featur +@@ -2949,6 +2963,8 @@ def mshstk : Flag<["-"],
> "mshstk">, Group<m_x86_Featur def mno_shstk : Flag<["-"],
> "mno-shstk">, Group<m_x86_Features_Group>; def
> mretpoline_external_thunk : Flag<["-"], "mretpoline-external-thunk">,
> Group<m_x86_Features_Group>; def mno_retpoline_external_thunk :
> Flag<["-"], "mno-retpoline-external-thunk">,
> Group<m_x86_Features_Group>; Index:
> patches/patch-tools_clang_lib_CodeGen_TargetInfo_cpp
> ===================================================================
> RCS file: patches/patch-tools_clang_lib_CodeGen_TargetInfo_cpp diff
> -N patches/patch-tools_clang_lib_CodeGen_TargetInfo_cpp
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ patches/patch-tools_clang_lib_CodeGen_TargetInfo_cpp 24
> Jan 2020 17:37:47 -0000 @@ -0,0 +1,128 @@
> +$OpenBSD$
> +
> +Add -msvr4-struct-return for powerpc.
> +
> +Index: tools/clang/lib/CodeGen/TargetInfo.cpp
> +--- tools/clang/lib/CodeGen/TargetInfo.cpp.orig
> ++++ tools/clang/lib/CodeGen/TargetInfo.cpp
> +@@ -4092,22 +4092,39 @@ namespace {
> + /// PPC32_SVR4_ABIInfo - The 32-bit PowerPC ELF (SVR4) ABI
> information.
> + class PPC32_SVR4_ABIInfo : public DefaultABIInfo {
> +   bool IsSoftFloatABI;
> ++  bool IsRetSmallStructInRegABI;
> +
> +   CharUnits getParamTypeAlignment(QualType Ty) const;
> +
> + public:
> +-  PPC32_SVR4_ABIInfo(CodeGen::CodeGenTypes &CGT, bool SoftFloatABI)
> +-      : DefaultABIInfo(CGT), IsSoftFloatABI(SoftFloatABI) {}
> ++  PPC32_SVR4_ABIInfo(CodeGen::CodeGenTypes &CGT, bool SoftFloatABI,
> ++                     bool RetSmallStructInRegABI)
> ++      : DefaultABIInfo(CGT), IsSoftFloatABI(SoftFloatABI),
> ++        IsRetSmallStructInRegABI(RetSmallStructInRegABI) {}
> +
> ++  ABIArgInfo classifyReturnType(QualType RetTy) const;
> ++
> ++  void computeInfo(CGFunctionInfo &FI) const override {
> ++    if (!getCXXABI().classifyReturnType(FI))
> ++      FI.getReturnInfo() = classifyReturnType(FI.getReturnType());
> ++    for (auto &I : FI.arguments())
> ++      I.info = classifyArgumentType(I.type);
> ++  }
> ++
> +   Address EmitVAArg(CodeGenFunction &CGF, Address VAListAddr,
> +                     QualType Ty) const override;
> + };
> +
> + class PPC32TargetCodeGenInfo : public TargetCodeGenInfo {
> + public:
> +-  PPC32TargetCodeGenInfo(CodeGenTypes &CGT, bool SoftFloatABI)
> +-      : TargetCodeGenInfo(new PPC32_SVR4_ABIInfo(CGT,
> SoftFloatABI)) {} ++  PPC32TargetCodeGenInfo(CodeGenTypes &CGT, bool
> SoftFloatABI, ++                         bool RetSmallStructInRegABI)
> ++      : TargetCodeGenInfo(new PPC32_SVR4_ABIInfo(CGT, SoftFloatABI,
> ++
> RetSmallStructInRegABI)) {}
> +
> ++  static bool isStructReturnInRegABI(const llvm::Triple &Triple,
> ++                                     const CodeGenOptions &Opts);
> ++
> +   int getDwarfEHStackPointer(CodeGen::CodeGenModule &M) const
> override {
> +     // This is recovered from gcc output.
> +     return 1; // r1 is the dedicated stack pointer
> +@@ -4142,6 +4159,34 @@ CharUnits
> PPC32_SVR4_ABIInfo::getParamTypeAlignment(Qu
> +   return CharUnits::fromQuantity(4);
> + }
> +
> ++ABIArgInfo PPC32_SVR4_ABIInfo::classifyReturnType(QualType RetTy)
> const { ++  uint64_t Size;
> ++
> ++  // -msvr4-struct-return puts small aggregates in GPR3 and GPR4.
> ++  if (isAggregateTypeForABI(RetTy) && IsRetSmallStructInRegABI &&
> ++      (Size = getContext().getTypeSize(RetTy)) <= 64) {
> ++    // System V ABI (1995), page 3-22, specified:
> ++    // > A structure or union whose size is less than or equal to 8
> bytes ++    // > shall be returned in r3 and r4, as if it were first
> stored in the ++    // > 8-byte aligned memory area and then the low
> addressed word were ++    // > loaded into r3 and the high-addressed
> word into r4.  Bits beyond ++    // > the last member of the
> structure or union are not defined. ++    //
> ++    // GCC for big-endian PPC32 inserts the pad before the first
> member, ++    // not "beyond the last member" of the struct.  To stay
> compatible ++    // with GCC, we coerce the struct to an integer of
> the same size. ++    // LLVM will extend it and return i32 in r3, or
> i64 in r3:r4. ++    if (Size == 0)
> ++      return ABIArgInfo::getIgnore();
> ++    else {
> ++      llvm::Type *CoerceTy = llvm::Type::getIntNTy(getVMContext(),
> Size); ++      return ABIArgInfo::getDirect(CoerceTy);
> ++    }
> ++  }
> ++
> ++  return DefaultABIInfo::classifyReturnType(RetTy);
> ++}
> ++
> + // TODO: this implementation is now likely redundant with
> + // DefaultABIInfo::EmitVAArg.
> + Address PPC32_SVR4_ABIInfo::EmitVAArg(CodeGenFunction &CGF, Address
> VAList, +@@ -4299,6 +4344,25 @@ Address
> PPC32_SVR4_ABIInfo::EmitVAArg(CodeGenFunction
> +   return Result;
> + }
> +
> ++bool PPC32TargetCodeGenInfo::isStructReturnInRegABI(
> ++    const llvm::Triple &Triple, const CodeGenOptions &Opts) {
> ++  assert(Triple.getArch() == llvm::Triple::ppc);
> ++
> ++  switch (Opts.getStructReturnConvention()) {
> ++  case CodeGenOptions::SRCK_Default:
> ++    break;
> ++  case CodeGenOptions::SRCK_OnStack: // -maix-struct-return
> ++    return false;
> ++  case CodeGenOptions::SRCK_InRegs: // -msvr4-struct-return
> ++    return true;
> ++  }
> ++
> ++  if (Triple.isOSBinFormatELF() && !Triple.isOSLinux())
> ++    return true;
> ++
> ++  return false;
> ++}
> ++
> + bool
> +
> PPC32TargetCodeGenInfo::initDwarfEHRegSizeTable(CodeGen::CodeGenFunction
> &CGF,
> +                                                 llvm::Value
> *Address) const { +@@ -9330,9 +9394,13 @@ const TargetCodeGenInfo
> &CodeGenModule::getTargetCodeG
> +     return SetCGInfo(new ARMTargetCodeGenInfo(Types, Kind));
> +   }
> +
> +-  case llvm::Triple::ppc:
> ++  case llvm::Triple::ppc: {
> ++    bool RetSmallStructInRegABI =
> ++        PPC32TargetCodeGenInfo::isStructReturnInRegABI(Triple,
> CodeGenOpts);
> +     return SetCGInfo(
> +-        new PPC32TargetCodeGenInfo(Types, CodeGenOpts.FloatABI ==
> "soft")); ++        new PPC32TargetCodeGenInfo(Types,
> CodeGenOpts.FloatABI == "soft", ++
> RetSmallStructInRegABI)); ++  }
> +   case llvm::Triple::ppc64:
> +     if (Triple.isOSBinFormatELF()) {
> +       PPC64_SVR4_ABIInfo::ABIKind Kind = PPC64_SVR4_ABIInfo::ELFv1;
> Index: patches/patch-tools_clang_lib_Driver_ToolChains_Clang_cpp
> ===================================================================
> RCS
> file: /cvs/ports/devel/llvm/patches/patch-tools_clang_lib_Driver_ToolChains_Clang_cpp,v
> retrieving revision 1.11 diff -u -p -r1.11
> patch-tools_clang_lib_Driver_ToolChains_Clang_cpp
> --- patches/patch-tools_clang_lib_Driver_ToolChains_Clang_cpp
> 7 Jan 2020 22:59:43 -0000 1.11 +++
> patches/patch-tools_clang_lib_Driver_ToolChains_Clang_cpp 24
> Jan 2020 17:37:47 -0000 @@ -1,5 +1,6 @@ $OpenBSD:
> patch-tools_clang_lib_Driver_ToolChains_Clang_cpp,v 1.11 2020/01/07
> 22:59:43 jca Exp $ +- Add -msvr4-struct-return for powerpc.
>  - Make LLVM create strict aligned code for OpenBSD/arm64.
>  - Disable -fstrict-aliasing per default on OpenBSD.
>  - Enable -fwrapv by default
> @@ -33,7 +34,27 @@ $OpenBSD: patch-tools_clang_lib_Driver_T
>  Index: tools/clang/lib/Driver/ToolChains/Clang.cpp
>  --- tools/clang/lib/Driver/ToolChains/Clang.cpp.orig
>  +++ tools/clang/lib/Driver/ToolChains/Clang.cpp
> -@@ -3899,9 +3899,12 @@ void Clang::ConstructJob(Compilation &C,
> const JobActi +@@ -3870,6 +3870,19 @@ void
> Clang::ConstructJob(Compilation &C, const JobActi
> +     CmdArgs.push_back(A->getValue());
> +   }
> +
> ++  if (Arg *A = Args.getLastArg(options::OPT_maix_struct_return,
> ++                               options::OPT_msvr4_struct_return)) {
> ++    if (TC.getArch() != llvm::Triple::ppc) {
> ++      D.Diag(diag::err_drv_unsupported_opt_for_target)
> ++          << A->getSpelling() << RawTriple.str();
> ++    } else if
> (A->getOption().matches(options::OPT_maix_struct_return)) { ++
> CmdArgs.push_back("-maix-struct-return"); ++    } else {
> ++
> assert(A->getOption().matches(options::OPT_msvr4_struct_return)); +
> +      CmdArgs.push_back("-msvr4-struct-return"); ++    }
> ++  }
> ++
> +   if (Arg *A = Args.getLastArg(options::OPT_fpcc_struct_return,
> +                                options::OPT_freg_struct_return)) {
> +     if (TC.getArch() != llvm::Triple::x86) {
> +@@ -3899,9 +3912,12 @@ void Clang::ConstructJob(Compilation &C,
> const JobActi OFastEnabled ? options::OPT_Ofast :
> options::OPT_fstrict_aliasing; // We turn strict aliasing off by
> default if we're in CL mode, since MSVC // doesn't do any TBAA.
> @@ -48,7 +69,7 @@ Index: tools/clang/lib/Driver/ToolChains
>       CmdArgs.push_back("-relaxed-aliasing");
>     if (!Args.hasFlag(options::OPT_fstruct_path_tbaa,
>                       options::OPT_fno_struct_path_tbaa))
> -@@ -4527,7 +4530,8 @@ void Clang::ConstructJob(Compilation &C, const
> JobActi +@@ -4527,7 +4543,8 @@ void Clang::ConstructJob(Compilation
> &C, const JobActi options::OPT_fno_strict_overflow)) {
>       if (A->getOption().matches(options::OPT_fno_strict_overflow))
>         CmdArgs.push_back("-fwrapv");
> @@ -58,7 +79,7 @@ Index: tools/clang/lib/Driver/ToolChains
>  
>     if (Arg *A = Args.getLastArg(options::OPT_freroll_loops,
>                                  options::OPT_fno_reroll_loops))
> -@@ -4544,9 +4548,46 @@ void Clang::ConstructJob(Compilation &C,
> const JobActi +@@ -4544,9 +4561,46 @@ void
> Clang::ConstructJob(Compilation &C, const JobActi false))
>       CmdArgs.push_back(Args.MakeArgString("-mspeculative-load-hardening"));
>  
> @@ -106,7 +127,7 @@ Index: tools/clang/lib/Driver/ToolChains
>     // Translate -mstackrealign
>     if (Args.hasFlag(options::OPT_mstackrealign,
> options::OPT_mno_stackrealign, false))
> -@@ -5029,6 +5070,18 @@ void Clang::ConstructJob(Compilation &C,
> const JobActi +@@ -5029,6 +5083,18 @@ void
> Clang::ConstructJob(Compilation &C, const JobActi
> options::OPT_fno_rewrite_imports, false); if (RewriteImports)
>       CmdArgs.push_back("-frewrite-imports");
> Index: patches/patch-tools_clang_lib_Frontend_CompilerInvocation_cpp
> ===================================================================
> RCS
> file: /cvs/ports/devel/llvm/patches/patch-tools_clang_lib_Frontend_CompilerInvocation_cpp,v
> retrieving revision 1.3 diff -u -p -r1.3
> patch-tools_clang_lib_Frontend_CompilerInvocation_cpp
> ---
> patches/patch-tools_clang_lib_Frontend_CompilerInvocation_cpp
> 6 Jul 2019 15:06:36 -0000 1.3 +++
> patches/patch-tools_clang_lib_Frontend_CompilerInvocation_cpp
> 24 Jan 2020 17:37:47 -0000 @@ -18,6 +18,8 @@ essentially gadget free,
> leaving only th jumping into the instruction stream partway through
> other instructions. Work to remove these gadgets will continue
> through other mechanisms. +Add -msvr4-struct-return for powerpc. +
>  Index: tools/clang/lib/Frontend/CompilerInvocation.cpp
>  --- tools/clang/lib/Frontend/CompilerInvocation.cpp.orig
>  +++ tools/clang/lib/Frontend/CompilerInvocation.cpp
> @@ -30,3 +32,25 @@ Index: tools/clang/lib/Frontend/Compiler
>     if (Arg *A = Args.getLastArg(OPT_mstack_probe_size)) {
>       StringRef Val = A->getValue();
>       unsigned StackProbeSize = Opts.StackProbeSize;
> +@@ -1197,11 +1199,18 @@ static bool ParseCodeGenArgs(CodeGenOptions
> &Opts, Arg
> +       Diags.Report(diag::err_drv_invalid_value) <<
> A->getAsString(Args) << Val;
> +   }
> +
> +-  if (Arg *A = Args.getLastArg(OPT_fpcc_struct_return,
> OPT_freg_struct_return)) { +-    if
> (A->getOption().matches(OPT_fpcc_struct_return)) { ++  // X86_32 has
> -fppc-struct-return and -freg-struct-return. ++  // PPC32 has
> -maix-struct-return and -msvr4-struct-return. ++  if (Arg *A =
> ++          Args.getLastArg(OPT_fpcc_struct_return,
> OPT_freg_struct_return, ++
> OPT_maix_struct_return, OPT_msvr4_struct_return)) { ++    const
> Option &O = A->getOption(); ++    if
> (O.matches(OPT_fpcc_struct_return) || ++
> O.matches(OPT_maix_struct_return)) {
> +       Opts.setStructReturnConvention(CodeGenOptions::SRCK_OnStack);
> +     } else {
> +-      assert(A->getOption().matches(OPT_freg_struct_return));
> ++      assert(O.matches(OPT_freg_struct_return) ||
> ++             O.matches(OPT_msvr4_struct_return));
> +       Opts.setStructReturnConvention(CodeGenOptions::SRCK_InRegs);
> +     }
> +   }

Reply | Threaded
Open this post in threaded view
|

Re: [macppc] devel/llvm: add clang -msvr4-struct-return

George Koehler-2
In reply to this post by Jeremie Courreges-Anglas-2
On Thu, 30 Jan 2020 16:52:06 +0100
Jeremie Courreges-Anglas <[hidden email]> wrote:

> Two things came to my mind regarding your diff:
>
> - on reviews.llvm.org you say this affects OpenBSD, NetBSD and FreeBSD,
>   but your code changes the default for anything ELF+non-Linux:
>
> > ++  if (Triple.isOSBinFormatELF() && !Triple.isOSLinux())
> > ++    return true;
>
>   Are there only four platforms supported?  Would it be safer to
>   explicitely change the default on *BSD?

PowerPC has 2 platforms, AIX and Darwin (Mac OS X), not using ELF.
These platforms don't work: LLVM doesn't support Darwin, LLVM 8.0.1
(in OpenBSD ports) mistakingly uses ELF for AIX, and LLVM git master
gives error like, "FramePointer is not implemented on AIX yet."

I recently found an email proposing gcc -msvr4-struct-return:
https://gcc.gnu.org/ml/gcc-patches/2001-11/msg01468.html

The System V ABI PowerPC (1995, elfspec_ppc.pdf) said to return small
structs in r3/r4, but GCC never implemented this until 2001.  Because
this is an ABI change, the email proposed that Linux, NetBSD, FreeBSD
would stay on -maix-struct-return, but all other ELF targets would
switch to -msvr4-struct-return.  (GCC doesn't target PowerPC OpenBSD;
our ports-gcc uses a patch to add the target.)  I believe that NetBSD
and FreeBSD changed ABIs, so Linux is the last odd platform.

> - I guess you did not choose the name of the options randomly.  Lazy
>   question: were SVR4 and AIX the first users of those ABI variants?
>   Or is there another reason?

IBM's AIX went first, and made an ABI for XCOFF.
Apple's Mac OS got the ABI from AIX.

Sun wrote the ELF ABI (1995), probably for Solaris (their distro of
System V Release 4), but mkLinux and NetBSD also used ELF.

--George

Reply | Threaded
Open this post in threaded view
|

Re: [macppc] devel/llvm: add clang -msvr4-struct-return

George Koehler-2
In reply to this post by Charlene Wendling
On Fri, 31 Jan 2020 19:33:52 +0100
Charlene Wendling <[hidden email]> wrote:

> I've finished the build with your diff on my PowerBook G4 A1138, where
> it took 29 hours, and so i tested:

*Thank you!*  I killed my macppc build because 1 C++ file (Parser.cpp)
got stuck for hours trying to use over 800M swap on my iMac G3's
internal hard disk.  (It has 512M RAM.)  I have now restarted the build
after disabling swapencrypt and switching to swap on NFS (trying to
borrow RAM from my amd64 desktop), but I might still be days away from
completing the build.

My now-broken PowerBook G4 (PowerBook5,4; A1095?) took about 28 hours
to build clang 7.0.1, but that was with 1G RAM, and running its MPC7447A
cpu at apm -L half speed (666 MHz, the default setting); I didn't like
the loud fan at full speed (1333 MHz).  Your RAM amount and cpu speed
(if your PowerBook has dfs(4)) may affect your build time.

> $ DISPLAY=:0 ./xcbtest
> conn = 0xcb5ee000
> cookie.sequence = 1
> reply = 0xaccbbd20

This looks good to me.

> I had to bring back games/galois to ports-gcc because it was broken at
> startup with ports-clang, so i tried to build it with your clang; it's
> playable, there are still segfaults when messing with options, and i
> forgot to build it with debug infos on. I could if asked to, but this
> is already an improvement!

If I can finish my ports-clang build, I might try galois on my iMac.
(No X server right now, but I might ssh -X from amd64.)

--George