3. nov. 2017 20:30 je oseba "Jeff Law" <l...@redhat.com> napisala:
>
> On 11/03/2017 11:38 AM, Uros Bizjak wrote:
>>
>> On Fri, Nov 3, 2017 at 6:13 PM, Jeff Law <l...@redhat.com> wrote:
>>>
>>> On 11/03/2017 04:46 AM, Uros Bizjak wrote:
>>>>
>>>>
>>>> On Fri, Nov 3, 2017 at 11:14 AM, Richard Biener
>>>> <richard.guent...@gmail.com> wrote:
>>>>>
>>>>>
>>>>> On Fri, Nov 3, 2017 at 9:38 AM, Uros Bizjak <ubiz...@gmail.com> wrote:
>>>>>>>
>>>>>>>
>>>>>>>              * config/i386/i386.c (ix86_emit_restore_reg_using_pop):
>>>>>>> Prototype.
>>>>>>>              (ix86_adjust_stack_and_probe_stack_clash): Use a
push/pop
>>>>>>> sequence
>>>>>>>              to probe at the start of a noreturn function.
>>>>>>>
>>>>>>>              * gcc.target/i386/stack-check-12.c: New test
>>>>>>
>>>>>>
>>>>>>
>>>>>> -      emit_stack_probe (plus_constant (Pmode, stack_pointer_rtx,
>>>>>> -       -GET_MODE_SIZE (word_mode)));
>>>>>> +      rtx_insn *insn = emit_insn (gen_push (gen_rtx_REG (word_mode,
>>>>>> 0)));
>>>>>>
>>>>>> Please use AX_REG instead of 0.
>>>>>>
>>>>>> +      RTX_FRAME_RELATED_P (insn) = 1;
>>>>>> +      ix86_emit_restore_reg_using_pop (gen_rtx_REG (word_mode, 0));
>>>>>>
>>>>>> Also here.
>>>>>>
>>>>>>          emit_insn (gen_blockage ());
>>>>>>
>>>>>> BTW: Could we use an unused register here, if available? %eax is used
>>>>>> to pass first argument in regparm functions on 32bit targets.
>>>>>
>>>>>
>>>>>
>>>>> Can you push %[er]sp?  What about partial reg stalls when using other
>>>>> registers (if the last set was a movb to it)?  I guess [er]sp is safe
>>>>> here
>>>>> as was [re]ax due to the ABI?
>>>>
>>>>
>>>>
>>>> That would work, too. I believe, that this won't trigger stack engine
>>>> [1], but since the operation is a bit unusual, let's ask HJ to be
>>>> sure.
>>>>
>>>> [1] https://en.wikipedia.org/wiki/Stack_register#Stack_engine
>>>
>>>
>>> How about %esi in 32 bit mode and %rax in 64 bit mode?  I think that
avoids
>>> hitting the parameter passing registers.
>>
>>
>> That is a good choice. IMO, It warrants a small comment, in the
>> source, why this choice.
>
> Here's a patch implementing that approach.
>
> Bootstrapped and regression tested on x86_64.  Also spot tested the new
test on x86.
>
> OK for the trunk now?
>
> Jeff
>
>             * config/i386/i386.c (ix86_emit_restore_reg_using_pop):
Prototype.
>             (ix86_adjust_stack_and_probe_stack_clash): Use a push/pop
sequence
>             to probe at the start of a noreturn function.
>
>             * gcc.target/i386/stack-check-12.c: New test

OK.

Thanks, Uros.

> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index 25b28a1..1b83755 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -101,6 +101,8 @@ static void ix86_print_operand_address_as (FILE *,
rtx, addr_space_t, bool);
>  static bool ix86_save_reg (unsigned int, bool, bool);
>  static bool ix86_function_naked (const_tree);
>  static bool ix86_notrack_prefixed_insn_p (rtx);
> +static void ix86_emit_restore_reg_using_pop (rtx);
> +
>
>  #ifndef CHECK_STACK_LIMIT
>  #define CHECK_STACK_LIMIT (-1)
> @@ -12124,8 +12126,14 @@ ix86_adjust_stack_and_probe_stack_clash (const
HOST_WIDE_INT size)
>       we just probe when we cross PROBE_INTERVAL.  */
>    if (TREE_THIS_VOLATILE (cfun->decl))
>      {
> -      emit_stack_probe (plus_constant (Pmode, stack_pointer_rtx,
> -                                      -GET_MODE_SIZE (word_mode)));
> +      /* We can safely use any register here since we're just going to
push
> +        its value and immediately pop it back.  But we do try and avoid
> +        argument passing registers so as not to introduce dependencies in
> +        the pipeline.  For 32 bit we use %esi and for 64 bit we use
%rax.  */
> +      rtx dummy_reg = gen_rtx_REG (word_mode, TARGET_64BIT ? AX_REG :
SI_REG);
> +      rtx_insn *insn = emit_insn (gen_push (dummy_reg));
> +      RTX_FRAME_RELATED_P (insn) = 1;
> +      ix86_emit_restore_reg_using_pop (dummy_reg);
>        emit_insn (gen_blockage ());
>      }
>
> diff --git a/gcc/testsuite/gcc.target/i386/stack-check-12.c
b/gcc/testsuite/gcc.target/i386/stack-check-12.c
> new file mode 100644
> index 0000000..cb69bb0
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/stack-check-12.c
> @@ -0,0 +1,19 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fstack-clash-protection -mtune=generic" } */
> +/* { dg-require-effective-target supports_stack_clash_protection } */
> +
> +__attribute__ ((noreturn)) void exit (int);
> +
> +__attribute__ ((noreturn)) void
> +f (void)
> +{
> +  asm volatile ("nop" ::: "edi");
> +  exit (1);
> +}
> +
> +/* { dg-final { scan-assembler-not "or\[ql\]" } } */
> +/* { dg-final { scan-assembler "pushl  %esi" { target ia32 } } } */
> +/* { dg-final { scan-assembler "popl   %esi" { target ia32 } } }*/
> +/* { dg-final { scan-assembler "pushq  %rax" { target { ! ia32 } } } } */
> +/* { dg-final { scan-assembler "popq   %rax" { target { ! ia32 } } } }*/
> +
>

Reply via email to