Skip to content
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

Set F7 GCC optimizations to -O2 for F745 and -Os for F277 Always #8953

Merged
merged 2 commits into from
Apr 7, 2023

Conversation

DzikuVx
Copy link
Member

@DzikuVx DzikuVx commented Apr 5, 2023

As shown in #8905 CMAKE and various optimizations levels do not like each other with GCC 10 family.

This PR forces F722 always to -Os and F745 -O2

On MR tasks execution times look fine on default settings (4kHz gyro and 2kHz on PID). Some in-flight tests might be useful as I don't have any F7 right now to test.

For more details see https://gist.github.com/stronnag/08f79fa9ae3c1f23e2f6631df1d37fb5#file-inav-and-modern-compilers-md prepared by @stronnag

Related to #8948

Might fix ExBus issues on F722 boards as defined in #8899

@DzikuVx DzikuVx added this to the 6.1 milestone Apr 5, 2023
@ghost
Copy link

ghost commented Apr 5, 2023

Finally, thanks God. So it is now officially clear there is a problem with a compiler GCC 10.x.x OK, I will wait for 6.1 release and try it.

@breadoven
Copy link
Collaborator

Do you think this might be a factor in #8950 ? The gyro behaviour when it decided to do a death spin looks very strange. Raw gyro and filtered don't correlate at all (assuming they should of course) and the values rocket in 1 log in a ms or so and unrealistically drop to 0 for a time in the other other log.

@stronnag
Copy link
Collaborator

stronnag commented Apr 7, 2023

There are examples of people whom I respect advising against using -O3 for mission critical software, for example the Linux kernel:

INAV actually uses, in some places -Ofast, inter alia:

 -Ofast
   Disregard strict standards compliance.  -Ofast enables all -O3
   optimizations.  It also enables optimizations that are not
   valid for all standard-compliant programs.  It turns on
   -ffast-math, -fallow-store-data-races ...

Whether -fallow-store-data-races is applicable to INAV is debatable (ISR, proto-threads, home-brew tasking ...), but it doesn't sound at all nice.

I'd prefer to see consistent optimisation (-O2 or -Os) across everything, rather than piecemeal on specific targets.

@DzikuVx
Copy link
Member Author

DzikuVx commented Apr 7, 2023

@breadoven I'm not really expecting this is the case. We have -Ofast in certain places for the last 3 years. This is not a new thing.
The new thing was putting everything on -Ofast on H7.
Anyhow, let's go back to more standard optimizations with 6.1 and see how it goes

@DzikuVx DzikuVx merged commit e5faa49 into release_6.1.0 Apr 7, 2023
@DzikuVx DzikuVx deleted the dzikuvx-switch-f7-optimizations branch April 7, 2023 07:38
@ghost
Copy link

ghost commented Apr 7, 2023

It is hard to read yours arguing guys. It brings even more distrust. Even if something worked for over 3 yeas but it is not right so why do you keep it like that? I do not understand what are you talking about but clearly understand Jonathans arguments and believe he is right.

Anyway, leave my thoughts without comments. I just need to filter my mind somehow. :)

@stronnag
Copy link
Collaborator

stronnag commented Apr 7, 2023

It is also the case that we've updated the compiler over those releases and the compiler has changed how it implements various optimisation levels. So that we've done something over a number of releases doesn't really matter if the compiler has changed. It is certainly the case that there were significant changes to gcc optimiation between 9.x and later versions.

@DzikuVx
Copy link
Member Author

DzikuVx commented Apr 7, 2023

I'm fully aware of that @stronnag . But even that is not a last moment change. The previous GCC update happened for INAV 4.0 in 2021. After that we stayed with the same GCC and the same optimization levels.

I'm not saying it's not connected or related. I'm saying that I don't expect this is caused by that. It's not just that probable. Could be, but I don't think so.

@ghost
Copy link

ghost commented Apr 7, 2023

Pawel, in one of your latest videos you said: "we (developers) are special". Well, I would not expect from special developer to say words like probably or could be or I don't think so. You live in binary world so there are only 2 possibilities you know it or you don't know it. Simply, probably could think belong to second category. Now I'm being cruel, but it's my plane that I'm literally putting in your hands.

@DzikuVx
Copy link
Member Author

DzikuVx commented Apr 7, 2023

No, we do not live in a binary world. The whole nature of our world is probabilistic. Just look at quantum mechanics.
Some things are more probable, some things are less probable.
When for almost 2 years nothing major changed in some aspects (here see optimization levels on F7 and F4 as in opposite to H7 where something changed), the chance that it caused something today is not very probable. Not impossible. Just not probable.

