[PATCH] pr13012: Support -fpcc-struct-return for x86-32

[John McCall]

On Jun 7, 2013, at 12:07 PM, Arthur O'Dwyer <arthur.j.odwyer at gmail.com> wrote:
> On Wed, Jun 5, 2013 at 1:23 PM, Arthur O'Dwyer
> <arthur.j.odwyer at gmail.com> wrote:
>> 
>>  This is my first post to cfe-commits, so my apologies if I get
>> something wrong.
>> A while back I filed an issue on Bugzilla requesting support for
>> -fpcc-struct-return:
>> http://llvm.org/bugs/show_bug.cgi?id=13012
>> 
>> Recently I wrote a real patch [...] I believe I don't have the necessary
>> credentials to commit the patch anyway, so basically I'm looking for
>> a patron to adopt this patch. :)
>> 
>> To observe what the patch does:
>>  echo "struct S {int i;} foo() {return (struct S){42};}" >foo.c
>>  clang -m32 -O3 -fomit-frame-pointer foo.c -S -o reg.s
>>  clang -m32 -O3 -fomit-frame-pointer foo.c -S -o pcc.s -fpcc-struct-return
>>  diff reg.s pcc.s
> 
> Ping? Any takers? Any comments at all?
> 
> -Arthur
> 
> 
> Index: include/clang/Frontend/CodeGenOptions.def
> ===================================================================
> --- include/clang/Frontend/CodeGenOptions.def (revision 183239)
> +++ include/clang/Frontend/CodeGenOptions.def (working copy)
> @@ -85,6 +85,7 @@
>                                         ///< enabled.
> VALUE_CODEGENOPT(OptimizationLevel, 3, 0) ///< The -O[0-4] option specified.
> VALUE_CODEGENOPT(OptimizeSize, 2, 0) ///< If -Os (==1) or -Oz (==2)
> is specified.
> +VALUE_CODEGENOPT(PassSmallStructsConvention, 2, 0) /// If
> -fpcc-struct-return (==1) or -freg-struct-return (==2) is specified.

Use ENUM_CODEGENOPT instead of magic numbers, please.

> +def fpcc_struct_return : Flag<["-"], "fpcc-struct-return">,
> Group<f_Group>, Flags<[CC1Option]>,
> +  HelpText<"Return small structs on the stack">;

"Override the default ABI to force all structs to be returned on the stack"

> def fpch_preprocess : Flag<["-"], "fpch-preprocess">, Group<f_Group>;
> def fpic : Flag<["-"], "fpic">, Group<f_Group>;
> def fno_pic : Flag<["-"], "fno-pic">, Group<f_Group>;
> @@ -668,6 +670,8 @@
> def fprofile_generate : Flag<["-"], "fprofile-generate">, Group<f_Group>;
> def framework : Separate<["-"], "framework">, Flags<[LinkerInput]>;
> def frandom_seed_EQ : Joined<["-"], "frandom-seed=">,
> Group<clang_ignored_f_Group>;
> +def freg_struct_return : Flag<["-"], "freg-struct-return">,
> Group<f_Group>, Flags<[CC1Option]>,
> +  HelpText<"Return small structs in registers">;

"Override the default ABI to encourage small structs to be returned in registers"

> Index: lib/Driver/Tools.cpp
> ===================================================================
> --- lib/Driver/Tools.cpp (revision 183239)
> +++ lib/Driver/Tools.cpp (working copy)
> @@ -2160,6 +2160,15 @@
>     CmdArgs.push_back(A->getValue());
>   }
> 
> +  if (Arg *A = Args.getLastArg(options::OPT_fpcc_struct_return,
> options::OPT_freg_struct_return)) {
> +    if (A->getOption().matches(options::OPT_fpcc_struct_return)) {
> +      CmdArgs.push_back("-fpcc-struct-return");
> +    } else {
> +      assert(A->getOption().matches(options::OPT_freg_struct_return));
> +      CmdArgs.push_back("-freg-struct-return");
> +    }
> +  }
> +

Please error out if the target is not i386, since you've only added
support for the switch on that target.

> Index: lib/CodeGen/TargetInfo.cpp
> ===================================================================
> --- lib/CodeGen/TargetInfo.cpp (revision 183239)
> +++ lib/CodeGen/TargetInfo.cpp (working copy)
> @@ -4402,7 +4402,26 @@
>   return ResAddr;
> }
> 
> +static bool shouldPassSmallStructsInRegsByDefault(const llvm::Triple &Triple) {

Please make this a static method of X86_32TargetCodeGenInfo, rename it to
isStructReturnInRegABI, and pass it the CodeGenOpts so that it can be
completely in charge of the decision.

John.