MarlinFirmware / Marlin

Marlin is an optimized firmware for RepRap 3D printers based on the Arduino platform. Many commercial 3D printers come with Marlin installed. Check with your vendor if you need source code for your specific machine.
https://marlinfw.org
GNU General Public License v3.0
16.28k stars 19.23k forks source link

UBL - Studio 7 coaching & tips #6405

Closed Bob-the-Kuhn closed 7 years ago

Bob-the-Kuhn commented 7 years ago

I'd like to split the Studio 7 specific stuff out from Issue #6331.

I'll copy what I think is relevant into this issue.

Bob-the-Kuhn commented 7 years ago

bgort commented 2 days ago

This is great info. Thanks @JustAnother1.

I did find that the Atmel ICE supposedly works with openocd in several places, as it's CMSIS-DAP compliant (whatever that is).

Here's one of the links: http://vk5tu.livejournal.com/56648.html

Hopefully this is true, as I'd like to not have to use Atmel Studio 7 if I don't want to.

Bob-the-Kuhn commented 7 years ago

JustAnother1 commented 2 days ago

So, with that pig tail in the picture up above, can some other J-Tag interface talk to the Atmega-2560 board? Or can only the MK2 do it?

The ATMEL Jtag-ICE (and it's clones) would not be able to talk to it. The mk2 and up (mk3, ATMEL ICE) are able to talk to it. The reason is that Atmel Studio reads the ID Bytes of the chips and then decides to not work with them if the open Hardware ice is connected. The reason seems to be just them being nice.

Openocd supported boards should also be able to talk to the chip.

There might be an issue though. The AVR has fuse bits. and one fuse bits disables the JTAG interface. But all the Debuggers also have ISP. And via ISP the JTAG Fuse bit can be enabled. On my Arduino 2560 the JTAG interface was disabled and I had to use the ISP interface to enable it before I could use the JTAG interface. On my Rumba board the JTAG was enabled. YMMV

Bob-the-Kuhn commented 7 years ago

JustAnother1 commented 2 days ago • edited

@Bob-the-Kuhn Yes I use Atmel Studio7 (I have also used the older versions) to debug.

As to tutorial: There is nothing that directly comes to mind. But I'm not aware what the issues are that on your learning curve. It probably makes most sense to add all information about how to compile and set up debugging to the Marlin documentation. This way others can also benefit from it. I think there is a Wiki somewhere. If you start a page and send me a link I will take a look and fill in the gaps if I can.

@Roxy-3D One handy trick if compiling with Atmel Studio is an issue:

In Atmel studio create a project and add the source. Then instead ob build just click on debug. It will complain that the *.elf file is missing. Take the already compiled elf file and put it at the place named and click debug again. Atmel Studio will then download that elf file into the chip and will start debugging it. If the source files are at the right position everything should work fine.

Bob-the-Kuhn commented 7 years ago

JustAnother1 commented 2 days ago

How hard is it to get Marlin to compile inside Atmel Studio?

That depends on how much Marlin depends on the Arduino specific stuff. I have converted this firmware https://github.com/minnow-pmc/Minnow from Arduino to non Arduino.

For all the Arduino libraries that are used you need to copy the .c and .h files into your project. Arduino then also helps with missing includes. If the code relies on Arduino IDE to add include statement then you have to also add them.

But instead of letting Atmel Studio create a new make file (that it then hides) you can also configure Atmel Studio to just execute an already existing makefile. If building Marlin using the Marlin provided makefile works then that should also be enough to get Atmel Studio to debug it.

Bob-the-Kuhn commented 7 years ago

bgort commented 2 days ago • edited

FYI, there's a free addon for Atmel Studio 7 for Arduino ('Arduino IDE for Atmel Studio 7') that, I believe, provides all of the libraries, etc., and what looks like a limited version of the visualmicro.com Arduino debugger? I haven't tried any of this yet, but it's there and it installed fine.

There's also a pro version for $29 one-time (non-profit/non-commercial price) that adds a bunch of features that look worthwhile ('advanced memory report', 'almost real-time free memory report', etc.). Will reply here when I've tried it all out. EDIT: Looks like my ATMEL ICE won't be here until Monday, as Mouser used almost-always-horribly-slow UPS Mail Innovations; that's what I get for choosing the cheapest option.

Bob-the-Kuhn commented 7 years ago

I was able to use the Arduino IDE for Atmel Studio 7 to compile & upload RCBugFix. I did have to make the following change to get it to compile:

added the following to C:\Users\Bob\AppData\Local\Arduino15\packages\arduino\hardware\avr\1.6.18\cores\arduino\main.cpp
          extern void setup();
          extern void loop();

I got to the point of adding breakpoints when I found out that it's debugging features were VERY limited. Basically what you do is highlight the source line and then click the breakpoint icon and it adds code immediately after. You then re-compile & upload. You can monitor memory locations also using this method. You can have as many breakpoints as you like.

Here's the link to their debugging page and one that goes into the differences between it and what most people are used to.

I never actually did any breakpoints. Studio 7 wouldn't let me edit any of the source files. I've posted a message asking for help on that.

The really nice thing is the documentation is very detailed. It helped me to understand Studio 7 a lot better.

I'm pretty sure that we'll need the JTAG debugger so I've abandoned this path.

Bob-the-Kuhn commented 7 years ago

Wonder of wonders!! I've actually been able to use the JTAG interface to upload code & set a breakpoint!

Now I need to understand the map and the memory structure and learn a lot more about debugging.

Roxy-3D commented 7 years ago

Wow!!!! That is amazing!!!!

bgort commented 7 years ago

Fantastic!

JustAnother1 commented 7 years ago

@Bob-the-Kuhn

Wonder of wonders!! I've actually been able to use the JTAG interface to upload code & set a breakpoint!

This is still with the Arduino Plug-in or with "pure" Atmel Studio 7 ?

What was the issue that you had to overcome to be able to do JTAG + Breakpoints?

Bob-the-Kuhn commented 7 years ago

This is with the "pure" Atmel Studio 7.

The Arduino Plug-in doesn't support any debuggers. It just adds code after a statement of interest and then you need to re-compile.

Roxy-3D commented 7 years ago

I can't wait until you get your J-Tag gizmo! I want to hear you can set break points and look at things at the source level!

JustAnother1 commented 7 years ago

@Bob-the-Kuhn Do you use Atmel Studio to compile Marlin ? If so could you send me a link to the code you use. I already have the "J-Tag gizmo" and could start taking a look,..

Bob-the-Kuhn commented 7 years ago

Here's my entire directory structure . It contains the Studio 7 project info so you should be able to just open it.

Also attached are the steps I took to get to this point. debug log.txt

The debug image doesn't have the strange line numbers. The only unusual item I've seen is G26 locks up the system.

EDIT - turns out it eventually comes back. I could have sworn that G26 produces a series of prints telling you what it was doing.

Bob-the-Kuhn commented 7 years ago

?? I just downloaded it. It's 13.8MB

Please try again.

Also, it's RCBugFix from late 19 APR.

Roxy-3D commented 7 years ago

I'll get Studio-7 installed...

JustAnother1 commented 7 years ago

@Bob-the-Kuhn thank you for the files. I could compile it and upload it to an Arduino2560 (no Printer connected). I have changed the baud-rate to 115200 and the serial interface works.

Debugging also works. Setting break points, starting, stopping, single stepping.

Suggestions for good breakpoint are welcome ;-)

