Hi George, On Tue, Feb 04, 2020 at 08:39:12PM -0500, George Koehler wrote: > Hello tech list, > > This is a diff for base-clang. It would change the powerpc target to > return small structs in registers r3 and r4. This would fix an > incompatibility with gcc in OpenBSD macppc. I fear that if I commit > the diff, I might break snapshot builds. I am looking for help.
The diff looks sane to me. I don't see anything that would obviously break snaps, and the option is limited to only the ppc platform. I am running it through a release build here (amd64) and will know how that turned out tomorrow after work, so will be able to confirm that you won't break snaps on that arch. I don't expect there to be any problem there. I should be able to test it on macppc over the weekend, so if you're paranoid you can wait to see how that goes. Cheers, Todd > > I posted the diff upstream: https://reviews.llvm.org/D73290 > Then backported it to ports-clang and mailed it to ports@: > https://marc.info/?l=openbsd-ports&m=158026852003182&w=2 > This is the same diff for base-clang. > > Apply diff in /usr/src/gnu/llvm, then build clang like > > # cd /usr/src/gnu/usr.bin/clang > # make obj > # make > # make install > > My macppc machine, an iMac G3 with 512M RAM, is too slow to build llvm > and clang. The iMac is now in the 11th day of building ports-clang > (but I turned it off overnight), and within the last 200 of over 4400 > targets. After a .cpp file got stuck while using over 800M swap (on > hard disk), I set vm.swapencrypt.enable=0 and switched to swap on NFS. > (For contrast, my amd64 desktop built ports-clang in about 2 hours.) > > cwen@ built ports-clang with my diff on a G4 in "29 hours". > rgc <rgci...@disroot.org> built base-clang with the diff in this mail. > Both got good results from my small test programs in the attachment > (sret-examples.tar.gz) to my ports@ mail. > > The diff affects all platforms (if they build clang), so if the diff > would break clang, it might break arches other than macppc. I built > and installed base-clang with this diff on amd64 (using make -j4 to > speed up the build). Then, with the patched clang, I built an amd64 > release(8) of base and xenocara. I used my new install66.iso > to install a new amd64 virtual machine, including the patched clang. > > We build most of OpenBSD macppc with base-gcc, but we use base-clang > for at least clang itself. I don't know whether the patched > base-clang can still build base-clang, whether the patch would break > the base build on macppc. > > The patch adds -maix-struct-return and -msvr4-struct-return: > > $ clang -msvr-struct-return -x c /dev/null > clang: error: unknown argument '-msvr-struct-return', did you mean > '-msvr4-struct-return'? > $ clang -msvr4-struct-return -x c /dev/null > clang: error: unsupported option '-msvr4-struct-return' for target > 'amd64-unknown-openbsd6.6' > > On OpenBSD macppc, gcc defaults to -msvr4-struct-return (to return > small structs in registers r3 and r4), but clang without the patch > always acts like gcc -maix-struct-return (to return small structs in > memory through a sret pointer). The patch adds the options and > changes the default to -msvr4-struct-return on OpenBSD (and other > ELF except Linux). The options work only on 32-bit powerpc. > > When the patch returns a struct in registers, it coerces the struct > to an integer; this changes the LLVM IR from clang to not use a sret > pointer. The option handling for -m{aix,svr4}-struct-return is mostly > a copy of that for -f{pcc,reg}-struct-return on i386. > > --George > > Index: tools/clang/include/clang/Driver/Options.td > =================================================================== > RCS file: /cvs/src/gnu/llvm/tools/clang/include/clang/Driver/Options.td,v > retrieving revision 1.11 > diff -u -p -r1.11 Options.td > --- tools/clang/include/clang/Driver/Options.td 23 Jun 2019 22:05:15 > -0000 1.11 > +++ tools/clang/include/clang/Driver/Options.td 30 Jan 2020 01:13:29 > -0000 > @@ -2238,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>; > Index: tools/clang/lib/CodeGen/TargetInfo.cpp > =================================================================== > RCS file: /cvs/src/gnu/llvm/tools/clang/lib/CodeGen/TargetInfo.cpp,v > retrieving revision 1.1.1.7 > diff -u -p -r1.1.1.7 TargetInfo.cpp > --- tools/clang/lib/CodeGen/TargetInfo.cpp 23 Jun 2019 21:37:39 -0000 > 1.1.1.7 > +++ tools/clang/lib/CodeGen/TargetInfo.cpp 30 Jan 2020 01:13:30 -0000 > @@ -4092,12 +4092,24 @@ 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; > @@ -4105,8 +4117,13 @@ public: > > 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. > @@ -4142,6 +4159,34 @@ CharUnits PPC32_SVR4_ABIInfo::getParamTy > 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(Co > 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:: > 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: tools/clang/lib/Driver/ToolChains/Clang.cpp > =================================================================== > RCS file: /cvs/src/gnu/llvm/tools/clang/lib/Driver/ToolChains/Clang.cpp,v > retrieving revision 1.20 > diff -u -p -r1.20 Clang.cpp > --- tools/clang/lib/Driver/ToolChains/Clang.cpp 25 Oct 2019 00:40:56 > -0000 1.20 > +++ tools/clang/lib/Driver/ToolChains/Clang.cpp 30 Jan 2020 01:13:30 > -0000 > @@ -3870,6 +3870,19 @@ void Clang::ConstructJob(Compilation &C, > 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) { > Index: tools/clang/lib/Frontend/CompilerInvocation.cpp > =================================================================== > RCS file: /cvs/src/gnu/llvm/tools/clang/lib/Frontend/CompilerInvocation.cpp,v > retrieving revision 1.4 > diff -u -p -r1.4 CompilerInvocation.cpp > --- tools/clang/lib/Frontend/CompilerInvocation.cpp 23 Jun 2019 22:05:15 > -0000 1.4 > +++ tools/clang/lib/Frontend/CompilerInvocation.cpp 30 Jan 2020 01:13:30 > -0000 > @@ -1199,11 +1199,18 @@ static bool ParseCodeGenArgs(CodeGenOpti > 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); > } > } >
