diff mbox

x86 interrupt attribute

Message ID CAMe9rOrY_p40DRhb4XQvw2J6PEP87mN+6JELkvkawojMUzZtSQ@mail.gmail.com
State New
Headers show

Commit Message

H.J. Lu Oct. 4, 2015, 11:17 p.m. UTC 
On Sun, Oct 4, 2015 at 2:07 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Sun, Oct 4, 2015 at 10:51 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> On Sun, Oct 4, 2015 at 1:00 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>>> On Sun, Oct 4, 2015 at 8:15 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>>
>>>>> Looking a bit deeper into the code, it looks that we want to realign
>>>>> the stack in the interrupt handler. Let's assume that interrupt
>>>>> handler is calling some other function that saves SSE vector regs to
>>>>> the stack. According to the x86 ABI, incoming stack of the called
>>>>> function is assumed to be aligned to 16 bytes. But, interrupt handler
>>>>> violates this assumption, since the stack could be aligned to only 4
>>>>> bytes for 32bit and 8 bytes for 64bit targets. Entering the called
>>>>> function with stack, aligned to less than 16 bytes will certainly
>>>>> violate ABI.
>>>>>
>>>>> So, it looks to me that we need to realign the stack in the interrupt
>>>>> handler unconditionally to 16bytes. In this case, we also won't need
>>>>> the following changes:
>>>>>
>>>>
>>>> Current stack alignment implementation requires at least
>>>> one, maybe two, scratch registers:
>>>>
>>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67841
>>>>
>>>> Extend it to the interrupt handler, which doesn't have any scratch
>>>> registers may require significant changes in backend as well as
>>>> register allocator.
>>>
>>> But without realignment, the handler is unusable for anything but
>>> simple functions. The handler will crash when called function will try
>>> to save vector reg to stack.
>>>
>>
>> We can use unaligned load and store to avoid crash.
>
> Oh, sorry, I meant "called function will crash", like:
>
> -> interrupt when %rsp = 0x...8 ->
> -> interrupt handler ->
> -> calls some function that tries to save xmm reg to stack
> -> crash in the called function
>

It should be fixed by this patch.   But we need to fix stack
alignment in interrupt handler to avoid scratch register.

Comments

Uros Bizjak Oct. 5, 2015, 9:29 a.m. UTC | #1 
On Mon, Oct 5, 2015 at 1:17 AM, H.J. Lu <hjl.tools@gmail.com> wrote:

>>>>>> Looking a bit deeper into the code, it looks that we want to realign
>>>>>> the stack in the interrupt handler. Let's assume that interrupt
>>>>>> handler is calling some other function that saves SSE vector regs to
>>>>>> the stack. According to the x86 ABI, incoming stack of the called
>>>>>> function is assumed to be aligned to 16 bytes. But, interrupt handler
>>>>>> violates this assumption, since the stack could be aligned to only 4
>>>>>> bytes for 32bit and 8 bytes for 64bit targets. Entering the called
>>>>>> function with stack, aligned to less than 16 bytes will certainly
>>>>>> violate ABI.
>>>>>>
>>>>>> So, it looks to me that we need to realign the stack in the interrupt
>>>>>> handler unconditionally to 16bytes. In this case, we also won't need
>>>>>> the following changes:
>>>>>>
>>>>>
>>>>> Current stack alignment implementation requires at least
>>>>> one, maybe two, scratch registers:
>>>>>
>>>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67841
>>>>>
>>>>> Extend it to the interrupt handler, which doesn't have any scratch
>>>>> registers may require significant changes in backend as well as
>>>>> register allocator.
>>>>
>>>> But without realignment, the handler is unusable for anything but
>>>> simple functions. The handler will crash when called function will try
>>>> to save vector reg to stack.
>>>>
>>>
>>> We can use unaligned load and store to avoid crash.
>>
>> Oh, sorry, I meant "called function will crash", like:
>>
>> -> interrupt when %rsp = 0x...8 ->
>> -> interrupt handler ->
>> -> calls some function that tries to save xmm reg to stack
>> -> crash in the called function
>>
>
> It should be fixed by this patch.   But we need to fix stack
> alignment in interrupt handler to avoid scratch register.
>
>
> --
> H.J.
> ---
> commit 15f48be1dc7ff48207927d0b835e593d058f695b
> Author: H.J. Lu <hjl.tools@gmail.com>
> Date:   Sun Oct 4 16:14:03 2015 -0700
>
>     Correctly set incoming stack boundary for interrupt handler
>
> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> index 7ebdcd9..0f0cc3c 100644
> --- a/gcc/config/i386/i386.c
> +++ b/gcc/config/i386/i386.c
> @@ -12037,8 +12037,11 @@ ix86_minimum_incoming_stack_boundary (bool sibcall)
>  {
>    unsigned int incoming_stack_boundary;
>
> +  /* Stack of interrupt handler is always aligned to word_mode.  */
> +  if (cfun->machine->func_type != TYPE_NORMAL)
> +    incoming_stack_boundary = TARGET_64BIT ? 64 : 32;

Just a heads up that in order to support stack realignmnent on x86_64,
MIN_STACK_BOUNDARY will soon be changed to BITS_PER_WORD, so you can
use it in the line above. Please see comment #5 and #6 of PR 66697
[1].

>    /* Prefer the one specified at command line. */
> -  if (ix86_user_incoming_stack_boundary)
> +  else if (ix86_user_incoming_stack_boundary)
>      incoming_stack_boundary = ix86_user_incoming_stack_boundary;
>    /* In 32bit, use MIN_STACK_BOUNDARY for incoming stack boundary
>       if -mstackrealign is used, it isn't used for sibcall check and

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66697

Uros.
Yulia Koval Oct. 13, 2015, 12:18 p.m. UTC | #2 
Here is the current version of the patch with all the fixes.
Regtested\bootstraped it on 64 bit.

We need a pointer since interrupt handler will update data pointing
to by frame.  Since error_code isn't at the normal location where the
parameter is passed on stack and frame isn't in a hard register, we
changed ix86_function_arg:

+  if (!cum->caller && cfun->machine->func_type != TYPE_NORMAL)
+    {
+      /* The first argument of interrupt handler is a pointer and
+        points to the return address slot on stack.  The optional
+        second argument is an integer for error code on stack.  */
+      gcc_assert (type != NULL_TREE);
+      if (POINTER_TYPE_P (type))
+       {
+         if (cfun->machine->func_type == TYPE_EXCEPTION)
+           /* (AP) in the current frame in exception handler.  */
+           arg = arg_pointer_rtx;
+         else
+           /* -WORD(AP) in the current frame in interrupt handler.  */
+           arg = force_reg (Pmode,
+                            plus_constant (Pmode, arg_pointer_rtx,
+                                           -UNITS_PER_WORD));
+         if (mode != Pmode)
+           arg = convert_to_mode (mode, arg, 1);
+       }
+      else
+       {
+         gcc_assert (TREE_CODE (type) == INTEGER_TYPE
+                     && cfun->machine->func_type == TYPE_EXCEPTION
+                     && mode == word_mode);
+         /* The error code is at -WORD(AP) in the current frame in
+            exception handler.  */
+         arg = gen_rtx_MEM (word_mode,
+                            plus_constant (Pmode, arg_pointer_rtx,
+                                           -UNITS_PER_WORD));
+       }
+
+      return arg;
+    }
+