Roxy-3D commented 7 years ago

I wish you had a printer connected! I would give you a Configuration.h file that causes the Stack Pointer to get corrupted after a G28 as soon as you start doing a G29 with no command parameters.

JustAnother1 commented 7 years ago

I could put the code onto a Prusa i3 clone using a Rumba board. If you have the configuration for that and the breakpoint Ideas,...

Roxy-3D commented 7 years ago

I think that would be fine. Do you have a configuration that has a Z-Probe? Because then all's we do is turn on UBL with a 15x15 mesh. We probably don't even need to probe or build a mesh. We also turn on M100, BabyStepping and FilamentChange. I think you will see the problem very quickly.

To verify you can cause the problem, you would reset, do M100 I, G28, and I think a G29 (with no command parameters) is going to cause the stack pointer corruption. If so.... You would just repeat the those 3 commands, and break point at the G29 to start trying to figure out what is causing the problem. However... If it is happening in an interrupt routine... there will be a level of indirection. We will see things getting sick after G29 changes something that the LCD or thermal code uses.

JustAnother1 commented 7 years ago

I have no printer with a Z Probe. I also haven't found where I would place a Breakpoint for "G29". I'm in this tread because I know something about JTAG, not about Bed leveling,..

And then the Bigger issue: There is something wrong with the serial line. Most times it just sends gibberish. Might have something to do with logic level before first byte send. I validated with a Logic Analyzer and it also can not decode it. It worked one Time. Don't know what I did differently on that one though. Am I the only one seeing that at 115200 ?

Maybe we better wait for @Bob-the-Kuhn 's Atmel ICE to arrive. The setup is confirmed working, so he should be up and running as soon as the parcel arrives,...