Interesting see

@RoadyFPV
Copy link
Contributor

RoadyFPV commented Apr 7, 2023

I first reported about it in this issue: #7434

@ghost
Copy link

ghost commented Apr 7, 2023

I am curious why there are only a few people complaining or noticed there is a problem. How many people have H7 and F2 controllers? I would say many but is it really many? How come only a few people have a problem? How many people use iNav compiled in GCC 10.x.x? Maybe not so many and now when v6.0.0 was recommended to everyone the planes will start falling in bigger numbers... Do not take this as a complain, I only have a head full of questions. I crashed twice because of the iNav and I need to know that everything was fixed to feel safe for the next flight. I like iNav but I cannot trust it and it drives me crazy.

@RoadyFPV
Copy link
Contributor

RoadyFPV commented Apr 7, 2023

@DzikuVx how can i test this Version?

@DzikuVx
Copy link
Member Author

DzikuVx commented Apr 7, 2023

@RoadyFPV you can download artifacts from the GitHub CI
image

@RoadyFPV
Copy link
Contributor

RoadyFPV commented Apr 7, 2023

@DzikuVx I don't have any Idea how to get it. Maybe you can compile me a test version for MATEKF722SE?

@DzikuVx
Copy link
Member Author

DzikuVx commented Apr 7, 2023

https://github.com/iNavFlight/inav/actions/runs/4621320316 on the very bottom is a link to all the artifacts from this PR

@fermiums22
Copy link

I crashed twice because of the iNav and I need to know that everything was fixed to feel safe for the next flight. I like iNav but I cannot trust it and it drives me crazy.

Absolutely the same story. One drone has flown away so far has not been found, the second one is now broken =)
But in inav there is an autopilot and in beta there is no so we are torturing inav!

@RoadyFPV
Copy link
Contributor

RoadyFPV commented Apr 7, 2023

@DzikuVx no change, fc is still freezing

@stronnag
Copy link
Collaborator

stronnag commented Apr 7, 2023

@DzikuVx no change, fc is still freezing

Remind me, which FC?

@RoadyFPV
Copy link
Contributor

RoadyFPV commented Apr 7, 2023

MATEKF722SE

@ghost
Copy link

ghost commented Apr 7, 2023

In all the sadness the positive thing is that we know the GCC 10.x.x is the problem

@stronnag
Copy link
Collaborator

stronnag commented Apr 7, 2023

MATEKF722SE

