cpc / openasip

Open Application-Specific Instruction Set processor tools (OpenASIP)
http://openasip.org
Other
140 stars 42 forks source link

oa-selftest fails due to warnings under Clang #185

Closed sarahec closed 1 year ago

sarahec commented 1 year ago

I was testing compilation using clang (which works, by the way) but tce-selftest fails when the compiler emits some warnings.

Since the compilation tests check the return code and stderr, and run the compiled program, can we remove the assertion that stdout is empty?

P.S. I'm using clang so I can use the language server to find all the calls to the database handling so we can do the HW DB update safely.

pjaaskel commented 1 year ago

Good to hear TCE compiles with Clang. It must have been more than a decade since I tested the last time :)

Can you add -Wno* flags to filter out those warnings or better just fix the root causes for the warnings? Somehow it feels better so we can weed out surprising warnings instead of hiding all warnings which might be good to fix.

Great to hear that you are attempting to update HWDB to the modern world.

sarahec commented 1 year ago

Agreed, I'll add -Wno for now and look at the passing vs. failing behavior to see if we can add a heuristic to the self-test.

Re: fixing the warnings, since clang-tidy also works, I want to look through the lint messages and see how many I can clean up. This would also take care of the warnings in the test case. I'll file that as its own issue and PR.

sarahec commented 1 year ago
AssertionError: 'In file included from /home/sec/local/inc[833 chars]d.\n' != ''
- In file included from /home/sec/local/include/PluginCompileWrapper.cc:39:
- /home/sec/local/include/TCEISelLowering.cc:1108:25: warning: 188 enumeration values not handled in switch: 'INVALID_SIMPLE_VALUE_TYPE', 'Other', 'i1'... [-Wswitch]
-                 switch (elemVT.SimpleTy) {
-                         ^~~~~~~~~~~~~~~
- In file included from /home/sec/local/include/PluginCompileWrapper.cc:42:
- /home/sec/local/include/TCESubtarget.cc:77:13: warning: comparison of address of 'this->InstrItins' not equal to a null pointer is always true [-Wtautological-pointer-compare]
-     assert(&InstrItins != nullptr && "IstrItins nullptr");
-             ^~~~~~~~~~    ~~~~~~~
- /usr/include/assert.h:90:27: note: expanded from macro 'assert'
-      (static_cast <bool> (expr)                                         \
-                           ^~~~
- 2 warnings generated.

It's coming from stderr, so we'll need to either suppress the warnings or use a heuristic. Both of the warnings are coming from deep enough in the code that I don't want to touch the code until I can look at a more comprehensive fix based on linting.

pjaaskel commented 1 year ago

These look easily fixable. Is the switch missing a default? The latter is true, you can just remove the assert?

sarahec commented 1 year ago

The problem isn't happening under oa-selftest, so I'm closing this.

sarahec commented 1 year ago

The warnings re-surfaced. I'll file a small patch to handle them.