Roxy-3D commented 7 years ago

I don't know what happens if you try to turn on UBL without a Z-Probe. But UBL with a big mesh seems to cause the problem. The big problem is you don't know how it will manifest itself.

But the break point for G29 is simple. The start of the routine is: gcode_G29() And then depending upon what sub-function is required (if any)... I can add more specifics.

Bob-the-Kuhn commented 7 years ago

@Roxy-3D - please post your configuration files so I can try them.

@JustAnother1 - I have the Dragon debugger. I do not have an ICE on order. I don't think anyone does. A couple of us have the cheap Chinese MK2 clones on the way. Those come out of Hong Kong or China so it could be 6 weeks before we see them.

A breakpoint to start - try line 317 in ubl_G29.cpp. It's the gcode_G29() function.

We'll need to enable the M100 command & re-compile. The compiler put the last set of files under the DEBUG directory. I had to copy copy them up one level before I could upload it to the target via the JTAG interface.

Roxy-3D commented 7 years ago

Here you go: But this is for my gMax with a 400mm x 400mm bed. It may be prudent to bump the size (and inset) down. It does have a BL-Touch so, at least that code isn't going to be shifting around for you.

Configuration_adv-h.txt Configuration-h.txt _Bootscreen-h.txt

With these files... I would do a

and the stack was corrupted. If you can get that failure mode to happen... Setting a break point at gcode_G29 and stepping over all the functions calls in gcode_G29() is going to be VERY informative! Hopefully you get the stack pointer moving, because being able to single step is going to tell us what function is doing the dirty deed.

JustAnother1 commented 7 years ago

@Bob-the-Kuhn Thanks for the breakpoint location. But that won't help before the UART gets fixed, as I can not send any commands. If I send something I don't see anything getting replied.

Does the Dragon support the mega2560?

bgort commented 7 years ago

@JustAnother1 - I have the Dragon debugger. I do not have an ICE on order. I don't think anyone does. A couple of us have the cheap Chinese MK2 clones on the way. Those come out of Hong Kong or China so it could be 6 weeks before we see them.

I have an ICE on the way; it'll be here Monday.

Bob-the-Kuhn commented 7 years ago

I have NOT been able to get Roxy's configuration to fail when using Studio 7 with the 'Arduino IDE for Atmel Studio 7' add in. I added in a few more options to reduce the available memory & still couldn't get it to fail.

I was able to get really strange line numbers when I used the Arduino environment to compile it.

I'm wondering if the problem is the older Arduino compiler (4.9.2) vs. Studio 7 (5.4.2) or some of the compiler flags

I was able to finally get the failing Arduino image into an active Studio 7 debugging session. Unfortunately I couldn't do anything beyond stopping & starting the target because I can't supply it a useful disassembly listing nor a useful symbol table. Can't set breakpoints, can't single step.

bgort commented 7 years ago

My Atmel-ICE arrived early yesterday so I ended up playing with Atmel Studio 7 for a while. Feedback-wise, and for whatever it's worth, the ICE seems to be really well done and I like it, so far.

Anyway, I was able to get a disassembly out of AS7 only after starting a generic 'GCC {C,C++} Executable Project', as opposed to an 'Ardunio Sketch'. It seems that someone somewhere disabled or somehow prevented the compiler's disassembly output (the .lss and other intermediate files) from being created for Arduino projects, even though the associated options are enabled in the project properties/options. I had the same problem with debugging, in that without the disassembly and such, I couldn't do much of anything. I have yet to try Marlin in AS7, though I'm expecting a (debugging) 'fight' if I use the 'Arduino Sketch' project formats; I haven't tried the 'Create project from Arduino Sketch' option that I believe comes with AS7, but maybe that will work better?

Also, I'm using platformio as opposed to Arduino for compiling and flashing, generally, and still see the problem; I don't think it relies on the compiler Arduino distributes, but rather just my system's avr-g++, which on the Rpi3 I'm using is v4.8.1. I'll see about upgrading and will try a known bad configuration shortly. Hopefully this is all it is, but I guess that creates another problem...

EDIT: Looks like I can't upgrade the gcc on this Debian image I'm using on the RPi - no newer versions available. I've been wanting to use my preferred Gentoo for a while for other reasons and on a larger uSD card, so will move over to it today. Probably take a good portion of the day to get there, though.

EDIT2: I was wrong about platformio. It's using its own toolchain, avr-gcc/g++, etc.

JustAnother1 commented 7 years ago

I was able to do single stepping and all with the directory supplied by Bob-the-Kuhn. But yes if the generation of debug information is disabled during building then debugging can only show the assembly,...

