On Thu, Oct 1, 2015 at 6:08 PM, H.J. Lu <hjl.to...@gmail.com> wrote:
> On Thu, Oct 1, 2015 at 8:59 AM, Uros Bizjak <ubiz...@gmail.com> wrote:
>> On Thu, Oct 1, 2015 at 2:24 AM, H.J. Lu <hjl.to...@gmail.com> wrote:
>>> On Wed, Sep 30, 2015 at 12:53 PM, Yulia Koval <vaalfr...@gmail.com> wrote:
>>>> Done.
>>>>
>>>
>>> + /* If true, the current function is an interrupt service
>>> + routine as specified by the "interrupt" attribute. */
>>> + BOOL_BITFIELD is_interrupt : 1;
>>> +
>>> + /* If true, the current function is an exception service
>>> + routine as specified by the "interrupt" attribute. */
>>> + BOOL_BITFIELD is_exception : 1;
>>>
>>>
>>> It is not very clear what is the difference between is_interrupt
>>> and is_exception. How about
>>>
>>> /* If true, the current function is an interrupt service routine with
>>> a pointer argument and an optional integer argument as specified by
>>> the "interrupt" attribute. */
>>> BOOL_BITFIELD is_interrupt : 1;
>>>
>>> /* If true, the current function is an interrupt service routine with
>>> a pointer argument and an integer argument as specified by the
>>> "interrupt" attribute. */
>>> BOOL_BITFIELD is_exception : 1;
>>
>> Actually, both BOOL_BITFIELD flags should be rewritten as 2-bit
>> ENUM_BITFIELD using descriptive enum, e.g.
>>
>> ENUM_BITFIELD(function_type) func_type : 2;
>>
>> with
>>
>> TYPE_NORMAL = 0,
>> TYPE_INTERRUPT,
>> TYPE_EXCEPTION
>>
>> This will simplify checking of function types, and make everything
>> more readable and maintainable.
>>
>
> Since an exception handler is a subset of interrupt handlers,
> we need to check 2 bits separately. Make the field 2 bits
> doesn't make code more maintainable.
For example, the following code:
+ if (!cum->caller && cfun->machine->is_interrupt)
+ {
+ /* The first argument of interrupt handler is a pointer and
+ points to the return address 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->is_exception)
would become:
if (!cum->caller && cfun->machine->func_type != TYPE_NORMAL)
{
[...]
if (cfun->machine->func_type == TYPE_EXCEPTION)
[...]
It is kind of unintuitive if the function is an interrupt and an
exception at the same time, as is implied in the original code. In
proposed improvement, it is clear that it is not normal, and is later
refined to an exception.
Uros.
> --
> H.J.
