pulp-platform / pulp-riscv-gnu-toolchain

Other
68 stars 50 forks source link

Compiler behaves unexpectedly #18

Open acy54321 opened 4 years ago

acy54321 commented 4 years ago

Hi,

There are some problems encountered here. During the calculation of the unsign memory load behavior, it was found that the behavior of the compiler may be wrong through the analysis of the assembly.

Related C code test.c : #include <stdio.h>unsigned int sum_output = 0;int main() {unsigned int MEM[][2] = { {0x1000,0x1000}, {0x1000,0x1000},{0x1000,0x1000}, {0x1000,0x1000}, }; for (unsigned char data = 0x7F; data < 0xFF; data ++) { for (unsigned int i = 0; i < sizeof(MEM) / 4 / 2; i++) { volatile unsigned char *address = (unsigned char *)MEM[i][0]; for (unsigned int j = 0; j < (MEM[i][1] - MEM[i][0]) / sizeof(unsigned char) + 1; j++) { asm volatile("nop;nop;"); asm volatile("nop;nop;"); sum_output += address[j]; asm volatile("nop;nop;"); asm volatile("nop;nop;"); } } } return 0;}

format edit by bluew:

#include <stdio.h>

unsigned int sum_output = 0;
int main() {
   unsigned int MEM[][2] = {
   {0x1000,0x1000},
   {0x1000,0x1000},
   {0x1000,0x1000},
   {0x1000,0x1000}, };
   for (unsigned char data = 0x7F; data < 0xFF; data ++) {
       for (unsigned int i = 0; i < sizeof(MEM) / 4 / 2; i++) {
           volatile unsigned char *address = (unsigned char *)MEM[i][0];
           for (unsigned int j = 0; j < (MEM[i][1] - MEM[i][0]) / sizeof(unsigned char) + 1; j++) {
               asm volatile("nop;nop;"); asm volatile("nop;nop;");
               sum_output += address[j];
               asm volatile("nop;nop;"); asm volatile("nop;nop;");
           }
       }
    }
    return 0;
}

Compiler Version : Release Label v1.0.16-pulp-riscv-gcc https://github.com/pulp-platform/pulp-riscv-gnu-toolchain/releases/download/v1.0.16/v1.0.16-pulp-riscv-gcc-ubuntu-16.tar.bz2

**The analysis process is as follows: Step 1(original) : riscv32-unknown-elf-gcc -O2 -march=rv32IMCXpulpv2 -Wa,-march=rv32IMCXpulpv2 -Wextra -Wall -Wno-unused-function -fmerge-all-constants -fno-exceptions -fno-unwind-tables -fno-asynchronous-unwind-tables -fdata-sections -ffunction-sections -fdiagnostics-color=always -fstack-usage -nostartfiles -fsigned-char -Wno-pointer-sign -Wl,--no-check-sections -Wl,--gc-sections -Wall -Werror -Wno-shift-negative-value -Wno-unused-but-set-variable -Wno-unused-variable -msave-restore -c test.c -o test.o image

Note : I used "unsigned char *" assigned to memory, but assembly use 'p.lb' not 'p.lbu' This instruction has a sign extension behavior, so use the instruction will cause a1 register value error, when value >= 0x80, because 0x80 will sign extension to 0xFFFF_FF80,The next operation(add/sub....) will cause an error result.

Step 2 ("unsigned char " modify to "unsigned short ") : riscv32-unknown-elf-gcc -O2 -march=rv32IMCXpulpv2 -Wa,-march=rv32IMCXpulpv2 -Wextra -Wall -Wno-unused-function -fmerge-all-constants -fno-exceptions -fno-unwind-tables -fno-asynchronous-unwind-tables -fdata-sections -ffunction-sections -fdiagnostics-color=always -fstack-usage -nostartfiles -fsigned-char -Wno-pointer-sign -Wl,--no-check-sections -Wl,--gc-sections -Wall -Werror -Wno-shift-negative-value -Wno-unused-but-set-variable -Wno-unused-variable -msave-restore -c test.c -o test.o image

Note : Assembly behaves as expected use 'p.lhu'

Step 3 (change -march to original RISCV32) : riscv32-unknown-elf-gcc -O2 -march=rv32IMC -Wa,-march=rv32IMC -Wextra -Wall -Wno-unused-function -fmerge-all-constants -fno-exceptions -fno-unwind-tables -fno-asynchronous-unwind-tables -fdata-sections -ffunction-sections -fdiagnostics-color=always -fstack-usage -nostartfiles -fsigned-char -Wno-pointer-sign -Wl,--no-check-sections -Wl,--gc-sections -Wall -Werror -Wno-shift-negative-value -Wno-unused-but-set-variable -Wno-unused-variable -msave-restore -c test.c -o test.o image

Note : Assembly behaves as expected use 'lbu'

Conclusion : Through the above three experiments, can find Compiler behaves unexpectedly on above programming method.

Please help clarify whether this situation is correct

bluewww commented 4 years ago

I used "unsigned char *" assigned to memory, but assembly use 'p.lb' not 'p.lbu' This instruction has a sign extension behavior, so use the instruction will cause a1 register value error, when value >= 0x80, because 0x80 will sign extension to 0xFFFF_FF80,The next operation(add/sub....) will cause an error result.

Indeed looks incorrect assembly is being emitted. @haugoug ?

haugoug commented 4 years ago

Indeed, the assembly emitted is wrong. We'll try to have a look soon. Meanwhile if you see any possible fix, let us know.

acy54321 commented 4 years ago

2020/2/13 :

**The analysis process is as follows: Step 4 (use Compiler dumping at various stages of processing the intermediate language tree to a file) : riscv32-unknown-elf-gcc -O2 -march=rv32IMCXpulpv2 -Wa,-march=rv32IMCXpulpv2 -Wextra -Wall -Wno-unused-function -fmerge-all-constants -fno-exceptions -fno-unwind-tables -fno-asynchronous-unwind-tables -fdata-sections -ffunction-sections -fdiagnostics-color=always -fstack-usage -nostartfiles -fsigned-char -Wno-pointer-sign -Wl,--no-check-sections -Wl,--gc-sections -Wall -Werror -Wno-shift-negative-value -Wno-unused-but-set-variable -Wno-unused-variable -msave-restore -c test.c -o test.o -fdump-tree-all

Note :

  1. ("unsigned char *") I observed test.c.259r.combine stage, memory unsigned load operation have been replace to 'loadqi_ind_reg_reg' too early, (define in gcc/config/riscv/riscv.md) cause subsequent assembly code conversion errors.
acy54321 commented 4 years ago

Hi @haugoug , I unfamiliar with GCC compiler and not find solution. Is there any planning time for this question?

haugoug commented 4 years ago

We don't really work on the compiler anymore, since the design of the core has been transferred to openhw group, I don't know if someone will take care of this issue. Maybe you can check with them if they have some plan to take over the compiler.