Another option for updating the avr-gcc is to compile it from source. The avr-libc web page has a very good step by step explanation of how to do it.

Marlin has a Makefile. Using the makefile should solve all the "Arduino Sketch does not generate debug information" issue,...

bgort commented 7 years ago

Thank you. Yeah, I've already moved to Gentoo on my RPi, compiled gcc (5.4.0) for avr, and am pretty well set up, etc. Am also looking at the Makefile (which seems pretty stale) and Marlin's cmake stuff.

Keep getting sidetracked, but will get back into it shortly.

Bob-the-Kuhn commented 7 years ago

I've finally made progress. Using RCBugFix from this evening and Roxy's config files I was able to see the stepper ISR pushing registers unto the stack but the stack was at least 200 bytes away from where it had been previously.

I got one screen shot of the data before Studio 7 crashed. isr stepping on unused ram

Notice that there is virgin memory before and after the ISR's register dump. My best guess is some routine modified the stack pointer just before enabling interrupts.

Here's the memory data from the corrupted section. ISR corrupted memory.xlsx.txt

Studio 7 is back up. Here's a second memory capture. Same locations, some data has changed. Memory dump 2nd capture.txt

The PC was at 0x0ac82 when the break point was hit. Here's a disassembly of that area. Disassembly 0x0ac82.txt

The PC of the interrupted routine should be in the first bytes. I'll see if I can match up a disassembly with the PC.

Here's a link to the ELF file. https://www.dropbox.com/s/etuvugg07xlnah2/temp.ino.elf.zip?dl=0

And one to the symbol file. Note that RAM & registers start at 0x0800000. https://www.dropbox.com/s/lt11rq1vhe5yibv/symbols.zip?dl=0

Here's a link to my Studio 7 project. I'm running the pro version of Arduino for Studio 7 with the IDE pointed to my Arduino 1.8.1 directory. You'll need to delete the "copy" part of the top level directory before it'll be useful. My path was c:\work\Studio_7_Arduino_Roxy_1A

https://www.dropbox.com/s/igwpc2860daxb48/Studio_7_Arduino_Roxy_1A%20-%20Copy.zip?dl=0

Bob-the-Kuhn commented 7 years ago

Here's the disassemblies for all the possible 3 byte PC values I could pull out of the memory dumps.

I didn't see anything that looked like an interrupt enable near any of them.

One aggravating item is the Studio 7 disassembles use word addresses for the PC while the one file that has the entire FLASH disassembled uses byte addresses.

All the disassemblies have the source code line numbers in them.

Roxy-3D commented 7 years ago

Notice that there is virgin memory before and after the ISR's register dump. My best guess is some routine modified the stack pointer just before enabling interrupts.

Yes! That is one of the symptoms I was seeing. And I have a couple of observations...

Notice that there is virgin memory before and after the ISR's register dump. My best guess is some routine modified the stack pointer just before enabling interrupts.

Maybe... I don't know what the failure mechanism is. But right now I'm thinking we have a bad pointer that is clobbering the 0x3D and 0x3E area. That is the location where the Stack Pointer is kept. And in fact, it might be just hammering the high byte of the stack pointer because that would be more likely to 'displace' the stack in a new area like we are seeing.

Also... it might be good to dump the entire RAM for two reasons

GOOD WORK BOB!!!!! WE FINALLY HAVE A WAY TO GET A HANDLE ON THIS BUG!!!!

JustAnother1 commented 7 years ago

Good work Bob!

Just a tip for the search. One should always also look at the decimal value of the numbers one encounters. Some values are really "magical". In that they pint to something very specific. And usually one knows these special values only in decimal or hexadecimal. 0x3d is 61, 0x3e is 62. Does these numbers ring a bell? If not then go on searching,...

Roxy-3D commented 7 years ago

