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);
> + }
> + }
No comments:
Post a Comment