t-crest / patmos

Patmos is a time-predictable VLIW processor, and the processor for the T-CREST project
http://patmos.compute.dtu.dk
BSD 2-Clause "Simplified" License
134 stars 72 forks source link

Towards the compiler #135

Open schoeberl opened 1 year ago

schoeberl commented 1 year ago

Not really an issue, but notes (from @Emoun) what needs to be done in the Patmos C source files and in the handbook:

Changes:

Notes: Handbook says by default clang compiles to bitcode (section 8.2.1). This is wrong.

schoeberl commented 7 months ago

Naked functions are not allowed to have C code? That isn't good. What about the bootloader?

Emoun commented 7 months ago

Yes, see here for the definition in RISC-V and here for how microsoft treats them. We need to stick to convention and also disallow C code in naked functions.

This not an issue, as you can call another function that is implemented in C after you do any needed setup. The std _start function has already been fixed so it uses naked functions correctly. I did not consider that the bootloader also has a start function that is naked, so I'll get that fixed at some point too.

schoeberl commented 7 months ago

The Microsoft statement does not explicitly disallow C code. But I see the point. I guess this is not a C standard, but compiler-specific.

On bootloader: don't you need to start with a naked function anyway as long as no one has setup at least the stack pointer?

Emoun commented 7 months ago

Yes, you are right about microsoft, but GCC seems to me to treat C code in naked functions as undefined behavior. I presume clang by extension does the same, but did not find any documentation for this after a quick search.

Our bootloader does start with a naked function that sets up the stack using inline asm.

schoeberl commented 7 months ago

But it calls C functions with C code (the call is C, not inline assembly): https://github.com/t-crest/patmos/blob/f237b5ce18f5f853362541c4010ea4ffd8826e8f/c/include/bootable.h#L67 Is this considered OK, then?

Emoun commented 7 months ago

No, not in my opinion. That is what I was refering to earlier that I need to fix. Just like I fixed the newlib start function.

schoeberl commented 7 months ago

But there you the call itself in assembler, but the bootloader does not. Could this be the issue with the bootloader?

Emoun commented 7 months ago

It definitely needs fixing. I don't think it's the current issue, as I seem to remember the bootloader failing only when downloading a program and not before. But perhaps the issue only manifests at that point but is indeed caused by _start. We will see when I get to it.

schoeberl commented 7 months ago

I would propose to wrap those three functions into one, so you only need to call one function from assembler.