OK! I typed more than I needed to.... Bob already had his blood hounds sniffing down the path I was thinking should be explored! :) He provided a disassembly file with 0x4366 in it. And what is there? UBL's G29 is calling lcd_quick_feedback(); But as the theory goes... That call is AFTER the corruption happened. So it must be a little further up the code that the problem happened???

  void gcode_G29() {
00004333  PUSH R2       Push register on stack 
00004334  PUSH R3       Push register on stack 
00004335  PUSH R4       Push register on stack 
00004336  PUSH R5       Push register on stack 
00004337  PUSH R6       Push register on stack 
00004338  PUSH R7       Push register on stack 
00004339  PUSH R8       Push register on stack 
0000433A  PUSH R9       Push register on stack 
0000433B  PUSH R10      Push register on stack 
0000433C  PUSH R11      Push register on stack 
0000433D  PUSH R12      Push register on stack 
0000433E  PUSH R13      Push register on stack 
0000433F  PUSH R14      Push register on stack 
00004340  PUSH R15      Push register on stack 
00004341  PUSH R16      Push register on stack 
00004342  PUSH R17      Push register on stack 
00004343  PUSH R28      Push register on stack 
00004344  PUSH R29      Push register on stack 
--- C:\Users\Bob\AppData\Local\Temp\VMBuilds\temp\mega_atmega2560\Release/ubl_G29.cpp 
00004345  IN R28,0x3D       In from I/O location 
00004346  IN R29,0x3E       In from I/O location 
00004347  SUBI R28,0xBC     Subtract immediate 
00004348  SBCI R29,0x04     Subtract immediate with carry 
00004349  IN R0,0x3F        In from I/O location 
0000434A  CLI       Global Interrupt Disable 
0000434B  OUT 0x3E,R29      Out to I/O location 
0000434C  OUT 0x3F,R0       Out to I/O location 
0000434D  OUT 0x3D,R28      Out to I/O location 
  }
0000434E  IN R18,0x3D       In from I/O location 
0000434F  IN R19,0x3E       In from I/O location 
00004350  SUBI R28,0x49     Subtract immediate 
00004351  SBCI R29,0xFB     Subtract immediate with carry 
00004352  STD Y+1,R19       Store indirect with displacement 
00004353  STD Y+0,R18       Store indirect with displacement 
00004354  SUBI R28,0xB7     Subtract immediate 
00004355  SBCI R29,0x04     Subtract immediate with carry 
    if (!code_seen('N') && axis_unhomed_error(true, true, true))  // Don't allow auto-leveling without homing first
00004356  LDI R24,0x4E      Load immediate 
00004357  CALL 0x0000B48D       Call subroutine 
00004359  CPSE R24,R1       Compare, skip if equal 
0000435A  RJMP PC+0x0005        Relative jump 
0000435B  CALL 0x0001121C       Call subroutine 
0000435D  CPSE R24,R1       Compare, skip if equal 
0000435E  RJMP PC+0x0285        Relative jump 
      LCD_MESSAGEPGM("Doing G29 UBL!");
0000435F  LDI R22,0x00      Load immediate 
00004360  LDI R24,0xF7      Load immediate 
00004361  LDI R25,0x10      Load immediate 
00004362  CALL 0x00009044       Call subroutine 
      lcd_quick_feedback();
00004364  CALL 0x000108F4       Call subroutine 
    x_flag = code_seen('X') && code_has_value();
00004366  LDI R24,0x58      Load immediate 
00004367  CALL 0x0000B48D       Call subroutine 
00004369  CPSE R24,R1       Compare, skip if equal 
--- C:\Users\Bob\AppData\Local\Temp\VMBuilds\temp\mega_atmega2560\Release/ubl_G29.cpp 
0000436A  CALL 0x0000B526       Call subroutine 
0000436C  STS 0x06B1,R24        Store direct to data space 
    x_pos = x_flag ? code_value_float() : current_position[X_AXIS];
0000436E  TST R24       Test for Zero or Minus 
0000436F  BREQ PC+0x04      Branch if equal 
00004370  CALL 0x0000B4F4       Call subroutine 
00004372  RJMP PC+0x0009        Relative jump 
Roxy-3D commented 7 years ago

In the previous post, why is the code messing with the stack frame at 0000434B ? I guess it is setting up the stack frame for the function, but this is very close to where the stack gets corrupted!!!!

00004345  IN R28,0x3D       In from I/O location 
00004346  IN R29,0x3E       In from I/O location 
00004347  SUBI R28,0xBC     Subtract immediate 
00004348  SBCI R29,0x04     Subtract immediate with carry 
00004349  IN R0,0x3F        In from I/O location 
0000434A  CLI       Global Interrupt Disable 
0000434B  OUT 0x3E,R29      Out to I/O location 
0000434C  OUT 0x3F,R0       Out to I/O location 
0000434D  OUT 0x3D,R28      Out to I/O location 

If we can set code break points... It might be possible to see the corruption happen by break pointing at gcode_G29 and stepping down towards 0x4366.

Bob-the-Kuhn commented 7 years ago

I've found the culprit but I haven't figured out the fix.

Here's the problem code. It's the IN and the OUT commands that are the issue.

