tum-ei-eda / etiss

Extendable Translating Instruction Set Simulator
https://tum-ei-eda.github.io/etiss/
Other
29 stars 36 forks source link

GCCJIT: enable -Wall flag in debug mode #145

Closed PhilippvK closed 3 months ago

PhilippvK commented 4 months ago

Use -Wall flag to print all warnings generated by GCCJIT when jit.debug=true to ease debugging.

PhilippvK commented 4 months ago

After this change, a long list of warnings shows up which should be eliminated by adapting ETISS/M2-ISA-R:

./tmp/9LcIJB/code_21.c:1763:56: warning: suggest parentheses around comparison in operand of ‘|’ [-Wparentheses]
 1763 | if (cpu->return_pending | cpu->exception | cpu->nextPc != 268449458ULL) return cpu->exception;
./tmp/9LcIJB/code_24.c:26:1: warning: label ‘instr_exit_268479698’ defined but not used [-Wunused-label]
   26 | instr_exit_268479698:
      | ^~~~~~~~~~~~~~~~~~~~
./tmp/9LcIJB/code_24.c:15:14: warning: unused variable ‘mem_ret_code’ [-Wunused-variable]
   15 | etiss_uint32 mem_ret_code = 0;

@wysiwyng

wysiwyng commented 4 months ago

-Wunused should be disabled, these labels and variables are more or less impossible to omit conditionally in M2-ISA-R.

The parentheses warnings should indeed be investigated, although for your example I don't really see what GCC complains about.

PhilippvK commented 4 months ago

-Wunused should be disabled

Done.

Here is the parentheses warnings with the annotation. The LHS of the != are probably ambiguous here...

./tmp/00Y7sj/code_3.c:1980:56: warning: suggest parentheses around comparison in operand of ‘|’ [-Wparentheses]
 1980 | if (cpu->return_pending | cpu->exception | cpu->nextPc != 268487074ULL) return cpu->exception;
      |                                            ~~~~~~~~~~~~^~~~~~~~~~~~~~~
wysiwyng commented 4 months ago

The error here is the use of the | operator in the first place, instead of ||. Should be fixed in https://github.com/tum-ei-eda/etiss/commit/2ca6acb742d9a9b3d1f4ddcc2b0c151f78f4b792 and https://github.com/tum-ei-eda/M2-ISA-R/commit/4bd99d3875df8aaa4428fa7c6dba583d557551ad.

PhilippvK commented 3 months ago

@wysiwyng Thanks for that fix. Can we merge this?