jacobly0 / llvm-project

This fork of the canonical git mirror of the LLVM subversion repository adds (e)Z80 targets. Please refer to the wiki for important build instructions.
https://github.com/jacobly0/llvm-project/wiki
123 stars 15 forks source link

__attribute__((__interrupt__)) doesn't produce correct code #26

Closed cocoacrumbs closed 2 years ago

cocoacrumbs commented 2 years ago

Hi,

I tried using __attribute__((__interrupt__)) so that a C routine is called when an interrupt happens but I discovered the following:

Consider the following C code:

__attribute__((__interrupt__))
void interrupt_code_generation_test(void)
{
    int a;
    int b;
    int c;

    a = 1;
    b = 2;
    c = a + b;
} /* end interrupt_code_generation_test */

void non_interrupt_code_generation_test(void)
{
    int a;
    int b;
    int c;

    a = 1;
    b = 2;
    c = a + b;
} /* end non_interrupt_code_generation_test */

int main(void)
{
    int a;
    int b;
    int c;

    a = 1;
    b = 2;
    c = a + b;

    return c;
} /* end main */

I get the following assembly code back when using this command line ./ez80-none-elf-clang -S -target ez80-none-elf main.c:

    section .text,"ax",@progbits
    assume  adl = 1
    section .text,"ax",@progbits
    public  _interrupt_code_generation_test
_interrupt_code_generation_test:
    exx
    push    ix
    ld  ix, 0
    add ix, sp
    ld  hl, -9
    add hl, sp
    ld  sp, hl
    ld  hl, 1
    ld  de, 2
    ld  (ix - 9), hl
    ld  (ix - 12), de
    ld  hl, (ix - 9)
    ld  de, (ix - 12)
    add hl, de
    ld  (ix - 15), hl
    ld  hl, 9
    add hl, sp
    ld  sp, hl
    pop ix
    exx
    ret
    section .text,"ax",@progbits

    section .text,"ax",@progbits
    public  _non_interrupt_code_generation_test
_non_interrupt_code_generation_test:
    push    ix
    ld  ix, 0
    add ix, sp
    ld  hl, -9
    add hl, sp
    ld  sp, hl
    ld  hl, 1
    ld  de, 2
    ld  (ix - 3), hl
    ld  (ix - 6), de
    ld  hl, (ix - 3)
    ld  de, (ix - 6)
    add hl, de
    ld  (ix - 9), hl
    ld  hl, 9
    add hl, sp
    ld  sp, hl
    pop ix
    ret
    section .text,"ax",@progbits

    section .text,"ax",@progbits
    public  _main
_main:
    push    ix
    ld  ix, 0
    add ix, sp
    ld  hl, -12
    add hl, sp
    ld  sp, hl
    or  a, a
    sbc hl, hl
    ld  de, 1
    ld  bc, 2
    ld  (ix - 3), hl
    ld  (ix - 6), de
    ld  (ix - 9), bc
    ld  hl, (ix - 6)
    ld  de, (ix - 9)
    add hl, de
    ld  (ix - 12), hl
    ld  hl, (ix - 12)
    ld  iy, 12
    add iy, sp
    ld  sp, iy
    pop ix
    ret
    section .text,"ax",@progbits

    ident   "clang version 14.0.0 (https://github.com/jacobly0/llvm-project.git b1303ec02a1932c74c306658339d3386d7f46b47)"
    extern  __Unwind_SjLj_Register
    extern  __Unwind_SjLj_Unregister

In the assembly code we can see that the offsets in _interrupt_code_generation_test are different than those (correctly) generated for _non_interrupt_code_generation_test.

We also see that for _interrupt_code_generation_test, the ex af, af', reti and ei instructions are missing.

I was able to work around this with a small wrapper assembly code like this to implement the isr_uart0() routine:

_isr_uart0_wrapper:
    ex      af, af'
    exx

    call    _isr_uart0

    ex      af, af'
    exx
    ei
    reti

with _isr_uart0 being a normal C function with this prototype void isr_uart0(void) and without the __attribute__((__interrupt__)) of course. Implemented like this I got a working uart0 interrupt routine .

This is also the way how the ZDS II C compiler generates code for C functions preceded by the #pragma interrupt. I.e. starting with an ex af, af' and exx and ending with ex af, af', exx, ei and reti.