--- C:\Users\Bob\AppData\Local\Temp\VMBuilds\temp\mega_atmega2560\Release/ubl_G29.cpp 
  void gcode_G29() {
00004333  PUSH R2       Push register on stack 
00004334  PUSH R3       Push register on stack 
00004335  PUSH R4       Push register on stack 
00004336  PUSH R5       Push register on stack 
00004337  PUSH R6       Push register on stack 
00004338  PUSH R7       Push register on stack 
00004339  PUSH R8       Push register on stack 
0000433A  PUSH R9       Push register on stack 
0000433B  PUSH R10      Push register on stack 
0000433C  PUSH R11      Push register on stack 
0000433D  PUSH R12      Push register on stack 
0000433E  PUSH R13      Push register on stack 
0000433F  PUSH R14      Push register on stack 
00004340  PUSH R15      Push register on stack 
00004341  PUSH R16      Push register on stack 
00004342  PUSH R17      Push register on stack 
00004343  PUSH R28      Push register on stack 
00004344  PUSH R29      Push register on stack 
--- C:\Users\Bob\AppData\Local\Temp\VMBuilds\temp\mega_atmega2560\Release/ubl_G29.cpp 
00004345  IN R28,0x3D       In from I/O location 
00004346  IN R29,0x3E       In from I/O location 
00004347  SUBI R28,0xBC     Subtract immediate 
00004348  SBCI R29,0x04     Subtract immediate with carry 
00004349  IN R0,0x3F        In from I/O location 
0000434A  CLI       Global Interrupt Disable 
0000434B  OUT 0x3E,R29      Out to I/O location 
0000434C  OUT 0x3F,R0       Out to I/O location 
0000434D  OUT 0x3D,R28      Out to I/O location 
  }
0000434E  IN R18,0x3D       In from I/O location 

I think this is a "byte addresses vs. word addresses" issue in the Arduino library/code.

They're playing with the following registers and using the larger numbers as the address.

0x1F (0x3F) EECR 
0x1E (0x3E) GPIOR0
0x1D (0x3D) EIMSK

The problem is the 0x3E & 0x3D is also sometimes used to address the stack pointer.

0x3E (0x5E) SPH 
0x3D (0x5D) SPL

What's happening is they're reading in values into the Y register (R28 & R29) and then later on writing those values out to addresses 0x3D & 0x3E.

These screenshots show the stack pointer getting written to.

At the start the stack pointer has a reasonable value.

capture ok

R28 gets an initial value

capture r28 in

R29 gets an initial value

capture 29 in

R28 final value

capture r28 final value

R29 final value

capture r29 final value

Stack pointer high has been written to

capture sp high changed

Stack pointer low has been written to.

capture sp low changed

Roxy-3D commented 7 years ago

I don't want to put these words in your mouth... But it almost seems like you are saying "Compiler Bug!"

I don't understand what it is trying to accomplish with this sequence. Is it trying to create a stack frame of 0x4bc in size? AND... what is 0x3F's role in this logic? What is the compiler trying to accomplish with the read from 0x3F into R0 ? Is it reading port 0x3f into R0 and then writing that same value back out to port 0x3F ? And if that is valid... Why is it in the middle of the stack pointer update?

00004345  IN R28,0x3D       In from I/O location 
00004346  IN R29,0x3E       In from I/O location 
00004347  SUBI R28,0xBC     Subtract immediate 
00004348  SBCI R29,0x04     Subtract immediate with carry 
00004349  IN R0,0x3F        In from I/O location 
0000434A  CLI       Global Interrupt Disable 
0000434B  OUT 0x3E,R29      Out to I/O location 
0000434C  OUT 0x3F,R0       Out to I/O location 
0000434D  OUT 0x3D,R28      Out to I/O location 

Right now... without a better understanding... It kind of appears it is trying to setup a stack frame of 0x4bc of size for G29 local variables. But if you look through the gcode_G29() function, it doesn't use very much storage. It calls some functions that use more storage. but gcode_G29() doesn't need 1200 bytes of local storage??? (I just went through it line by line...)

Is it possible to have the compiler give us the details of what the stack frame looks like for each function? It sure would be interesting to see what it thinks is allocated at each location on the gcode_G29() stack frame.

@Bob-the-Kuhn One more question... I tried to look at the disassembly listings you provided. But they didn't go far enough.

Roxy-3D commented 7 years ago

