search

riscv-software-src / riscv-tests

Other
902 stars 463 forks source link

When building benchmarks/dhrystone/, -Wimplicit-int and -Wimplicit-function-declaration cause many errors #598

Closed Heng-Zhou closed 5 days ago

Heng-Zhou commented 6 days ago

I encountered errors when running make command in "Building from repository" to build the riscv-tests repo:

....../riscv-tests/benchmarks/dhrystone/dhrystone.c:20:1: error: return type defaults to 'int' [-Wimplicit-int]
   20 | Proc_6 (Enum_Val_Par, Enum_Ref_Par)
      | ^~~~~~
....../riscv-tests/benchmarks/dhrystone/dhrystone.c: In function 'Proc_6':
....../riscv-tests/benchmarks/dhrystone/dhrystone.c:29:9: error: implicit declaration of function 'Func_3' [-Wimplicit-function-declaration]
   29 |   if (! Func_3 (Enum_Val_Par))
      |         ^~~~~~
....../riscv-tests/benchmarks/dhrystone/dhrystone.c: At top level:
....../riscv-tests/benchmarks/dhrystone/dhrystone.c:54:1: error: return type defaults to 'int' [-Wimplicit-int]
   54 | Proc_7 (Int_1_Par_Val, Int_2_Par_Val, Int_Par_Ref)
      | ^~~~~~

Similar errors can also be found at many other places of dhrystone.c and dhrystone_main.c. Can you please help me fix this issue? Thank you.

Heng-Zhou commented 6 days ago

Just saw the fix here: https://github.com/riscv-software-src/riscv-tests/compare/master...pratikkedar:riscv-tests:pratik-test

Can the unit tests team please apply the fix as soon as possible? Thanks a lot.

aswaterman commented 5 days ago

No. Messing with the dhrystone benchmark source code is not a good idea, even if the changes are seemingly benign. Better to alter the Makefile to suppress the errors. It looks like #587 chose that approach; I merged it, so I'll close this PR as resolved.

Heng-Zhou commented 5 days ago

This problem stems from bad C/C++ coding style in dhrystone.c and dhrystone_main.c, not from missing of compiling flags. If there is any mess due to the fix, it is the mess of the dhrystone source codes, which are exactly what we should fix.

aswaterman commented 4 days ago

I know that the archaic style of the Dhrystone source code is the problem. But benchmark results are not valid if the source code is altered. Anyway, the problem has been worked around, so we should consider this issue resolved.