Attached you can find a patch which generates the assembly code as described above but it might need improvements (I'm a total newbie regarding debugging compilers).

I did not find what causes the wrong offsets when __attribute__((__interrupt__)) is used (that will take a lot more time to study for me).

A minor problem arises when optimization is used. E.g. using this command line: ./ez80-none-elf-clang -S -target ez80-none-elf main.c -O1:

    section .text,"ax",@progbits
    assume  adl = 1
    section .text,"ax",@progbits
    public  _interrupt_code_generation_test
_interrupt_code_generation_test:
    ret
    section .text,"ax",@progbits

    section .text,"ax",@progbits
    public  _non_interrupt_code_generation_test
_non_interrupt_code_generation_test:
    ret
    section .text,"ax",@progbits

    section .text,"ax",@progbits
    public  _main
_main:
    ld  hl, 3
    ret
    section .text,"ax",@progbits

    ident   "clang version 14.0.0 (https://github.com/jacobly0/llvm-project.git b1303ec02a1932c74c306658339d3386d7f46b47)"
    extern  __Unwind_SjLj_Register
    extern  __Unwind_SjLj_Unregister

The problems I have here are:

Best regards,

Koen

0004-Fix-EI-RETI_partial-fix.patch.zip

adriweb commented 2 years ago

Regarding the "optimized out" part, have you tried __attribute__((optnone))?

cocoacrumbs commented 2 years ago

Hi,

To avoid confusion I tried your suggestion like so:

__attribute__((__interrupt__))
__attribute__((optnone))
void interrupt_code_generation_test(void)
{
    int a;
    int b;
    int c;

    a = 1;
    b = 2;
    c = a + b;
} /* end interrupt_code_generation_test */

void non_interrupt_code_generation_test(void)
{
    int a;
    int b;
    int c;

    a = 1;
    b = 2;
    c = a + b;
} /* end non_interrupt_code_generation_test */

int main(void)
{
    int a;
    int b;
    int c;

    a = 1;
    b = 2;
    c = a + b;

    return c;
} /* end main */

And this is the resulting assembly:

    section .text,"ax",@progbits
    assume  adl = 1
    section .text,"ax",@progbits
    public  _interrupt_code_generation_test
_interrupt_code_generation_test:
    exx
    ld  iy, 0
    add iy, sp
    ld  hl, -9
    add hl, sp
    ld  sp, hl
    ld  hl, 1
    ld  de, 2
    ld  (iy - 9), hl
    ld  (iy - 12), de
    ld  hl, (iy - 9)
    ld  de, (iy - 12)
    add hl, de
    ld  (iy - 15), hl
    ld  hl, 9
    add hl, sp
    ld  sp, hl
    exx
    ret
    section .text,"ax",@progbits

    section .text,"ax",@progbits
    public  _non_interrupt_code_generation_test
_non_interrupt_code_generation_test:
    ret
    section .text,"ax",@progbits

    section .text,"ax",@progbits
    public  _main
_main:
    ld  hl, 3
    ret
    section .text,"ax",@progbits

    ident   "clang version 14.0.0 (https://github.com/jacobly0/llvm-project.git b1303ec02a1932c74c306658339d3386d7f46b47)"
    extern  __Unwind_SjLj_Register
    extern  __Unwind_SjLj_Unregister

So, it seems that __attribute__((optnone)) works :-)

Best regards,

Koen

jacobly0 commented 2 years ago

User interrupt handling was removed before I ever got around to implement it, if you could provide the asm you want it to generate I can work on it.

Also, it's normal for a function that does nothing to be optimized to a function that does nothing, but you still need a ret to return from the routine of course.

cocoacrumbs commented 2 years ago

Hi,

Looking at the output of the Zilog compiler when the #pragma interrupt is used, then I see that this code sequence is generated:

    exx
    ex      af, af'

    ; interrupt handling code

    exx
    ex      af, af'
    ei
    reti    

Attached a zip file with various files that might help to understand what code needs to be generated based on a real, working interrupt handling code for uart0 (basically a copy of the one present in the Zilog ZDS II toolchain).

In the asm sub folder you'll find:

The other files in the root directory might give more insight maybe how the C compiler currently generates assembly code:

But, with this patch applied, the code is still not working. When I compare isruart0-no-attribute-interrupt.c.lst and isruart0-attribute-interrupt-and-patch.c.lst I still see a few differences in the interrupt handling code itself:

    lea hl, ix - 5              lea hl, ix - 5   <== wrong offset?
                        ...
    ld  (ix - 5), l             ld  (ix - 20), l

                        ...
.LBB0_8:
    ld  l, (ix - 5)             ld  l, (ix - 20)
                        ...
.LBB0_25:
    ld  l, (ix - 5)             ld  l, (ix - 20)

As my manually modified assembly code in the asm directory illustrated, the code works without different offsets.

Something else I bumped into when I tried comparing the output with x86-64 assembly. I got this error message:

$ clang main.c -S
main.c:3:1: error: x86-64 'interrupt' attribute only applies to functions that have only a pointer parameter optionally followed by an integer parameter
__attribute__((__interrupt__))

I don't know if this only applies to x86-64 code or for all cpu's. I tried changing the code to void isr_uart0(void *framePtr); but that made no difference for me.

I hope this can help a bit to get to working interrupt handling code? The patch I mentioned is attached to my initial comment when I opened this issue.

interruptHandlingExamples.zip

jacobly0 commented 2 years ago

I still have no clue what the expected output is so closing unless there are still issues.