I may be on to something.... On the surface, that 0x4bc size stack frame doesn't make any sense because gcode_G29() does not use any where near that much variable space. But look at this... The compiler is in-lining the 'W' (What) command inside the gcode_G29() function. The What_Command() is only used by and called by gcode_G29(). Is the compiler trying to eliminate stand alone functions? But in order to do that, it has to increase the stack frame of the function where it is being pulled into?


    //
    // Much of the 'What?' command can be eliminated. But until we are fully debugged, it is
    // good to have the extra information. Soon... we prune this to just a few items
    //
    if (code_seen('W')) g29_what_command();
    b108:   87 e5           ldi r24, 0x57   ; 87
    b10a:   0e 94 8d b4     call    0x1691a ; 0x1691a <code_seen(char)>
    b10e:   88 23           and r24, r24
    b110:   09 f4           brne    .+2         ; 0xb114 <gcode_G29() [clone .part.5] [clone .lto_priv.192]+0x2aae>
    b112:   1b c2           rjmp    .+1078      ; 0xb54a <gcode_G29() [clone .part.5] [clone .lto_priv.192]+0x2ee4>
g29_what_command():
C:\Users\Bob\AppData\Local\Temp\VMBuilds\temp\mega_atmega2560\Release/ubl_G29.cpp:1140
  /**
   * Much of the 'What?' command can be eliminated. But until we are fully debugged, it is
   * good to have the extra information. Soon... we prune this to just a few items
   */
  void g29_what_command() {
    const uint16_t k = E2END - ubl.eeprom_start;
    b114:   80 91 0c 02     lds r24, 0x020C ; 0x80020c <unified_bed_leveling::eeprom_start>
    b118:   2f ef           ldi r18, 0xFF   ; 255
    b11a:   3f e0           ldi r19, 0x0F   ; 15
    b11c:   79 01           movw    r14, r18
    b11e:   e8 1a           sub r14, r24
    b120:   f1 08           sbc r15, r1
    b122:   87 fd           sbrc    r24, 7
    b124:   f3 94           inc r15
    b126:   eb e9           ldi r30, 0x9B   ; 155
    b128:   fb e0           ldi r31, 0x0B   ; 11
serialprintPGM():
Bob-the-Kuhn commented 7 years ago

I have no idea how Marlin works!! There's 122 of these "write to stack pointer" groups in the Marlin code.

In the ZIP file is a disassembler output for the entire FLASH and a list of the 122 groups.

There's a twist to this disassembler output. It uses byte addressing. The ones in the previous posts used word addressing.

Roxy-3D commented 7 years ago

When I found the 'W'-hat Command thing... I was really after something else. I was looking to see if the end of the gcode_G29() command subtracted the 0x4bc stack frame off of the stack at the very end. I'm having trouble finding the end of the gcode_G29() function because of all the code it pulled into it.

Roxy-3D commented 7 years ago

So... I'm jumping ahead a little bit... But if we compile ubl_G29.cpp with no optimizations, I bet it doesn't inline all those support functions. And as a result, the stack frame of gcode_G29() doesn't have to be the sum total of what it needs along with the inlined functions that get pulled in. I think there is a fair chance if we either turn off all optimization for ubl_G29.cpp or at least figure out how to tell it to not inline anything... The problem goes away????

Roxy-3D commented 7 years ago

There are pair of "write to stack pointer" entries in your list for the end of gcode_G29(). Check this out the 0xbb2a and 0xbb2c entry. That is where it is removing the 0x4bc stack frame from the start of the function. I think we know what is going on... We need to figure out how to tell the compiler not to inline stuff in the gcode_G29() function. We can't afford the stack space to handle all of the functions it can possibly call at the same time. We need to only incur the stack space usage as we need it. Not everything possible all at once and during the whole function!!!

I smell an 'attribute' discussion soon!

(Although... If it is trying to remove the 0x4bc stack frame, shouldn't it be doing add's instead of subtracts??? I don't understand the AVR assembly code very well. But very clearly, this is the code it uses to end the gcode_G29() function. It is doing all the corresponding POP's and a return. But now I don't see the push's that were at the start of G29 before in the other listing. Are you using several different version of the compiler?)

    lcd_reset_alert_level();
    LCD_MESSAGEPGM("");
    lcd_quick_feedback();

    ubl.has_control_of_lcd_panel = false;
  }
    bb22:   c9 54           subi    r28, 0x49   ; 73
    bb24:   db 4f           sbci    r29, 0xFB   ; 251
    bb26:   68 81           ld  r22, Y
    bb28:   79 81           ldd r23, Y+1    ; 0x01
    bb2a:   c7 5b           subi    r28, 0xB7   ; 183
    bb2c:   d4 40           sbci    r29, 0x04   ; 4
    bb2e:   0f b6           in  r0, 0x3f    ; 63
    bb30:   f8 94           cli
    bb32:   7e bf           out 0x3e, r23   ; 62
    bb34:   0f be           out 0x3f, r0    ; 63
    bb36:   6d bf           out 0x3d, r22   ; 61
    bb38:   c4 54           subi    r28, 0x44   ; 68
    bb3a:   db 4f           sbci    r29, 0xFB   ; 251
    bb3c:   0f b6           in  r0, 0x3f    ; 63
    bb3e:   f8 94           cli
    bb40:   de bf           out 0x3e, r29   ; 62
    bb42:   0f be           out 0x3f, r0    ; 63
    bb44:   cd bf           out 0x3d, r28   ; 61
    bb46:   df 91           pop r29
    bb48:   cf 91           pop r28
    bb4a:   1f 91           pop r17
    bb4c:   0f 91           pop r16
    bb4e:   ff 90           pop r15
    bb50:   ef 90           pop r14
    bb52:   df 90           pop r13
    bb54:   cf 90           pop r12
    bb56:   bf 90           pop r11
    bb58:   af 90           pop r10
    bb5a:   9f 90           pop r9
    bb5c:   8f 90           pop r8
    bb5e:   7f 90           pop r7
    bb60:   6f 90           pop r6
    bb62:   5f 90           pop r5
    bb64:   4f 90           pop r4
    bb66:   3f 90           pop r3
    bb68:   2f 90           pop r2
    bb6a:   08 95           ret
