New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

firmware: Fix unused-var and type mismatch error in recent gcc (7+). #448

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@cr1901
Copy link
Contributor

cr1901 commented Aug 19, 2018

When compiling the HDMI2USB firmware w/ recent gcc versions (I first noticed them in 7+), the build would error out in pll.c and ci.c. These are my proposed fixes.

The pll.c workaround (void) pll_config10x[0]; is a no-op that makes gcc (and probably other C compilers) treat a variable as used. -Werror=unused-const-variable is enabled, and since it's a conditional compile for now that determines whether pll_config10x[0] actually gets used, I think this is a better solution until the S6 issues get worked out.

The ci.c fix was attempting to compare a char * to a char literal, and I believe that to be a genuine bug. :)

@@ -70,6 +70,8 @@ void pll_config_for_clock(int freq)
* Reproducible both with DRP and initial reconfiguration.
* Until this spartan6 weirdness is sorted out, just stick to 20x.
*/

(void) pll_config_10x[0]; // Dummy access to suppress to suppress -Werror=unused-const-variable.

This comment has been minimized.

This comment has been minimized.

@cr1901

cr1901 Aug 26, 2018

Contributor

Clearly, there is no universal solution :P. the sizeof solution causes warnings on newer compilers.

I think unused vars are infrequent enough that I prefer my solution with works just fine for this single use case. That said, if you really want a macro, __attribute__(unused) should be fine, since gcc/clang is currently the only compiler for all CPUs we support. But I would prefer to gate it behind a macro like the second answer (just in case).

What header in HDMI2USB/litex-buildenv should such a macro go?

@mithro

This comment has been minimized.

Copy link
Member

mithro commented Oct 27, 2018

@cr1901 Any idea why we are not seeing this issue on Travis now I have rolled the latest GCC?

@cr1901

This comment has been minimized.

Copy link
Contributor

cr1901 commented Oct 27, 2018

No, I haven't a clue. Perhaps GCC's warnings got less pessimistic?

@mithro

This comment has been minimized.

Copy link
Member

mithro commented Oct 28, 2018

It seems like this pull request has already been merged. I'm going to close it.

@mithro mithro closed this Oct 28, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment