From: Aaron C. <mar...@ho...> - 2004-08-29 07:34:30
|
I'm not sure what the process is for submitting patches to the compiler, so I'm posting my fix on this list. If there is a more appropriate place, please let me know. I've spent the last 2 days trying to figure out why my simple program that sends characters from a PIC to my PC's serial port would randomly exhibit strange behavior. I discovered that the high priority interrupt handler was not restoring the WREG, BSR, and STATUS registers. After looking at the compiler source I realized that this was done by design. It was taking advantage of the fact that these registers get stored on the fast register stack when a high priority interrupt occurs. The problem was that the compiler was not generating the proper return instruction so that these registers would be restored from the fast register stack. This caused random behavior in my program because the interrupt handler changed these 3 registers and they wouldn't get restored when the interrupt handler would exit. I've attached a fix below that causes the compiler to generate the proper return instruction. I have not studied the compiler source very much, so I'm not sure if what I did was the "right" way to do this. It seems to generate the proper assembly instruction for me so I figure that it should be close to the correct solution. If you have any questions or comments, please let me know. I hope this helps. Aaron Index: pic16/gen.c =================================================================== RCS file: /cvsroot/sdcc/sdcc/src/pic16/gen.c,v retrieving revision 1.42 diff -u -r1.42 gen.c --- pic16/gen.c 24 Jul 2004 11:25:54 -0000 1.42 +++ pic16/gen.c 29 Aug 2004 06:59:46 -0000 @@ -3663,6 +3663,9 @@ #endif if (IFFUNC_ISISR(sym->type)) { + /* Initially assume the fast register stack is being used */ + int doFastReturn = 1; + /* now we need to restore the registers */ /* if any registers used */ if (sym->regsUsed) { @@ -3699,6 +3702,9 @@ pic16_poppCodeOp( pic16_popCopyReg( &pic16_pc_bsr )); pic16_poppCodeOp( pic16_popCopyReg( &pic16_pc_status )); pic16_poppCodeOp( pic16_popCopyReg( &pic16_pc_wreg )); + + /* Signal that the fast register stack is not being used */ + doFastReturn = 0; } _G.interruptvector = 0; /* sanity check */ @@ -3711,8 +3717,15 @@ if (currFunc) { debugFile->writeEndFunction (currFunc, ic, 1); } - - pic16_emitpcodeNULLop(POC_RETFIE); + + if (doFastReturn) { + /* generate an RETFIE that restores registers from the + * fast register stack */ + pic16_emitpcode(POC_RETFIE, pic16_newpCodeOpLit(1)); + } + else { + pic16_emitpcodeNULLop(POC_RETFIE); + } } else { if (IFFUNC_ISCRITICAL(sym->type)) pic16_emitcode("setb","ea"); _________________________________________________________________ Is your PC infected? Get a FREE online computer virus scan from McAfee® Security. http://clinic.mcafee.com/clinic/ibuy/campaign.asp?cid=3963 |
From: Vangelis R. <vr...@ot...> - 2004-08-29 10:03:09
|
----- Original Message ----- From: "Aaron Colwell" <mar...@ho...> To: <sdc...@li...> Subject: [sdcc-devel] PIC16 interrupt handler fix > I'm not sure what the process is for submitting patches to the compiler, so > I'm posting my fix on > this list. If there is a more appropriate place, please let me know. In general bugs and bugfixes can be sumitted through the bug tracker service of SourceForge. When you go to the SF SDCC Project page you will see the above links... You can also submit them in this list, but the problem is that mail can be lost or overlooked. Entries in the bug tracker are stored in the SF server... > handler was not restoring the WREG, BSR, and STATUS registers. > After looking at the compiler source I realized that this was done by > design. It was taking advantage of the fact that these registers get stored > on the fast register stack when a high priority interrupt occurs. > The problem was that the compiler was not generating the proper return > instruction so that these registers would be restored from the fast register > stack. This caused random behavior in my program because the interrupt > handler changed these 3 registers and they wouldn't get restored when > the interrupt handler would exit. Hmmm... that's an obvious bug. It comes because of the fact that its diffucult to test interrupt functions with gpsim (pic simulator). Thanks for the patch. The fix will available to the public in a few hours... Regards, Vangelis |
From: Scott D. <sc...@da...> - 2004-08-30 15:49:29
|
On Sun, 29 Aug 2004, Vangelis Rokas wrote: <snip> > Hmmm... that's an obvious bug. It comes because of the fact that its diffucult > to test interrupt functions with gpsim (pic simulator). Thanks for the patch. Hi Vangelis, What exactly is it about gpsim that makes it difficult to test interrupt fuctions? Since I'm working on gpsim (and obviously not SDCC) I can maybe fix whatever inadequacy that you're observing. Scott |
From: Vangelis R. <vr...@ot...> - 2004-08-30 21:33:17
|
----- Original Message ----- From: "Scott Dattalo" <sc...@da...> To: <sdc...@li...> Subject: Re: [sdcc-devel] PIC16 interrupt handler fix > What exactly is it about gpsim that makes it difficult to test interrupt > fuctions? Since I'm working on gpsim (and obviously not SDCC) I can maybe > fix whatever inadequacy that you're observing. Well, now that I read again what I've wrote it seems that there is a problem with gpsim which prohibits me from testing interrupt functions. This is not what I meant. Its not something with gpsim, but in general its difficult to test interrupts with a simulator, because of the asynchronous nature they occur. Of course gpsim has many tools for that, but I haven't dig into them yet... OTOH the specific problem referenced here was entirely my fault since I've rewritten much of the interrupt generation function but didn't thoroughly tested the generated code. In fact my opinion for gpsim is that it is an excellent tool, it would be almost impossible to bring pic16 to the point it is now without gpsim simulator. In addition the new feature you've added for client/server interface has given me new ideas for writing regression tests. Vangelis |
From: Scott D. <sc...@da...> - 2004-08-31 14:32:38
|
On Tue, 31 Aug 2004, Vangelis Rokas wrote: > ----- Original Message ----- > From: "Scott Dattalo" <sc...@da...> > To: <sdc...@li...> > Subject: Re: [sdcc-devel] PIC16 interrupt handler fix > > >> What exactly is it about gpsim that makes it difficult to test interrupt >> fuctions? Since I'm working on gpsim (and obviously not SDCC) I can maybe >> fix whatever inadequacy that you're observing. > > Well, now that I read again what I've wrote it seems that there is a > problem with gpsim which prohibits me from testing interrupt functions. > This is not what I meant. <snip> Yes, debugging interrupts in general is difficult. I've added a new feature recently to gpsim that I call 'context debugging'. This feature allows the user to debug code running in a certain 'context' or state of the microcontroller. This feature was created for a proprietary processor, but I realized it would be useful for PICs. The basic idea is that for a processor like the 18F452 you could create three contexts; non-interrupt mode, high priority interrupts, and low priority interrupts. gpsim will create 3 different source browser windows that operate independently. So for example, if you wish to single step in just the non-interrupt code, then you'd give the non-interrupt source browser the gui focus and press 'S'. Similarly, if you wanted to debug just high-priority interrupts, you'd just give that window the focus. The infrastructure is completely in place right now for context debugging. The only thing lacking is that I would need to create multiple 'ProgramMemoryAccess' objects in the _16bit_processor base class. > In fact my opinion for gpsim is that it is an excellent tool, it would be almost > impossible to bring pic16 to the point it is now without gpsim simulator. > In addition the new feature you've added for client/server interface has given > me new ideas for writing regression tests. Yes, the client/server interface is specifically designed to allow an external process to control gpsim. While I've written a simple Python script to demonstrate the proof of concept, this area still needs some more work. If all you want to do is send command line commands, then the client/server interface would work just fine. What's really lacking is a good way of getting access to the simulator state. I've got two more items I plan to work on before returning to the client/server interface. First are command line macros. These will allow simple parameterized named macros to be created. The next feature will be "trigger actions". At the moment, you can define a break point in gpsim. This consist of two things: a condition followed by an action. For example, if the PC=123 then break. The condition is 'if the PC=123' and the action is 'break'. At the moment, both of these concepts are bundled up into the breakpoint class as single operation. So the only thing you can do when a condition occurs is to halt the simulation. When the trigger actions are completed, you'll be able to separate the condition (the trigger) from the action. In addition you'll be able to create customizable actions. For example, you may wish to pause every 1000 simulation cycles and call an external process (through the client/server interface). So maybe I can just help SDCC by making gpsim easier to use... Scott |