JustAnother1 commented 7 years ago

I smell an 'attribute' discussion soon!

It might be easier to move the "non-inline" functions to a separate file. This way the compiler can not inline them.

Roxy-3D commented 7 years ago

I smell an 'attribute' discussion soon!

It might be easier to move the "non-inline" functions to a separate file. This way the compiler can not inline them.

Yes... I was wondering about this. But here is the question I don't understand. How can it inline these functions when it doesn't know if some other files are calling the function? As it turns out... nobody else is calling the what_command(). But if it did... It can't be pulled into the gcode_G29() function. At least... that can't be the only instance of it.

More thinking... If we just turned off optimization for void gcode_G29() that would not hurts at all. The function doesn't do anything but decide what sub-commands were invoked and call them. There is very little room for optimization in that logic. And then... everything else could still have the full optimization performed on it. Splitting things out to separate files might help the problem. But breaking apart the functions that are only used with each other just makes things messier.

void __attribute__((optimize("O0"))) gcode_G29() {
JustAnother1 commented 7 years ago

How can it inline these functions when it doesn't know if some other files are calling the function?

The compiler is not allowed to inline functions that may be used from outside the file. But if the function is static then it can safely assume that it can inline it.

Splitting things out to separate files might help the problem. But breaking apart the functions that are only used with each other just makes things messier.

Moving the utility function used in G29 into a file called g29_util.c is IMHO not messier than having all of the G29 function in one file. Just shoving them around into other files that have nothing to do with G29 would be a big mess. But that is not my suggestion.

Splitting files that grow and grow into smaller files seems to me to be a natural way of keeping the code clean.

Roxy-3D commented 7 years ago

Good information! Yes, I guess I agree on the G29_util.cpp idea. And the suggestion about 'static' things does make sense. But void g29_what_command() is not declared static. So why does the compiler think this is OK to do?

Bob-the-Kuhn commented 7 years ago

I don't see this as a G29 issue. I checked another of the groups and it also changes the stack pointer.

All of these groups are associated with a CLI instruction that disables all interrupts.

Roxy-3D commented 7 years ago

I don't see this as a G29 issue. I checked another of the groups and it also changes the stack pointer. All of these groups are associated with a CLI instruction that disables all interrupts.

Yes... the fact that they would do a CLI makes sense if I understand things. If they are going to change the value of the stack pointer because they are creating a stack frame for local variables (or removing that storage at the end of a function call), they need the operation to be 'atomic'. They can't get halfway through it and get an interrupt with the stack pointer only halfway updated.

The CLI makes perfect sense during this operation... The thing that doesn't make sense is I don't see a corresponding SEI to turn interrupts back on following that.

JustAnother1 commented 7 years ago

But void g29_what_command() is not declared static. So why does the compiler think this is OK to do?

Because it is a cpp file! The static thing is for C only. In C++ everything not declared as external C is a C++ class. And functions in a C++ class can only be called from anywhere else if the function is a public (or protected) function of that class.

The prototype for the function is in ubl.h, but outside of the class definition. And ubl_G29.cpp doesn't even include that header file, so the compiler doesn't know about the definition.

In conclusion nobody outside the file can call the function so the compiler is free to inline it.

If you are interested in keeping the code clean then a good convention is to keep files having less than 1000 lines.