Thanks. Humour me, please try this hex. Alas (for my sanity) it has to be 7.0.0 (if you need a configurator, the examples from http://seyrsnys.myzen.co.uk/inav-configurator-next/ will work).

  • It may not even boot
  • If it does, please bench test (I understand this should be enough).

Please let us know what happens; I really hope it fails miserably.
inav_7.0.0_MATEKF722SE.hex.zip

@ghost
Copy link

ghost commented Apr 7, 2023

Wow, I am completely lost now. From my point of view if I am lucky maybe iNav 8.0.0 might be a version I can take for test flight. Unfortunately I have no idea what iNav 8.0.0 will look like. I am very sorry but iNav now is from my view unusable. The GCC 10.x.x is a huge disappointment...

I feel sorry for developers.

I am happy the iNav 6.0.0 compiled in GCC 9.3.1. is almost perfect for my Dji O3 Air Unit. It does not support HD but I can live with it. I am not able to let the iNav fly my models but at least the telemetry is working, It is expensive one but I am not complaining.

@ghost
Copy link

ghost commented Apr 7, 2023

Last words for today, is it really so hard for developers to compile a hex file for testing instead of provide the link no one instead of developers understands? We could work together but this is not the way...

I am so angry I found iNav, it was so promising but it only drives ma nuts.

@RoadyFPV
Copy link
Contributor

RoadyFPV commented Apr 8, 2023

such contributions help no one here. I can understand you well, it also annoys me very much that this problem exists and have also lost a dart xl...
I am also not a developer and work my way through here and try my best to support the developers.
if you do not understand something you can also ask, then you will certainly be helped.

@stronnag First test is running, lets see what happens.....

@RoadyFPV
Copy link
Contributor

RoadyFPV commented Apr 8, 2023

@stronnag FC Freeze around 40min

@DzikuVx
Copy link
Member Author

DzikuVx commented Apr 8, 2023

@RobinatiNav this is the first and final warning. Either you will start acting with respect towards INAV developers, contributors, and pilots, or I will ban your account and report to GitHub abuse.

Your comments and general attitude in the last few weeks are anything but helpful. Instead of driving the discussion forward and being helpful and respectful to others, you only managed to irritate all the devs that contributed in the GCC and ExBus issues. This is not how issues in the open-source world are solved.

This includes not sending personal emails to INAV core members with demands. Such behavior will not be tolerated as well.

Finally, I would like to remind you a fragment from the INAV license that you agreed to upon downloading:

THERE IS NO WARRANTY FOR THE PROGRAM, TO THE EXTENT PERMITTED BY
APPLICABLE LAW. EXCEPT WHEN OTHERWISE STATED IN WRITING THE COPYRIGHT
HOLDERS AND/OR OTHER PARTIES PROVIDE THE PROGRAM "AS IS" WITHOUT WARRANTY
OF ANY KIND, EITHER EXPRESSED OR IMPLIED, INCLUDING, BUT NOT LIMITED TO,
THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
PURPOSE. THE ENTIRE RISK AS TO THE QUALITY AND PERFORMANCE OF THE PROGRAM
IS WITH YOU. SHOULD THE PROGRAM PROVE DEFECTIVE, YOU ASSUME THE COST OF
ALL NECESSARY SERVICING, REPAIR OR CORRECTION.

IN NO EVENT UNLESS REQUIRED BY APPLICABLE LAW OR AGREED TO IN WRITING
WILL ANY COPYRIGHT HOLDER, OR ANY OTHER PARTY WHO MODIFIES AND/OR CONVEYS
THE PROGRAM AS PERMITTED ABOVE, BE LIABLE TO YOU FOR DAMAGES, INCLUDING ANY
GENERAL, SPECIAL, INCIDENTAL OR CONSEQUENTIAL DAMAGES ARISING OUT OF THE
USE OR INABILITY TO USE THE PROGRAM (INCLUDING BUT NOT LIMITED TO LOSS OF
DATA OR DATA BEING RENDERED INACCURATE OR LOSSES SUSTAINED BY YOU OR THIRD
PARTIES OR A FAILURE OF THE PROGRAM TO OPERATE WITH ANY OTHER PROGRAMS),
EVEN IF SUCH HOLDER OR OTHER PARTY HAS BEEN ADVISED OF THE POSSIBILITY OF
SUCH DAMAGES.

@ghost
Copy link

ghost commented Apr 8, 2023

such contributions help no one here. I can understand you well, it also annoys me very much that this problem exists and have also lost a dart xl... I am also not a developer and work my way through here and try my best to support the developers. if you do not understand something you can also ask, then you will certainly be helped.

@stronnag First test is running, lets see what happens.....

@RobinatiNav this is the first and final warning. Either you will start acting with respect towards INAV developers, contributors, and pilots, or I will ban your account and report to GitHub abuse.

Your comments and general attitude in the last few weeks are anything but helpful. Instead of driving the discussion forward and being helpful and respectful to others, you only managed to irritate all the devs that contributed in the GCC and ExBus issues. This is not how issues in the open-source world are solved.

This includes not sending personal emails to INAV core members with demands. Such behavior will not be tolerated as well.

Finally, I would like to remind you a fragment from the INAV license that you agreed to upon downloading:

THERE IS NO WARRANTY FOR THE PROGRAM, TO THE EXTENT PERMITTED BY
APPLICABLE LAW. EXCEPT WHEN OTHERWISE STATED IN WRITING THE COPYRIGHT
HOLDERS AND/OR OTHER PARTIES PROVIDE THE PROGRAM "AS IS" WITHOUT WARRANTY
OF ANY KIND, EITHER EXPRESSED OR IMPLIED, INCLUDING, BUT NOT LIMITED TO,
THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
PURPOSE. THE ENTIRE RISK AS TO THE QUALITY AND PERFORMANCE OF THE PROGRAM
IS WITH YOU. SHOULD THE PROGRAM PROVE DEFECTIVE, YOU ASSUME THE COST OF
ALL NECESSARY SERVICING, REPAIR OR CORRECTION.

IN NO EVENT UNLESS REQUIRED BY APPLICABLE LAW OR AGREED TO IN WRITING
WILL ANY COPYRIGHT HOLDER, OR ANY OTHER PARTY WHO MODIFIES AND/OR CONVEYS
THE PROGRAM AS PERMITTED ABOVE, BE LIABLE TO YOU FOR DAMAGES, INCLUDING ANY
GENERAL, SPECIAL, INCIDENTAL OR CONSEQUENTIAL DAMAGES ARISING OUT OF THE
USE OR INABILITY TO USE THE PROGRAM (INCLUDING BUT NOT LIMITED TO LOSS OF
DATA OR DATA BEING RENDERED INACCURATE OR LOSSES SUSTAINED BY YOU OR THIRD
PARTIES OR A FAILURE OF THE PROGRAM TO OPERATE WITH ANY OTHER PROGRAMS),
EVEN IF SUCH HOLDER OR OTHER PARTY HAS BEEN ADVISED OF THE POSSIBILITY OF
SUCH DAMAGES.

I am angry with you Pawel and Darren in particular. You never helped me even I asked for it many times. That's why I am so frustrated.

@RoadyFPV
Copy link
Contributor

RoadyFPV commented Apr 8, 2023

@DzikuVx Maybe you can try contacting marv-t, he implemented the Jeti code in BF.
He should maybe have the right Jeti stuff and tools to debug it.
it would be a fine move if he could support us there.

@DzikuVx
Copy link
Member Author

DzikuVx commented Apr 8, 2023

@RobinatiNav you are an adult, you should learn to manage your anger

@stronnag
Copy link
Collaborator

stronnag commented Apr 8, 2023

@stronnag FC Freeze around 40min

Thanks for testing.

@DzikuVx
Copy link
Member Author

DzikuVx commented Apr 8, 2023

@RoadyFPV the thing is that this is not a driver code. Or at least, not directly driver related. Seems to me this is stack management related as ExBus works heavily with Strings.

@RoadyFPV
Copy link
Contributor

RoadyFPV commented Apr 8, 2023

@stronnag you're welcome

@DzikuVx Thanks for the answer, unfortunately I don't understand anything about writing code or the like.

@RoadyFPV
Copy link
Contributor

RoadyFPV commented Apr 8, 2023

Is this helpful?
https://gcc.gnu.org/gcc-10/porting_to.html

what bothers me extremely, as soon as i set the tx frequency to 50Hz the fc doesn't seem to freeze anymore. i wrote an email to Jeti and asked if the tx frequency 50/100Hz affects the exbus output signal on the rx.

But why does it work on GCC9

50Hz refrashrate test is running

@RoadyFPV
Copy link
Contributor

RoadyFPV commented Apr 8, 2023

OK, right now i have a freeze in 50Hz mode too.....

@stronnag
Copy link
Collaborator

stronnag commented Apr 8, 2023

@RoadyFPV the thing is that this is not a driver code. Or at least, not directly driver related. Seems to me this is stack management related as ExBus works heavily with Strings.

I'm not so convinced. I ran a 70 minute BBL through the SITL with JETI TX and telemetry at > 100Hz (each TX packet appending a telemetry request) without any issue or evidence of stack corruption. I appreciate this is a completely different runtime, but it's one with an MMU that is pretty smart at faulting on memory corruption. The recorded telem log also looking consistent (all starting 0x3b, 0x01 etc); telem summary:

479470 items 14223828 bytes 4238.21 secs, 113.13 msg/sec, 3356.09 byte/sec

@DzikuVx
Copy link
Member Author

DzikuVx commented Apr 9, 2023

Could be. This is only my suspicion and they might be wrong.

@RoadyFPV
Copy link
Contributor

RoadyFPV commented Apr 9, 2023

If i disable the Telemetry option the freezes are gone...

seems like there is a timing issue in the tx/rx line?

@stronnag
Copy link
Collaborator

stronnag commented Apr 9, 2023

My view, based on your comment, the SITL experiment, and the behaviour of INAV serial transmission (dump) with GCC > 10 is that there is some bug / race condition under high volume serial transmit with modern GCC abetted by some Cmake influence I don't understand. JETIEXBUS telemetry is somewhat high volume (50Hz, 100Hz, excessive in my opinion). If JETI had a more moderate telemetry rate (like 10Hz), I doubt we'd be even talking about this.

This is seriously difficult to track down; without the hardware that tickles this issue, almost impossible.

@RoadyFPV
Copy link
Contributor

RoadyFPV commented Apr 10, 2023

last idea for testing from my side. i have now installed an resistor betwen rx/tx (2k2) (like the old way).
RX (EXBUS out) now connected to RX (FC), and a resistor between RX/TX on th FC.

Possible to reduce the telemetry update rate in the code?

@RoadyFPV
Copy link
Contributor

Freeze around 11 min with resistor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants