> On Oct 29, 2020, at 6:09 AM, Richard Sandiford <richard.sandif...@arm.com>
> wrote:
>
> Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
>> +/* Handle a "zero_call_used_regs" attribute; arguments as in
>> + struct attribute_spec.handler. */
>> +
>> +static tree
>> +handle_zero_call_used_regs_attribute (tree *node, tree name, tree args,
>> + int ARG_UNUSED (flags),
>> + bool *no_add_attrs)
>> +{
>> + tree decl = *node;
>> + tree id = TREE_VALUE (args);
>> +
>> + if (TREE_CODE (decl) != FUNCTION_DECL)
>> + {
>> + error_at (DECL_SOURCE_LOCATION (decl),
>> + "%qE attribute applies only to functions", name);
>> + *no_add_attrs = true;
>> + }
>> +
>> + if (TREE_CODE (id) != STRING_CST)
>> + {
>> + error_at (DECL_SOURCE_LOCATION (decl),
>> + "attribute %qE arguments not a string", name);
>
> The existing message for this seems to be:
>
> "%qE argument not a string"
>
> (which seems a bit terse, but hey)
Okay.
>
>> + *no_add_attrs = true;
>> + }
>> +
>> + bool found = false;
>> + for (unsigned int i = 0; zero_call_used_regs_opts[i].name != NULL; ++i)
>> + if (strcmp (TREE_STRING_POINTER (id),
>> + zero_call_used_regs_opts[i].name) == 0)
>> + {
>> + found = true;
>> + break;
>> + }
>> +
>> + if (!found)
>> + {
>> + error_at (DECL_SOURCE_LOCATION (decl),
>> + "unrecognized zero_call_used_regs attribute: %qs",
>> + TREE_STRING_POINTER (id));
>
> The attribute name needs to be quoted, and it would be good if it
> wasn't hard-coded into the string:
>
> error_at (DECL_SOURCE_LOCATION (decl),
> "unrecognized %qE attribute argument %qs", name,
> TREE_STRING_POINTER (id));
Okay.
>
>> @@ -228,6 +228,10 @@ unsigned int flag_sanitize_coverage
>> Variable
>> bool dump_base_name_prefixed = false
>>
>> +; What subset of registers should be zeroed
>
> Think it would be useful to add “ on function return.”.
Okay.
>
>> +Variable
>> +unsigned int flag_zero_call_used_regs
>> +
>> ###
>> Driver
>>
>> diff --git a/gcc/df.h b/gcc/df.h
>> index 8b6ca8c..0f098d7 100644
>> --- a/gcc/df.h
>> +++ b/gcc/df.h
>> @@ -1085,6 +1085,7 @@ extern void df_update_entry_exit_and_calls (void);
>> extern bool df_hard_reg_used_p (unsigned int);
>> extern unsigned int df_hard_reg_used_count (unsigned int);
>> extern bool df_regs_ever_live_p (unsigned int);
>> +extern bool df_epilogue_uses_p (unsigned int);
>> extern void df_set_regs_ever_live (unsigned int, bool);
>> extern void df_compute_regs_ever_live (bool);
>> extern void df_scan_verify (void);
>> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
>> index c9f7299..b011c17 100644
>> --- a/gcc/doc/extend.texi
>> +++ b/gcc/doc/extend.texi
>> @@ -3992,6 +3992,96 @@ performing a link with relocatable output (i.e.@:
>> @code{ld -r}) on them.
>> A declaration to which @code{weakref} is attached and that is associated
>> with a named @code{target} must be @code{static}.
>>
>> +@item zero_call_used_regs ("@var{choice}")
>> +@cindex @code{zero_call_used_regs} function attribute
>> +
>> +The @code{zero_call_used_regs} attribute causes the compiler to zero
>> +a subset of all call-used registers at function return according to
>> +@var{choice}.
>
> Suggest dropping “according to @var{choice}” here, since it's now
> disconnected with the part that talks about what @var{choice} is.
Okay
>
>> +This is used to increase the program security by either mitigating
>
> s/the program security/program security/
Okay
>
>> +Return-Oriented Programming (ROP) or preventing information leak
>
> leakage
>
> (FWIW, I'm not sure “mitigating ROP” is really correct usage, but I don't
> have any better suggestions.)
Do you mean whether “mitigating ROP’ is one of the major purpose of this new
feature?
The initial main motivation of the new feature is for mitigating ROP. And the
reason for only zeroing
argument subset of the register is also for mitigating ROP.
>
>> +through registers.
>> +
>> +A ``call-used'' register is a register whose contents can be changed by
>> +a function call; therefore, a caller cannot assume that the register has
>> +the same contents on return from the function as it had before calling
>> +the function. Such registers are also called ``call-clobbered'',
>> +``caller-saved'', or ``volatile''.
>
> Reading it back, perhaps it would be better to put this whole paragraph
> in a footnote immediately after the first use of “call-used registers”,
> i.e.
>
> …call-used registers@footnote{A ``call-used'' register…}…
>
> It obviously breaks the flow when reading the raw .texi, but I think it
> reads better in the final version.
Okay.
>
>> +In order to satisfy users with different security needs and control the
>> +run-time overhead at the same time, GCC provides a flexible way to choose
>> +the subset of the call-used registers to be zeroed.
>
> Maybe s/GCC/the @var{choice} parameter/.
Okay.
>
>> +
>> +The three basic values of @var{choice} are:
>
> After which, I think this should be part of the previous paragraph.
Don’t understand here, could you explain a little bit more?
>
>> +@itemize @bullet
>> +@item
>> +@samp{skip} doesn't zero any call-used registers.
>> +
>> +@item
>> +@samp{used} only zeros call-used registers that are used in the function.
>> +A ``used'' register is one whose content has been set or referenced in
>> +the function.
>> +
>> +@item
>> +@samp{all} zeros all call-used registers.
>> +@end itemize
>> +
>> +In addition to these three basic choices, it is possible to modify
>> +@samp{used} or @samp{all} as follows:
>> +
>> +@itemize @bullet
>> +@item
>> +Adding @samp{-gpr} restricts the zeroing to general-purpose registers.
>> +
>> +@item
>> +Adding @samp{-arg} restricts the zeroing to registers that can sometimes
>> +be used to pass function arguments. This includes all argument registers
>> +defined by the platform's calling conversion, regardless of whether the
>> +function uses those registers for function arguments or not.
>> +@end itemize
>> +
>> +The modifiers can be used individually or together. If they are used
>> +together, they must appear in the order above.
>> +
>> +The full list of @var{choice}s is therefore:
>> +
>> +@itemize @bullet
>> +@item
>> +@samp{skip} doesn't zero any call-used register.
>> +
>> +@item
>> +@samp{used} only zeros call-used registers that are used in the function.
>> +
>> +@item
>> +@samp{all} zeros all call-used registers.
>> +
>> +@item
>> +@samp{used-arg} only zeros used call-used registers that pass arguments.
>> +
>> +@item
>> +@samp{used-gpr} only zeros used call-used general purpose registers.
>> +
>> +@item
>> +@samp{used-gpr-arg} only zeros used call-used general purpose registers that
>> +pass arguments.
>> +
>> +@item
>> +@samp{all-gpr-arg} zeros all call-used general purpose registers that pass
>> +arguments.
>> +
>> +@item
>> +@samp{all-arg} zeros all call-used registers that pass arguments.
>> +
>> +@item
>> +@samp{all-gpr} zeros all call-used general purpose registers.
>> +@end itemize
>
> By using a table, I meant:
>
> @table @samp
> @item skip
> …
>
> @item used
> …
> etc.
> @end @table
>
> Did you try that and think the output looked odd?
I will try to see what’s the output.
>
> I think the order should be more consistent. E.g. “all” should be listed
> with the other “all” options, and the ordering of foos for “used-foo”
> and “all-foo” should be the same. So maybe:
>
> - skip
> - used
> - used-arg
> - used-gpr
> - used-gpr-arg
> - all
> - all-arg
> - all-gpr
> - all-gpr-arg
Okay
>
>> +Among this list, @samp{used-gpr-arg}, @samp{used-arg}, @samp{all-gpr-arg},
>
> IMO s/Among/Of/ reads slightly better.
Okay.
>
>> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
>> index c049932..c9e3128 100644
>> --- a/gcc/doc/invoke.texi
>> +++ b/gcc/doc/invoke.texi
>> @@ -550,7 +550,7 @@ Objective-C and Objective-C++ Dialects}.
>> -funit-at-a-time -funroll-all-loops -funroll-loops @gol
>> -funsafe-math-optimizations -funswitch-loops @gol
>> -fipa-ra -fvariable-expansion-in-unroller -fvect-cost-model -fvpt @gol
>> --fweb -fwhole-program -fwpa -fuse-linker-plugin @gol
>> +-fweb -fwhole-program -fwpa -fuse-linker-plugin -fzero-call-used-regs
>> @gol
>> --param @var{name}=@var{value}
>> -O -O0 -O1 -O2 -O3 -Os -Ofast -Og}
>>
>> @@ -12550,6 +12550,19 @@ int foo (void)
>>
>> Not all targets support this option.
>>
>> +@item -fzero-call-used-regs=@var{choice}
>> +@opindex fzero-call-used-regs
>> +Zero call-used registers at function return to increase the program
>
> s/the program/program/
okay
>
>> +security by either mitigating Return-Oriented Programming (ROP) or
>> +preventing information leak through registers.
>
> s/leak/leakage/
Okay
>
>> @@ -173,6 +173,9 @@ struct GTY(()) rtl_data {
>> local stack. */
>> unsigned int stack_alignment_estimated;
>>
>> + /* How to zero call-used regsiters for this routine. */
>> + unsigned int zero_call_used_regs;
>> +
>
> Typo: regsiters. But I don't think we need to add this to crtl.
> It's just data that's passed between pass_zero_call_used_regs::execute
> and gen_call_used_regs_seq.
You are right.
Previously, this part of code was put into “gate()”, so adding this to crtl was
needed at that time.
But now it is not needed anymore.
>
>> /* How many NOP insns to place at each function entry by default. */
>> unsigned short patch_area_size;
>>
>> @@ -285,6 +287,24 @@ enum sanitize_code {
>> | SANITIZE_BOUNDS_STRICT
>> };
>>
>> +/* Different settings for zeroing subset of registers. */
>> +namespace zero_regs_code {
>
> Should only be one space after “namespace”. Having “code” in the name
> surprised me, think “flags” would be better.
Okay.
>
>> @@ -5815,6 +5817,101 @@ make_prologue_seq (void)
>> return seq;
>> }
>>
>> +/* Emit a sequence of insns to zero the call-used registers before RET. */
>> +using namespace zero_regs_code;
>
> Making the “using” file-wide is too much, since the file has a lot of code
> unrelated to this feature. It should go in the function body instead.
Okay
>
>> +
>> +static void
>> +gen_call_used_regs_seq (rtx_insn *ret)
>> +{
>> + bool gpr_only = true;
>> + bool used_only = true;
>> + bool arg_only = true;
>> +
>> + /* No need to zero call-used-regs in main (). */
>> + if (MAIN_NAME_P (DECL_NAME (current_function_decl)))
>> + return;
>> +
>> + /* No need to zero call-used-regs if __builtin_eh_return is called
>> + since it isn't a normal function return. */
>> + if (crtl->calls_eh_return)
>> + return;
>> +
>> + /* If gpr_only is true, only zero call-used registers that are
>> + general-purpose registers; if used_only is true, only zero
>> + call-used registers that are used in the current function;
>> + if arg_only is true, only zero call-used registers that pass
>> + parameters defined by the flatform's calling conversion. */
>> +
>> + gpr_only = crtl->zero_call_used_regs & ONLY_GPR;
>> + used_only = crtl->zero_call_used_regs & ONLY_USED;
>> + arg_only = crtl->zero_call_used_regs & ONLY_ARG;
>
> Guess it would be nice to be consistent about which side the “only”
> goes on. FWIW, I don't mind which way: GPR_ONLY etc. would be
> OK with me if you prefer that.
The current names are okay for me.
>
>> +
>> + /* For each of the hard registers, we should zero it if:
>> + 1. it is a call-used register;
>> + and 2. it is not a fixed register;
>> + and 3. it is not live at the return of the routine;
>> + and 4. it is general registor if gpr_only is true;
>> + and 5. it is used in the routine if used_only is true;
>> + and 6. it is a register that passes parameter if arg_only is true. */
>> +
>> + /* First, prepare the data flow information. */
>> + basic_block bb = BLOCK_FOR_INSN (ret);
>> + auto_bitmap live_out;
>> + bitmap_copy (live_out, df_get_live_out (bb));
>> + df_simulate_initialize_backwards (bb, live_out);
>> + df_simulate_one_insn_backwards (bb, ret, live_out);
>> +
>> + HARD_REG_SET selected_hardregs;
>> + CLEAR_HARD_REG_SET (selected_hardregs);
>> + for (unsigned int regno = 0; regno < FIRST_PSEUDO_REGISTER; regno++)
>> + {
>> + if (!crtl->abi->clobbers_full_reg_p (regno))
>> + continue;
>> + if (fixed_regs[regno])
>> + continue;
>> + if (REGNO_REG_SET_P (live_out, regno))
>> + continue;
>> + if (gpr_only
>> + && !TEST_HARD_REG_BIT (reg_class_contents[GENERAL_REGS], regno))
>> + continue;
>> + if (used_only && !df_regs_ever_live_p (regno))
>> + continue;
>> + if (arg_only && !FUNCTION_ARG_REGNO_P (regno))
>> + continue;
>> +
>> + /* Now this is a register that we might want to zero. */
>> + SET_HARD_REG_BIT (selected_hardregs, regno);
>> + }
>> +
>> + if (hard_reg_set_empty_p (selected_hardregs))
>> + return;
>> +
>> + /* Now that we have a hard register set that needs to be zeroed, pass it
>> to
>> + target to generate zeroing sequence. */
>> + HARD_REG_SET zeroed_hardregs;
>> + start_sequence ();
>> + zeroed_hardregs = targetm.calls.zero_call_used_regs (selected_hardregs);
>> + rtx_insn *seq = get_insns ();
>> + end_sequence ();
>> + if (seq)
>> + {
>> + /* Emit the memory blockage and register clobber asm volatile before
>> + the whole sequence. */
>> + start_sequence ();
>> + expand_asm_reg_clobber_mem_blockage (zeroed_hardregs);
>> + rtx_insn *seq_barrier = get_insns ();
>> + end_sequence ();
>> +
>> + emit_insn_before (seq_barrier, ret);
>> + emit_insn_before (seq, ret);
>> +
>> + /* Update the data flow information. */
>> + crtl->must_be_zero_on_return |= zeroed_hardregs;
>> + df_set_bb_dirty (EXIT_BLOCK_PTR_FOR_FN (cfun));
>> + }
>> +}
>> +
>> +
>> /* Return a sequence to be used as the epilogue for the current function,
>> or NULL. */
>>
>> @@ -6486,7 +6583,100 @@ make_pass_thread_prologue_and_epilogue (gcc::context
>> *ctxt)
>> {
>> return new pass_thread_prologue_and_epilogue (ctxt);
>> }
>> -
>>
>> +
>> +namespace {
>> +
>> +const pass_data pass_data_zero_call_used_regs =
>> +{
>> + RTL_PASS, /* type */
>> + "zero_call_used_regs", /* name */
>> + OPTGROUP_NONE, /* optinfo_flags */
>> + TV_NONE, /* tv_id */
>> + 0, /* properties_required */
>> + 0, /* properties_provided */
>> + 0, /* properties_destroyed */
>> + 0, /* todo_flags_start */
>> + 0, /* todo_flags_finish */
>> +};
>> +
>> +class pass_zero_call_used_regs: public rtl_opt_pass
>> +{
>> +public:
>> + pass_zero_call_used_regs (gcc::context *ctxt)
>> + : rtl_opt_pass (pass_data_zero_call_used_regs, ctxt)
>> + {}
>> +
>> + /* opt_pass methods: */
>> + virtual unsigned int execute (function *);
>> +
>> +}; // class pass_zero_call_used_regs
>> +
>> +unsigned int
>> +pass_zero_call_used_regs::execute (function *fun)
>> +{
>> + unsigned int zero_regs_type = UNSET;
>> + unsigned int attr_zero_regs_type = UNSET;
>> +
>> + tree attr_zero_regs
>> + = lookup_attribute ("zero_call_used_regs",
>> + DECL_ATTRIBUTES (fun->decl));
>
> The “= …” line should be indented by only two extra spaces. Although
> in this case it fits on two lines anyway:
>
> tree attr_zero_regs = lookup_attribute ("zero_call_used_regs",
> DECL_ATTRIBUTES (fun->decl));
Okay
>
>> + /* Get the type of zero_call_used_regs from function attribute. */
>
> “from the function attribute” Might be worth adding “We have already
> filtered out invalid attribute values.”, to explain why there's (rightly)
> no failure path.
Okay
>
>> + if (attr_zero_regs)
>> + {
>> + /* The TREE_VALUE of an attribute is a TREE_LIST whose TREE_VALUE
>> + is the attribute argument's value. */
>> + attr_zero_regs = TREE_VALUE (attr_zero_regs);
>> + gcc_assert (TREE_CODE (attr_zero_regs) == TREE_LIST);
>> + attr_zero_regs = TREE_VALUE (attr_zero_regs);
>> + gcc_assert (TREE_CODE (attr_zero_regs) == STRING_CST);
>> +
>> + for (unsigned int i = 0; zero_call_used_regs_opts[i].name != NULL;
>> ++i)
>> + if (strcmp (TREE_STRING_POINTER (attr_zero_regs),
>> + zero_call_used_regs_opts[i].name) == 0)
>> + {
>> + attr_zero_regs_type = zero_call_used_regs_opts[i].flag;
>> + break;
>> + }
>> + }
>> +
>> + zero_regs_type = attr_zero_regs_type;
>> +
>> + if (!zero_regs_type)
>> + zero_regs_type = flag_zero_call_used_regs;
>
> Having two variables seems unnecessarily complicated. I think the
> attribute code should assign directly to “zero_regs_type”.
You are right.
>
>> +
>> + crtl->zero_call_used_regs = zero_regs_type;
>> +
>> + /* No need to zero call-used-regs when no user request is present. */
>> + if (!(zero_regs_type & ENABLED))
>> + return 0;
>> +
>> + edge_iterator ei;
>> + edge e;
>> +
>> + /* This pass needs data flow information. */
>> + df_analyze ();
>> +
>> + /* Iterate over the function's return instructions and insert any
>> + register zeroing required by the -fzero-call-used-regs command-line
>> + option or the "zero_call_used_regs" function attribute. */
>> + FOR_EACH_EDGE (e, ei, EXIT_BLOCK_PTR_FOR_FN (cfun)->preds)
>> + {
>> + rtx_insn *insn = BB_END (e->src);
>> + if (JUMP_P (insn) && ANY_RETURN_P (JUMP_LABEL (insn)))
>> + gen_call_used_regs_seq (insn);
>
> As noted above, we could just pass “zero_regs_type” here rather than
> store it in crtl.
Okay.
>
>> @@ -987,6 +990,35 @@ default_function_value_regno_p (const unsigned int
>> regno ATTRIBUTE_UNUSED)
>> #endif
>> }
>>
>> +/* The default hook for TARGET_ZERO_CALL_USED_REGS. */
>> +
>> +HARD_REG_SET
>> +default_zero_call_used_regs (HARD_REG_SET need_zeroed_hardregs)
>> +{
>> + gcc_assert (!hard_reg_set_empty_p (need_zeroed_hardregs));
>> +
>> + for (unsigned int regno = 0; regno < FIRST_PSEUDO_REGISTER; regno++)
>> + if (TEST_HARD_REG_BIT (need_zeroed_hardregs, regno))
>> + {
>> + rtx_insn *last_insn = get_last_insn ();
>> + machine_mode mode = GET_MODE (regno_reg_rtx[regno]);
>> + rtx zero = CONST0_RTX (mode);
>> + rtx_insn *insn = emit_move_insn (regno_reg_rtx[regno], zero);
>> + if (!valid_insn_p (insn))
>> + {
>> + static bool issued_error;
>> + if (!issued_error)
>> + {
>> + issued_error = true;
>> + sorry ("%qs not supported on this target",
>> + "fzero-call-used_regs");
>
> "-fzero-call-used_regs”
Okay
>
>> + }
>> + delete_insns_since (last_insn);
>> + }
>> + }
>> + return need_zeroed_hardregs;
>> +}
>> +
>> rtx
>> default_internal_arg_pointer (void)
>> {
>> diff --git a/gcc/targhooks.h b/gcc/targhooks.h
>> index 44ab926..e0a925f 100644
>> --- a/gcc/targhooks.h
>> +++ b/gcc/targhooks.h
>> @@ -160,6 +160,7 @@ extern unsigned int default_function_arg_round_boundary
>> (machine_mode,
>> const_tree);
>> extern bool hook_bool_const_rtx_commutative_p (const_rtx, int);
>> extern rtx default_function_value (const_tree, const_tree, bool);
>> +extern HARD_REG_SET default_zero_call_used_regs (HARD_REG_SET);
>> extern rtx default_libcall_value (machine_mode, const_rtx);
>> extern bool default_function_value_regno_p (const unsigned int);
>> extern rtx default_internal_arg_pointer (void);
>> diff --git a/gcc/testsuite/c-c++-common/zero-scratch-regs-1.c
>> b/gcc/testsuite/c-c++-common/zero-scratch-regs-1.c
>> new file mode 100644
>> index 0000000..2463353
>> --- /dev/null
>> +++ b/gcc/testsuite/c-c++-common/zero-scratch-regs-1.c
>> @@ -0,0 +1,15 @@
>> +/* { dg-do run } */
>> +/* { dg-options "-O2 -fzero-call-used-regs=skip" } */
>> +
>> +volatile int result = 0;
>> +int
>> +__attribute__((noipa))
>> +foo (int x)
>> +{
>> + return x;
>> +}
>> +int main()
>> +{
>> + result = foo (2);
>> + return 0;
>> +}
>> diff --git a/gcc/testsuite/c-c++-common/zero-scratch-regs-10.c
>> b/gcc/testsuite/c-c++-common/zero-scratch-regs-10.c
>> new file mode 100644
>> index 0000000..bdaf8e7
>> --- /dev/null
>> +++ b/gcc/testsuite/c-c++-common/zero-scratch-regs-10.c
>> @@ -0,0 +1,92 @@
>> +/* { dg-do run } */
>> +/* { dg-options "-O2" } */
>> +
>> +#include <assert.h>
>> +int result = 0;
>> +
>> +int
>> +__attribute__((noipa))
>> +__attribute__ ((zero_call_used_regs("skip")))
>> +foo1 (int x)
>> +{
>> + return (x + 1);
>> +}
>> +
>> +int
>> +__attribute__((noipa))
>> +__attribute__ ((zero_call_used_regs("used-gpr-arg")))
>> +foo2 (int x)
>> +{
>> + return (x + 2);
>> +}
>> +
>> +int
>> +__attribute__((noipa))
>> +__attribute__ ((zero_call_used_regs("used-gpr")))
>> +foo3 (int x)
>> +{
>> + return (x + 3);
>> +}
>> +
>> +int
>> +__attribute__((noipa))
>> +__attribute__ ((zero_call_used_regs("used-arg")))
>> +foo4 (int x)
>> +{
>> + return (x + 4);
>> +}
>> +
>> +int
>> +__attribute__((noipa))
>> +__attribute__ ((zero_call_used_regs("used")))
>> +foo5 (int x)
>> +{
>> + return (x + 5);
>> +}
>> +
>> +int
>> +__attribute__((noipa))
>> +__attribute__ ((zero_call_used_regs("all-gpr-arg")))
>> +foo6 (int x)
>> +{
>> + return (x + 6);
>> +}
>> +
>> +int
>> +__attribute__((noipa))
>> +__attribute__ ((zero_call_used_regs("all-gpr")))
>> +foo7 (int x)
>> +{
>> + return (x + 7);
>> +}
>> +
>> +int
>> +__attribute__((noipa))
>> +__attribute__ ((zero_call_used_regs("all-arg")))
>> +foo8 (int x)
>> +{
>> + return (x + 8);
>> +}
>> +
>> +int
>> +__attribute__((noipa))
>> +__attribute__ ((zero_call_used_regs("all")))
>> +foo9 (int x)
>> +{
>> + return (x + 9);
>> +}
>> +
>> +int main()
>> +{
>> + result = foo1 (1);
>> + result += foo2 (1);
>> + result += foo3 (1);
>> + result += foo4 (1);
>> + result += foo5 (1);
>> + result += foo6 (1);
>> + result += foo7 (1);
>> + result += foo8 (1);
>> + result += foo9 (1);
>> + assert (result == 54);
>> + return 0;
>> +}
>> diff --git a/gcc/testsuite/c-c++-common/zero-scratch-regs-11.c
>> b/gcc/testsuite/c-c++-common/zero-scratch-regs-11.c
>> new file mode 100644
>> index 0000000..6756f57
>> --- /dev/null
>> +++ b/gcc/testsuite/c-c++-common/zero-scratch-regs-11.c
>> @@ -0,0 +1,92 @@
>> +/* { dg-do run } */
>> +/* { dg-options "-O2 -fzero-call-used-regs=all" } */
>> +
>> +#include <assert.h>
>> +int result = 0;
>> …
>
> I think this should just #include "zero-scratch-regs-10.c”.
Okay
>
>> diff --git a/gcc/testsuite/c-c++-common/zero-scratch-regs-2.c
>> b/gcc/testsuite/c-c++-common/zero-scratch-regs-2.c
>> new file mode 100644
>> index 0000000..73c3794
>> --- /dev/null
>> +++ b/gcc/testsuite/c-c++-common/zero-scratch-regs-2.c
>> @@ -0,0 +1,15 @@
>> +/* { dg-do run } */
>> +/* { dg-options "-O2 -fzero-call-used-regs=used-gpr-arg" } */
>> +
>> +volatile int result = 0;
>> +int
>> +__attribute__((noipa))
>> +foo (int x)
>> +{
>> + return x;
>> +}
>> +int main()
>> +{
>> + result = foo (2);
>> + return 0;
>> +}
>
> Similarly these can just #include "zero-scratch-regs-1.c". This makes
> it easier to update the tests in a consistent way in future (if necessary).
Okay, thanks for the suggestion.
>
>> diff --git a/gcc/testsuite/c-c++-common/zero-scratch-regs-attr-usages.c
>> b/gcc/testsuite/c-c++-common/zero-scratch-regs-attr-usages.c
>> new file mode 100644
>> index 0000000..c60e946
>> --- /dev/null
>> +++ b/gcc/testsuite/c-c++-common/zero-scratch-regs-attr-usages.c
>> @@ -0,0 +1,10 @@
>> +/* { dg-do compile} */
>> +/* { dg-options "-O2" } */
>> +
>> +int result __attribute__ ((zero_call_used_regs("all"))); /* { dg-error
>> "attribute applies only to functions" } */
>> +int
>> +__attribute__ ((zero_call_used_regs("gpr-arg-all")))
>> +foo1 (int x) /* { dg-error "unrecognized zero_call_used_regs attribute" } */
>> +{
>> + return (x + 1);
>> +}
>
> Could you also add a test for the string constant check? E.g. with
> __attribute__ ((zero_call_used_regs(1))).
Okay.
Thanks.
Qing
>
> Thanks,
> Richard
