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 } } } }*/ > + >