to return a pseudo register.  It violates

   Return where to put the arguments to a function.
   Return zero to push the argument on the stack, or a hard register in
   which to store the argument.

Register allocator has no problem with parameters in pseudo registers.
But GCC crashes when it tries to access DECL_INCOMING_RTL as a hard
register when generating debug information.  We worked around it by
doing

+
+  if (cfun->machine->func_type != TYPE_NORMAL)
+    {
+      /* Since the pointer argument of interrupt handler isn't a real
+         argument, adjust DECL_INCOMING_RTL for debug output.  */
+      tree arg = DECL_ARGUMENTS (current_function_decl);
+      gcc_assert (arg != NULL_TREE
+                 && POINTER_TYPE_P (TREE_TYPE (arg)));
+      if (cfun->machine->func_type == TYPE_EXCEPTION)
+       /* (AP) in the current frame in exception handler.  */
+       DECL_INCOMING_RTL (arg) = arg_pointer_rtx;
+      else
+       /* -WORD(AP) in the current frame in interrupt handler.  */
+       DECL_INCOMING_RTL (arg) = plus_constant (Pmode,
+                                                arg_pointer_rtx,
+                                                -UNITS_PER_WORD);
+    }


On Mon, Oct 5, 2015 at 12:29 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Mon, Oct 5, 2015 at 1:17 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>
>>>>>>> Looking a bit deeper into the code, it looks that we want to realign
>>>>>>> the stack in the interrupt handler. Let's assume that interrupt
>>>>>>> handler is calling some other function that saves SSE vector regs to
>>>>>>> the stack. According to the x86 ABI, incoming stack of the called
>>>>>>> function is assumed to be aligned to 16 bytes. But, interrupt handler
>>>>>>> violates this assumption, since the stack could be aligned to only 4
>>>>>>> bytes for 32bit and 8 bytes for 64bit targets. Entering the called
>>>>>>> function with stack, aligned to less than 16 bytes will certainly
>>>>>>> violate ABI.
>>>>>>>
>>>>>>> So, it looks to me that we need to realign the stack in the interrupt
>>>>>>> handler unconditionally to 16bytes. In this case, we also won't need
>>>>>>> the following changes:
>>>>>>>
>>>>>>
>>>>>> Current stack alignment implementation requires at least
>>>>>> one, maybe two, scratch registers:
>>>>>>
>>>>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67841
>>>>>>
>>>>>> Extend it to the interrupt handler, which doesn't have any scratch
>>>>>> registers may require significant changes in backend as well as
>>>>>> register allocator.
>>>>>
>>>>> But without realignment, the handler is unusable for anything but
>>>>> simple functions. The handler will crash when called function will try
>>>>> to save vector reg to stack.
>>>>>
>>>>
>>>> We can use unaligned load and store to avoid crash.
>>>
>>> Oh, sorry, I meant "called function will crash", like:
>>>
>>> -> interrupt when %rsp = 0x...8 ->
>>> -> interrupt handler ->
>>> -> calls some function that tries to save xmm reg to stack
>>> -> crash in the called function
>>>
>>
>> It should be fixed by this patch.   But we need to fix stack
>> alignment in interrupt handler to avoid scratch register.
>>
>>
>> --
>> H.J.
>> ---
>> commit 15f48be1dc7ff48207927d0b835e593d058f695b
>> Author: H.J. Lu <hjl.tools@gmail.com>
>> Date:   Sun Oct 4 16:14:03 2015 -0700
>>
>>     Correctly set incoming stack boundary for interrupt handler
>>
>> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
>> index 7ebdcd9..0f0cc3c 100644
>> --- a/gcc/config/i386/i386.c
>> +++ b/gcc/config/i386/i386.c
>> @@ -12037,8 +12037,11 @@ ix86_minimum_incoming_stack_boundary (bool sibcall)
>>  {
>>    unsigned int incoming_stack_boundary;
>>
>> +  /* Stack of interrupt handler is always aligned to word_mode.  */
>> +  if (cfun->machine->func_type != TYPE_NORMAL)
>> +    incoming_stack_boundary = TARGET_64BIT ? 64 : 32;
>
> Just a heads up that in order to support stack realignmnent on x86_64,
> MIN_STACK_BOUNDARY will soon be changed to BITS_PER_WORD, so you can
> use it in the line above. Please see comment #5 and #6 of PR 66697
> [1].
>
>>    /* Prefer the one specified at command line. */
>> -  if (ix86_user_incoming_stack_boundary)
>> +  else if (ix86_user_incoming_stack_boundary)
>>      incoming_stack_boundary = ix86_user_incoming_stack_boundary;
>>    /* In 32bit, use MIN_STACK_BOUNDARY for incoming stack boundary
>>       if -mstackrealign is used, it isn't used for sibcall check and
>
> [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66697
>
> Uros.
Yulia Koval Oct. 20, 2015, 1:21 p.m. UTC | #3 
The debug_info section for the interrupt function looks ok.

I tried to call it from assembler code to check it in gdb.

        pushl   $0x333 ;eflags
        pushl   $0x111 ;cs
        pushl $0x222   ;eip
        jmp  foo           ;interrupt function

#define uword_t unsigned int
struct interrupt_frame
{
     uword_t ip;
     uword_t cs;
     uword_t flags;
};

I get this inside the interrupt function:

Breakpoint 1, foo (frame=0xffffd560) at interrupt-1.c:7
7               a = (struct interrupt_frame*) frame;
(gdb) p/x ((struct interrupt_frame*)frame)->ip
$1 = 0x222
(gdb) p/x ((struct interrupt_frame*)frame)->cs
$3 = 0x111
(gdb) p/x ((struct interrupt_frame*)frame)->flags
$4 = 0x333

Frame pointer info looks ok.


On Tue, Oct 13, 2015 at 3:18 PM, Yulia Koval <vaalfreja@gmail.com> wrote:
>
> Here is the current version of the patch with all the fixes.
> Regtested\bootstraped it on 64 bit.
>
> We need a pointer since interrupt handler will update data pointing
> to by frame.  Since error_code isn't at the normal location where the
> parameter is passed on stack and frame isn't in a hard register, we
> changed ix86_function_arg:
>
> +  if (!cum->caller && cfun->machine->func_type != TYPE_NORMAL)
> +    {
> +      /* The first argument of interrupt handler is a pointer and
> +        points to the return address slot on stack.  The optional
> +        second argument is an integer for error code on stack.  */
> +      gcc_assert (type != NULL_TREE);
> +      if (POINTER_TYPE_P (type))
> +       {
> +         if (cfun->machine->func_type == TYPE_EXCEPTION)
> +           /* (AP) in the current frame in exception handler.  */
> +           arg = arg_pointer_rtx;
> +         else
> +           /* -WORD(AP) in the current frame in interrupt handler.  */
> +           arg = force_reg (Pmode,
> +                            plus_constant (Pmode, arg_pointer_rtx,
> +                                           -UNITS_PER_WORD));
> +         if (mode != Pmode)
> +           arg = convert_to_mode (mode, arg, 1);
> +       }
> +      else
> +       {
> +         gcc_assert (TREE_CODE (type) == INTEGER_TYPE
> +                     && cfun->machine->func_type == TYPE_EXCEPTION
> +                     && mode == word_mode);
> +         /* The error code is at -WORD(AP) in the current frame in
> +            exception handler.  */
> +         arg = gen_rtx_MEM (word_mode,
> +                            plus_constant (Pmode, arg_pointer_rtx,
> +                                           -UNITS_PER_WORD));
> +       }
> +
> +      return arg;
> +    }
> +
>
> to return a pseudo register.  It violates
>
>    Return where to put the arguments to a function.
>    Return zero to push the argument on the stack, or a hard register in
>    which to store the argument.
>
> Register allocator has no problem with parameters in pseudo registers.
> But GCC crashes when it tries to access DECL_INCOMING_RTL as a hard
> register when generating debug information.  We worked around it by
> doing
>
> +
> +  if (cfun->machine->func_type != TYPE_NORMAL)
> +    {
> +      /* Since the pointer argument of interrupt handler isn't a real
> +         argument, adjust DECL_INCOMING_RTL for debug output.  */
> +      tree arg = DECL_ARGUMENTS (current_function_decl);
> +      gcc_assert (arg != NULL_TREE
> +                 && POINTER_TYPE_P (TREE_TYPE (arg)));
> +      if (cfun->machine->func_type == TYPE_EXCEPTION)
> +       /* (AP) in the current frame in exception handler.  */
> +       DECL_INCOMING_RTL (arg) = arg_pointer_rtx;
> +      else
> +       /* -WORD(AP) in the current frame in interrupt handler.  */
> +       DECL_INCOMING_RTL (arg) = plus_constant (Pmode,
> +                                                arg_pointer_rtx,
> +                                                -UNITS_PER_WORD);
> +    }
>
>
> On Mon, Oct 5, 2015 at 12:29 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> > On Mon, Oct 5, 2015 at 1:17 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> >>>>>>> Looking a bit deeper into the code, it looks that we want to realign
> >>>>>>> the stack in the interrupt handler. Let's assume that interrupt
> >>>>>>> handler is calling some other function that saves SSE vector regs to
> >>>>>>> the stack. According to the x86 ABI, incoming stack of the called
> >>>>>>> function is assumed to be aligned to 16 bytes. But, interrupt handler
> >>>>>>> violates this assumption, since the stack could be aligned to only 4
> >>>>>>> bytes for 32bit and 8 bytes for 64bit targets. Entering the called
> >>>>>>> function with stack, aligned to less than 16 bytes will certainly
> >>>>>>> violate ABI.
> >>>>>>>
> >>>>>>> So, it looks to me that we need to realign the stack in the interrupt
> >>>>>>> handler unconditionally to 16bytes. In this case, we also won't need
> >>>>>>> the following changes:
> >>>>>>>
> >>>>>>
> >>>>>> Current stack alignment implementation requires at least
> >>>>>> one, maybe two, scratch registers:
> >>>>>>
> >>>>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67841
> >>>>>>
> >>>>>> Extend it to the interrupt handler, which doesn't have any scratch
> >>>>>> registers may require significant changes in backend as well as
> >>>>>> register allocator.
> >>>>>
> >>>>> But without realignment, the handler is unusable for anything but
> >>>>> simple functions. The handler will crash when called function will try
> >>>>> to save vector reg to stack.
> >>>>>
> >>>>
> >>>> We can use unaligned load and store to avoid crash.
> >>>
> >>> Oh, sorry, I meant "called function will crash", like:
> >>>
> >>> -> interrupt when %rsp = 0x...8 ->
> >>> -> interrupt handler ->
> >>> -> calls some function that tries to save xmm reg to stack
> >>> -> crash in the called function
> >>>
> >>
> >> It should be fixed by this patch.   But we need to fix stack
> >> alignment in interrupt handler to avoid scratch register.
> >>
> >>
> >> --
> >> H.J.
> >> ---
> >> commit 15f48be1dc7ff48207927d0b835e593d058f695b
> >> Author: H.J. Lu <hjl.tools@gmail.com>
> >> Date:   Sun Oct 4 16:14:03 2015 -0700
> >>
> >>     Correctly set incoming stack boundary for interrupt handler
> >>
> >> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
> >> index 7ebdcd9..0f0cc3c 100644
> >> --- a/gcc/config/i386/i386.c
> >> +++ b/gcc/config/i386/i386.c
> >> @@ -12037,8 +12037,11 @@ ix86_minimum_incoming_stack_boundary (bool sibcall)
> >>  {
> >>    unsigned int incoming_stack_boundary;
> >>
> >> +  /* Stack of interrupt handler is always aligned to word_mode.  */
> >> +  if (cfun->machine->func_type != TYPE_NORMAL)
> >> +    incoming_stack_boundary = TARGET_64BIT ? 64 : 32;
> >
> > Just a heads up that in order to support stack realignmnent on x86_64,
> > MIN_STACK_BOUNDARY will soon be changed to BITS_PER_WORD, so you can
> > use it in the line above. Please see comment #5 and #6 of PR 66697
> > [1].
> >
> >>    /* Prefer the one specified at command line. */
> >> -  if (ix86_user_incoming_stack_boundary)
> >> +  else if (ix86_user_incoming_stack_boundary)
> >>      incoming_stack_boundary = ix86_user_incoming_stack_boundary;
> >>    /* In 32bit, use MIN_STACK_BOUNDARY for incoming stack boundary
> >>       if -mstackrealign is used, it isn't used for sibcall check and
> >
> > [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66697
> >
> > Uros.
Yulia Koval Oct. 23, 2015, 1:11 p.m. UTC | #4 
Added fix for PR68037. Bootstraped/regtested on Linux/x86_64.

Implement x86 interrupt attribute

    The interrupt and exception handlers are called by x86 processors.  X86
    hardware pushes information onto stack and calls the handler.  The
    requirements are

    1. Both interrupt and exception handlers must use the 'IRET' instruction,
    instead of the 'RET' instruction, to return from the handlers.
    2. All registers are callee-saved in interrupt and exception handlers.
    3. The difference between interrupt and exception handlers is the
    exception handler must pop 'ERROR_CODE' off the stack before the 'IRET'
    instruction.

    The design goals of interrupt and exception handlers for x86 processors
    are:

    1. Support both 32-bit and 64-bit modes.
    2. Flexible for compilers to optimize.
    3. Easy to use by programmers.

    To implement interrupt and exception handlers for x86 processors, a
    compiler should support:

    'interrupt' attribute

    Use this attribute to indicate that the specified function with
    mandatory arguments is an interrupt or exception handler.  The compiler
    generates function entry and exit sequences suitable for use in an
    interrupt handler when this attribute is present.  The 'IRET' instruction,
    instead of the 'RET' instruction, is used to return from interrupt or
    exception handlers.  All registers, except for the EFLAGS register which
    is restored by the 'IRET' instruction, are preserved by the compiler.

    Any interruptible-without-stack-switch code must be compiled with
    -mno-red-zone since interrupt handlers can and will, because of the
    hardware design, touch the red zone.

    1. interrupt handler must be declared with a mandatory pointer argument:

    struct interrupt_frame;

    __attribute__ ((interrupt))
    void
    f (struct interrupt_frame *frame)
    {
    ...
    }

    and user must properly define the structure the pointer pointing to.

    2. exception handler:

    The exception handler is very similar to the interrupt handler with
    a different mandatory function signature:

    typedef unsigned int uword_t __attribute__ ((mode (__word__)));

    struct interrupt_frame;

    __attribute__ ((interrupt))
    void
    f (struct interrupt_frame *frame, uword_t error_code)
    {
    ...
    }

    and compiler pops the error code off stack before the 'IRET' instruction.

    The exception handler should only be used for exceptions which push an
    error code and all other exceptions must use the interrupt handler.
    The system will crash if the wrong handler is used.

    To be feature complete, compiler may implement the optional
    'no_caller_saved_registers' attribute:

    Use this attribute to indicate that the specified function has no
    caller-saved registers.  That is, all registers are callee-saved.
    The compiler generates proper function entry and exit sequences to
    save and restore any modified registers.

    The user can call functions specified with 'no_caller_saved_registers'
    attribute from an interrupt handler without saving and restoring all
    call clobbered registers.

    gcc/

    PR target/66960
    PR target/67630
    PR target/67634
    PR target/68037
    * config/i386/i386-protos.h (ix86_epilogue_uses): New prototype.
    * config/i386/i386.c (ix86_frame): Add nbndregs, nmaskregs,
    bnd_reg_save_offset and mask_reg_save_offset.
    (ix86_conditional_register_usage): Preserve all registers,
    except for function return registers if there are no caller-saved
    registers.
    (ix86_set_current_function): Set no_caller_saved_registers and
    func_type.  Call reinit_regs if AX register usage isn't
    consistent.
    (ix86_function_ok_for_sibcall): Return false if there are no
    caller-saved registers.
    (type_natural_mode): Don't warn ABI change for MMX in interrupt
    handler.
    (ix86_function_arg_advance): Skip for callee in interrupt
    handler.
    (ix86_function_arg): Handle arguments for callee in interrupt
    handler.
    (ix86_can_use_return_insn_p): Don't use `ret' instruction in
    interrupt handler.
    (ix86_epilogue_uses): New function.
    (ix86_hard_regno_scratch_ok): Likewise.
    (ix86_reg_ever_defined_p): Likewise.
    (ix86_nsaved_bndregs): Likewise.
    (ix86_nsaved_maskregs): Likewise.
    (ix86_emit_save_bnd_regs_using_mov): Likewise.
    (ix86_emit_save_mask_regs_using_mov): Likewise.
    (ix86_emit_restore_bnd_regs_using_mov): Likewise.
    (ix86_emit_restore_mask_regs_using_mov): Likewise.
    (ix86_handle_no_caller_saved_registers_attribute): Likewise.
    (ix86_handle_interrupt_attribute): Likewise.
    (ix86_save_reg): Preserve all registers in interrupt function
    after reload.  Preserve all registers, except for function
    return registers, if there are no caller-saved registers after
    reload.
    (ix86_nsaved_sseregs): Don't return 0 if there are no
    caller-saved registers.
    (ix86_compute_frame_layout): Set nbndregs and nmaskregs.  Set
    and allocate BND and MASK register save areas.  Allocate space to
    save full vector registers if there are no caller-saved registers.
    (ix86_emit_save_reg_using_mov): Don't use UNSPEC_STOREU to
    SSE registers.
    (ix86_emit_save_sse_regs_using_mov): Save full vector registers
    if there are no caller-saved registers.
    (find_drap_reg): Always use callee-saved register if there are
    no caller-saved registers.
    (ix86_minimum_incoming_stack_boundary): Return MIN_STACK_BOUNDARY
    for interrupt handler.
    (ix86_expand_prologue): Save BND and MASK registers.  Adjust
    DECL_INCOMING_RTL of the pointer argument for interrupt handler.
    (ix86_emit_restore_sse_regs_using_mov): Restore full vector
    registers if there are no caller-saved registers.
    (ix86_expand_epilogue): Restore BND and MASK registers.  Generate
    interrupt return for interrupt handler and pop the 'ERROR_CODE'
    off the stack before interrupt return in exception handler.
    (ix86_expand_move): Disallow 80387 instructions in exception
    handler.
    (ix86_expand_vector_move): Disallow MMX/3Dnow instructions in
    exception handler.
    (ix86_expand_call): Disallow calling interrupt handler directly.
    If there are no caller-saved registers, mark all registers that
    are clobbered by the call as clobbered.
    (ix86_attribute_table): Add interrupt and no_caller_saved_registers
    attributes.
    (TARGET_HARD_REGNO_SCRATCH_OK): New.
    * config/i386/i386.h (ACCUMULATE_OUTGOING_ARGS): Always use
    argument accumulation in interrupt function if stack may be
    realigned to avoid DRAP.
    * config/i386/i386.h (EPILOGUE_USES): New.
    (function_type): New enum.
    (machine_function): Add func_type and no_caller_saved_registers.
    * config/i386/i386.md (UNSPEC_INTERRUPT_RETURN): New.
    (interrupt_return): New pattern.
    * doc/extend.texi: Document x86 interrupt and
    no_caller_saved_registers attributes.

    gcc/testsuite/

    PR target/66960
    PR target/67630
    PR target/67634
    PR target/68037
    * gcc.dg/guality/pr68037-1.c: New test.
    * gcc.dg/guality/pr68037-2.c: Likewise.
    * gcc.dg/guality/pr68037-3.c: Likewise.
    * gcc.dg/torture/pr68037-1.c: Likewise.
    * gcc.dg/torture/pr68037-2.c: Likewise.
    * gcc.dg/torture/pr68037-3.c: Likewise.
    * gcc.target/i386/interrupt-1.c: Likewise.
    * gcc.target/i386/interrupt-2.c: Likewise.
    * gcc.target/i386/interrupt-3.c: Likewise.
    * gcc.target/i386/interrupt-4.c: Likewise.
    * gcc.target/i386/interrupt-5.c: Likewise.
    * gcc.target/i386/interrupt-6.c: Likewise.
    * gcc.target/i386/interrupt-7.c: Likewise.
    * gcc.target/i386/interrupt-8.c: Likewise.
    * gcc.target/i386/interrupt-9.c: Likewise.
    * gcc.target/i386/interrupt-10.c: Likewise.
    * gcc.target/i386/interrupt-11.c: Likewise.
    * gcc.target/i386/interrupt-12.c: Likewise.
    * gcc.target/i386/interrupt-13.c: Likewise.
    * gcc.target/i386/interrupt-14.c: Likewise.
    * gcc.target/i386/interrupt-15.c: Likewise.
    * gcc.target/i386/interrupt-16.c: Likewise.
    * gcc.target/i386/interrupt-17.c: Likewise.
    * gcc.target/i386/interrupt-18.c: Likewise.
    * gcc.target/i386/interrupt-19.c: Likewise.
    * gcc.target/i386/interrupt-20.c: Likewise.
    * gcc.target/i386/interrupt-21.c: Likewise.
    * gcc.target/i386/interrupt-22.c: Likewise.
    * gcc.target/i386/interrupt-23.c: Likewise.
    * gcc.target/i386/interrupt-24.c: Likewise.
    * gcc.target/i386/interrupt-25.c: Likewise.
    * gcc.target/i386/interrupt-26.c: Likewise.
    * gcc.target/i386/interrupt-387-err.c: Likewise.
    * gcc.target/i386/interrupt-bnd.c: Likewise.
    * gcc.target/i386/interrupt-iamcu.c: Likewise.
    * gcc.target/i386/interrupt-mmx-err.c: Likewise.
    * gcc.target/i386/interrupt-redzone-1.c: Likewise.
    * gcc.target/i386/interrupt-redzone-2.c: Likewise.
    * gcc.target/i386/interrupt-sibcall.c: Likewise.
    * gcc.target/i386/interrupt-switch-abi.c: Likewise.
    * gcc.target/i386/interrupt-xmm.c: Likewise.
    * gcc.target/i386/interrupt-ymm.c: Likewise.
    * gcc.target/i386/interrupt-zmm.c: Likewise.

On Tue, Oct 20, 2015 at 4:21 PM, Yulia Koval <vaalfreja@gmail.com> wrote:
> The debug_info section for the interrupt function looks ok.
>
> I tried to call it from assembler code to check it in gdb.
>
>         pushl   $0x333 ;eflags
>         pushl   $0x111 ;cs
>         pushl $0x222   ;eip
>         jmp  foo           ;interrupt function
>
> #define uword_t unsigned int
> struct interrupt_frame
> {
>      uword_t ip;
>      uword_t cs;
>      uword_t flags;
> };
>
> I get this inside the interrupt function:
>
> Breakpoint 1, foo (frame=0xffffd560) at interrupt-1.c:7
> 7               a = (struct interrupt_frame*) frame;
> (gdb) p/x ((struct interrupt_frame*)frame)->ip
> $1 = 0x222
> (gdb) p/x ((struct interrupt_frame*)frame)->cs
> $3 = 0x111
> (gdb) p/x ((struct interrupt_frame*)frame)->flags
> $4 = 0x333
>
> Frame pointer info looks ok.
>
>
> On Tue, Oct 13, 2015 at 3:18 PM, Yulia Koval <vaalfreja@gmail.com> wrote:
>>
>> Here is the current version of the patch with all the fixes.
>> Regtested\bootstraped it on 64 bit.
>>
>> We need a pointer since interrupt handler will update data pointing
>> to by frame.  Since error_code isn't at the normal location where the
>> parameter is passed on stack and frame isn't in a hard register, we
>> changed ix86_function_arg:
>>
>> +  if (!cum->caller && cfun->machine->func_type != TYPE_NORMAL)
>> +    {
>> +      /* The first argument of interrupt handler is a pointer and
>> +        points to the return address slot on stack.  The optional
>> +        second argument is an integer for error code on stack.  */
>> +      gcc_assert (type != NULL_TREE);
>> +      if (POINTER_TYPE_P (type))
>> +       {
>> +         if (cfun->machine->func_type == TYPE_EXCEPTION)
>> +           /* (AP) in the current frame in exception handler.  */
>> +           arg = arg_pointer_rtx;
>> +         else
>> +           /* -WORD(AP) in the current frame in interrupt handler.  */
>> +           arg = force_reg (Pmode,
>> +                            plus_constant (Pmode, arg_pointer_rtx,
>> +                                           -UNITS_PER_WORD));
>> +         if (mode != Pmode)
>> +           arg = convert_to_mode (mode, arg, 1);
>> +       }
>> +      else
>> +       {
>> +         gcc_assert (TREE_CODE (type) == INTEGER_TYPE
>> +                     && cfun->machine->func_type == TYPE_EXCEPTION
>> +                     && mode == word_mode);
>> +         /* The error code is at -WORD(AP) in the current frame in
>> +            exception handler.  */
>> +         arg = gen_rtx_MEM (word_mode,
>> +                            plus_constant (Pmode, arg_pointer_rtx,
>> +                                           -UNITS_PER_WORD));
>> +       }
>> +
>> +      return arg;
>> +    }
>> +
>>
>> to return a pseudo register.  It violates
>>
>>    Return where to put the arguments to a function.
>>    Return zero to push the argument on the stack, or a hard register in
>>    which to store the argument.
>>
>> Register allocator has no problem with parameters in pseudo registers.
>> But GCC crashes when it tries to access DECL_INCOMING_RTL as a hard
>> register when generating debug information.  We worked around it by
>> doing
>>
>> +
>> +  if (cfun->machine->func_type != TYPE_NORMAL)
>> +    {
>> +      /* Since the pointer argument of interrupt handler isn't a real
>> +         argument, adjust DECL_INCOMING_RTL for debug output.  */
>> +      tree arg = DECL_ARGUMENTS (current_function_decl);
>> +      gcc_assert (arg != NULL_TREE
>> +                 && POINTER_TYPE_P (TREE_TYPE (arg)));
>> +      if (cfun->machine->func_type == TYPE_EXCEPTION)
>> +       /* (AP) in the current frame in exception handler.  */
>> +       DECL_INCOMING_RTL (arg) = arg_pointer_rtx;
>> +      else
>> +       /* -WORD(AP) in the current frame in interrupt handler.  */
>> +       DECL_INCOMING_RTL (arg) = plus_constant (Pmode,
>> +                                                arg_pointer_rtx,
>> +                                                -UNITS_PER_WORD);
>> +    }
>>
>>
>> On Mon, Oct 5, 2015 at 12:29 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>> > On Mon, Oct 5, 2015 at 1:17 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> >
>> >>>>>>> Looking a bit deeper into the code, it looks that we want to realign
>> >>>>>>> the stack in the interrupt handler. Let's assume that interrupt
>> >>>>>>> handler is calling some other function that saves SSE vector regs to
>> >>>>>>> the stack. According to the x86 ABI, incoming stack of the called
>> >>>>>>> function is assumed to be aligned to 16 bytes. But, interrupt handler
>> >>>>>>> violates this assumption, since the stack could be aligned to only 4
>> >>>>>>> bytes for 32bit and 8 bytes for 64bit targets. Entering the called
>> >>>>>>> function with stack, aligned to less than 16 bytes will certainly
>> >>>>>>> violate ABI.
>> >>>>>>>
>> >>>>>>> So, it looks to me that we need to realign the stack in the interrupt
>> >>>>>>> handler unconditionally to 16bytes. In this case, we also won't need
>> >>>>>>> the following changes:
>> >>>>>>>
>> >>>>>>
>> >>>>>> Current stack alignment implementation requires at least
>> >>>>>> one, maybe two, scratch registers:
>> >>>>>>
>> >>>>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67841
>> >>>>>>
>> >>>>>> Extend it to the interrupt handler, which doesn't have any scratch
>> >>>>>> registers may require significant changes in backend as well as
>> >>>>>> register allocator.
>> >>>>>
>> >>>>> But without realignment, the handler is unusable for anything but
>> >>>>> simple functions. The handler will crash when called function will try
>> >>>>> to save vector reg to stack.
>> >>>>>
>> >>>>
>> >>>> We can use unaligned load and store to avoid crash.
>> >>>
>> >>> Oh, sorry, I meant "called function will crash", like:
>> >>>
>> >>> -> interrupt when %rsp = 0x...8 ->
>> >>> -> interrupt handler ->
>> >>> -> calls some function that tries to save xmm reg to stack
>> >>> -> crash in the called function
>> >>>
>> >>
>> >> It should be fixed by this patch.   But we need to fix stack
>> >> alignment in interrupt handler to avoid scratch register.
>> >>
>> >>
>> >> --
>> >> H.J.
>> >> ---
>> >> commit 15f48be1dc7ff48207927d0b835e593d058f695b
>> >> Author: H.J. Lu <hjl.tools@gmail.com>
>> >> Date:   Sun Oct 4 16:14:03 2015 -0700
>> >>
>> >>     Correctly set incoming stack boundary for interrupt handler
>> >>
>> >> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
>> >> index 7ebdcd9..0f0cc3c 100644
>> >> --- a/gcc/config/i386/i386.c
>> >> +++ b/gcc/config/i386/i386.c
>> >> @@ -12037,8 +12037,11 @@ ix86_minimum_incoming_stack_boundary (bool sibcall)
>> >>  {
>> >>    unsigned int incoming_stack_boundary;
>> >>
>> >> +  /* Stack of interrupt handler is always aligned to word_mode.  */
>> >> +  if (cfun->machine->func_type != TYPE_NORMAL)
>> >> +    incoming_stack_boundary = TARGET_64BIT ? 64 : 32;
>> >
>> > Just a heads up that in order to support stack realignmnent on x86_64,
>> > MIN_STACK_BOUNDARY will soon be changed to BITS_PER_WORD, so you can
>> > use it in the line above. Please see comment #5 and #6 of PR 66697
>> > [1].
>> >
>> >>    /* Prefer the one specified at command line. */
>> >> -  if (ix86_user_incoming_stack_boundary)
>> >> +  else if (ix86_user_incoming_stack_boundary)
>> >>      incoming_stack_boundary = ix86_user_incoming_stack_boundary;
>> >>    /* In 32bit, use MIN_STACK_BOUNDARY for incoming stack boundary
>> >>       if -mstackrealign is used, it isn't used for sibcall check and
>> >
>> > [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66697
>> >
>> > Uros.
Yulia Koval Nov. 6, 2015, 2:07 p.m. UTC | #5 
Hi,

I updated and reposted the patch. Regtested/bootstraped on
x86_64/Linux and i686/Linux. Ok for trunk?

Implement x86 interrupt attribute

The interrupt and exception handlers are called by x86 processors.  X86
hardware pushes information onto stack and calls the handler.  The
requirements are

1. Both interrupt and exception handlers must use the 'IRET' instruction,
instead of the 'RET' instruction, to return from the handlers.
2. All registers are callee-saved in interrupt and exception handlers.
3. The difference between interrupt and exception handlers is the
exception handler must pop 'ERROR_CODE' off the stack before the 'IRET'
instruction.

The design goals of interrupt and exception handlers for x86 processors
are:

1. Support both 32-bit and 64-bit modes.
2. Flexible for compilers to optimize.
3. Easy to use by programmers.

To implement interrupt and exception handlers for x86 processors, a
compiler should support:

'interrupt' attribute

Use this attribute to indicate that the specified function with
mandatory arguments is an interrupt or exception handler.  The compiler
generates function entry and exit sequences suitable for use in an
interrupt handler when this attribute is present.  The 'IRET' instruction,
instead of the 'RET' instruction, is used to return from interrupt or
exception handlers.  All registers, except for the EFLAGS register which
is restored by the 'IRET' instruction, are preserved by the compiler.

Any interruptible-without-stack-switch code must be compiled with
-mno-red-zone since interrupt handlers can and will, because of the
hardware design, touch the red zone.

1. interrupt handler must be declared with a mandatory pointer argument:

struct interrupt_frame;

__attribute__ ((interrupt))
void
f (struct interrupt_frame *frame)
{
...
}

and user must properly define the structure the pointer pointing to.

2. exception handler:

The exception handler is very similar to the interrupt handler with
a different mandatory function signature:

typedef unsigned int uword_t __attribute__ ((mode (__word__)));

struct interrupt_frame;

__attribute__ ((interrupt))
void
f (struct interrupt_frame *frame, uword_t error_code)
{
...
}

and compiler pops the error code off stack before the 'IRET' instruction.

The exception handler should only be used for exceptions which push an
error code and all other exceptions must use the interrupt handler.
The system will crash if the wrong handler is used.

To be feature complete, compiler may implement the optional
'no_caller_saved_registers' attribute:

Use this attribute to indicate that the specified function has no
caller-saved registers.  That is, all registers are callee-saved.
The compiler generates proper function entry and exit sequences to
save and restore any modified registers.

The user can call functions specified with 'no_caller_saved_registers'
attribute from an interrupt handler without saving and restoring all
call clobbered registers.

gcc/

PR target/66960
PR target/67630
PR target/67634
PR target/68037
* config/i386/i386-protos.h (ix86_epilogue_uses): New prototype.
* config/i386/i386.c (ix86_frame): Add nbndregs, nmaskregs,
bnd_reg_save_offset and mask_reg_save_offset.
(ix86_conditional_register_usage): Preserve all registers,
except for function return registers if there are no caller-saved
registers.
(ix86_set_current_function): Set no_caller_saved_registers and
func_type.  Call reinit_regs if AX register usage isn't
consistent.
(ix86_function_ok_for_sibcall): Return false if there are no
caller-saved registers.
(type_natural_mode): Don't warn ABI change for MMX in interrupt
handler.
(ix86_function_arg_advance): Skip for callee in interrupt
handler.
(ix86_function_arg): Handle arguments for callee in interrupt
handler.
(ix86_can_use_return_insn_p): Don't use `ret' instruction in
interrupt handler.
(ix86_epilogue_uses): New function.
(ix86_hard_regno_scratch_ok): Likewise.
(ix86_reg_ever_defined_p): Likewise.
(ix86_nsaved_bndregs): Likewise.
(ix86_nsaved_maskregs): Likewise.
(ix86_emit_save_bnd_regs_using_mov): Likewise.
(ix86_emit_save_mask_regs_using_mov): Likewise.
(ix86_emit_restore_bnd_regs_using_mov): Likewise.
(ix86_emit_restore_mask_regs_using_mov): Likewise.
(ix86_handle_no_caller_saved_registers_attribute): Likewise.
(ix86_handle_interrupt_attribute): Likewise.
(ix86_save_reg): Preserve all registers in interrupt function
after reload.  Preserve all registers, except for function
return registers, if there are no caller-saved registers after
reload.
(ix86_nsaved_sseregs): Don't return 0 if there are no
caller-saved registers.
(ix86_compute_frame_layout): Set nbndregs and nmaskregs.  Set
and allocate BND and MASK register save areas.  Allocate space to
save full vector registers if there are no caller-saved registers.
(ix86_emit_save_reg_using_mov): Don't use UNSPEC_STOREU to
SSE registers.
(ix86_emit_save_sse_regs_using_mov): Save full vector registers
if there are no caller-saved registers.
(find_drap_reg): Always use callee-saved register if there are
no caller-saved registers.
(ix86_minimum_incoming_stack_boundary): Return MIN_STACK_BOUNDARY
for interrupt handler.
(ix86_expand_prologue): Save BND and MASK registers.  Adjust
DECL_INCOMING_RTL of the pointer argument for interrupt handler.
(ix86_emit_restore_sse_regs_using_mov): Restore full vector
registers if there are no caller-saved registers.
(ix86_expand_epilogue): Restore BND and MASK registers.  Generate
interrupt return for interrupt handler and pop the 'ERROR_CODE'
off the stack before interrupt return in exception handler.
(ix86_expand_move): Disallow 80387 instructions in exception
handler.
(ix86_expand_vector_move): Disallow MMX/3Dnow instructions in
exception handler.
(ix86_expand_call): Disallow calling interrupt handler directly.
If there are no caller-saved registers, mark all registers that
are clobbered by the call as clobbered.
(ix86_attribute_table): Add interrupt and no_caller_saved_registers
attributes.
(TARGET_HARD_REGNO_SCRATCH_OK): New.
* config/i386/i386.h (ACCUMULATE_OUTGOING_ARGS): Always use
argument accumulation in interrupt function if stack may be
realigned to avoid DRAP.
* config/i386/i386.h (EPILOGUE_USES): New.
(function_type): New enum.
(machine_function): Add func_type and no_caller_saved_registers.
* config/i386/i386.md (UNSPEC_INTERRUPT_RETURN): New.
(interrupt_return): New pattern.
* doc/extend.texi: Document x86 interrupt and
no_caller_saved_registers attributes.

gcc/testsuite/

PR target/66960
PR target/67630
PR target/67634
PR target/68037
* gcc.dg/guality/pr68037-1.c: New test.
* gcc.dg/guality/pr68037-2.c: Likewise.
* gcc.dg/guality/pr68037-3.c: Likewise.
* gcc.dg/torture/pr68037-1.c: Likewise.
* gcc.dg/torture/pr68037-2.c: Likewise.
* gcc.dg/torture/pr68037-3.c: Likewise.
* gcc.target/i386/interrupt-1.c: Likewise.
* gcc.target/i386/interrupt-2.c: Likewise.
* gcc.target/i386/interrupt-3.c: Likewise.
* gcc.target/i386/interrupt-4.c: Likewise.
* gcc.target/i386/interrupt-5.c: Likewise.
* gcc.target/i386/interrupt-6.c: Likewise.
* gcc.target/i386/interrupt-7.c: Likewise.
* gcc.target/i386/interrupt-8.c: Likewise.
* gcc.target/i386/interrupt-9.c: Likewise.
* gcc.target/i386/interrupt-10.c: Likewise.
* gcc.target/i386/interrupt-11.c: Likewise.
* gcc.target/i386/interrupt-12.c: Likewise.
* gcc.target/i386/interrupt-13.c: Likewise.
* gcc.target/i386/interrupt-14.c: Likewise.
* gcc.target/i386/interrupt-15.c: Likewise.
* gcc.target/i386/interrupt-16.c: Likewise.
* gcc.target/i386/interrupt-17.c: Likewise.
* gcc.target/i386/interrupt-18.c: Likewise.
* gcc.target/i386/interrupt-19.c: Likewise.
* gcc.target/i386/interrupt-20.c: Likewise.
* gcc.target/i386/interrupt-21.c: Likewise.
* gcc.target/i386/interrupt-22.c: Likewise.
* gcc.target/i386/interrupt-23.c: Likewise.
* gcc.target/i386/interrupt-24.c: Likewise.
* gcc.target/i386/interrupt-25.c: Likewise.
* gcc.target/i386/interrupt-26.c: Likewise.
* gcc.target/i386/interrupt-27.c: Likewise.
* gcc.target/i386/interrupt-28.c: Likewise.
* gcc.target/i386/interrupt-387-err.c: Likewise.
* gcc.target/i386/interrupt-bnd.c: Likewise.
* gcc.target/i386/interrupt-iamcu.c: Likewise.
* gcc.target/i386/interrupt-mmx-err.c: Likewise.
* gcc.target/i386/interrupt-redzone-1.c: Likewise.
* gcc.target/i386/interrupt-redzone-2.c: Likewise.
* gcc.target/i386/interrupt-sibcall.c: Likewise.
* gcc.target/i386/interrupt-switch-abi.c: Likewise.
* gcc.target/i386/interrupt-xmm.c: Likewise.
* gcc.target/i386/interrupt-ymm.c: Likewise.
* gcc.target/i386/interrupt-zmm.c: Likewise.
Uros Bizjak Nov. 6, 2015, 3:31 p.m. UTC | #6 
On Fri, Nov 6, 2015 at 3:07 PM, Yulia Koval <vaalfreja@gmail.com> wrote:
> Hi,
>
> I updated and reposted the patch. Regtested/bootstraped on
> x86_64/Linux and i686/Linux. Ok for trunk?

This version still emits insns from ix86_function_arg, so NAK.

Uros.
H.J. Lu Nov. 6, 2015, 10:19 p.m. UTC | #7 
On Fri, Nov 6, 2015 at 7:31 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
> On Fri, Nov 6, 2015 at 3:07 PM, Yulia Koval <vaalfreja@gmail.com> wrote:
>> Hi,
>>
>> I updated and reposted the patch. Regtested/bootstraped on
>> x86_64/Linux and i686/Linux. Ok for trunk?
>
> This version still emits insns from ix86_function_arg, so NAK.
>
> Uros.

Hi Uros,

The ix86_function_arg change only applies the interrupt function body,
which has fixed parameters and won't be called by any functions directly.
We have verified that this approach works with all optimization levels.
This approach has no impact on normal functions. I don't think we should
change the middle-end for x86 interrupt functions which are never called
by GCC nor follows the normal psABI. We will address any issues which
may come up later.

If this isn't the right approach, do you have any suggestions?

Thanks.


H.J.
Jeff Law Nov. 7, 2015, 4:50 a.m. UTC | #8 
On 11/06/2015 03:19 PM, H.J. Lu wrote:
> On Fri, Nov 6, 2015 at 7:31 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
>> On Fri, Nov 6, 2015 at 3:07 PM, Yulia Koval <vaalfreja@gmail.com> wrote:
>>> Hi,
>>>
>>> I updated and reposted the patch. Regtested/bootstraped on
>>> x86_64/Linux and i686/Linux. Ok for trunk?
>>
>> This version still emits insns from ix86_function_arg, so NAK.
>>
>> Uros.
>
> Hi Uros,
>
> The ix86_function_arg change only applies the interrupt function body,
> which has fixed parameters and won't be called by any functions directly.
Uros was pretty clear that generating code from within ix86_function_arg 
was a NAK from his side (and I'd agree with that assessment from Uros). 
  Generating code like that from within FUNCTION_ARG is pretty bad.

Can you arrange to load up the arguments from within the prologue so 
that they look like they were passed in?  Alternately you have to make 
FUNCTION_ARG return 0 for those cases and arrange to define all the 
other argument passing offsets appropriately so those arguments can be 
found in the stack.

Jeff
diff mbox

Patch

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 7ebdcd9..0f0cc3c 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -12037,8 +12037,11 @@  ix86_minimum_incoming_stack_boundary (bool sibcall)
 {
   unsigned int incoming_stack_boundary;

+  /* Stack of interrupt handler is always aligned to word_mode.  */
+  if (cfun->machine->func_type != TYPE_NORMAL)
+    incoming_stack_boundary = TARGET_64BIT ? 64 : 32;
   /* Prefer the one specified at command line. */
-  if (ix86_user_incoming_stack_boundary)
+  else if (ix86_user_incoming_stack_boundary)
     incoming_stack_boundary = ix86_user_incoming_stack_boundary;
   /* In 32bit, use MIN_STACK_BOUNDARY for incoming stack boundary
      if -mstackrealign is used, it isn't used for sibcall check and