From: al_bin <al...@vp...> - 2013-01-10 18:50:33
|
On 10. 01. 2013 18:09, Raphael Neider wrote: > Hi, > >> sfr16 keyword exist for pic16. > I> t does? Well, that would be news to me ... Does it work?!? Yes, and works fine. As pic14 with patch from my previous post. ... > I agree that this feature would be nice. However, this needs support > in the code generator and (equally important) in the linker (gputils' > gplink): Currently gputils do not like two symbols (TMR1 and TMR1L) > occupying the same memory location (0x000F). > Unless someone volunteers to implement both sides (or multiple > volunteers step forward, together covering both sides ...), I do not > see this being implemented any time soon. > It is not sufficient to just "allow" the keyword and hope that > everything else just magically falls into place. > > Sorry to disappoint you. > No problem, for me it work well with only allow the keyword. Just want to share. See: ----------------------------file flash.c ----------------------------- #include <stdint.h> #define Nop() __asm NOP __endasm extern __at(0x018E) __sfr16 PMADR; extern __at(0x0190) __sfr16 PMDAT; typedef struct { unsigned RD : 1; unsigned WR : 1; unsigned WREN : 1; unsigned : 1; unsigned : 1; unsigned : 1; unsigned : 1; unsigned : 1; } __PMCON1bits_t; extern __at(0x018C) volatile __PMCON1bits_t PMCON1bits; uint16_t read_flash(uint16_t addr) { PMADR = addr; PMCON1bits.RD = 1; Nop(); Nop(); return PMDAT; } -------------------------------------------------------------------------------------- and result compiled from patched last SVN version: (no errors or warnings) xxx> sdcc --use-non-free --opt-code-size -mpic14 -pp12f752 -c flash.c -o flash.o ---------------------------flash.asm --------------------------------------------------------------------- ... code_flash code ;*** ; pBlock Stats: dbName = C ;*** ;entry: _read_flash ;Function start ; 2 exit points ;has an exit ;1 compiler assigned register : ; STK00 ;; Starting pCode block _read_flash ;Function start ; 2 exit points ; .line 21; "flash.c" uint16_t read_flash(uint16_t addr) BANKSEL _PMADR MOVWF (_PMADR + 1) MOVF STK00,W MOVWF _PMADR ; .line 25; "flash.c" PMCON1bits.RD = 1; BSF _PMCON1bits,0 NOP NOP ; .line 28; "flash.c" return PMDAT; BANKSEL _PMDAT MOVF _PMDAT,W MOVWF STK00 MOVF (_PMDAT + 1),W RETURN ; exit point of _read_flash ... -------------- Albert |
From: al_bin <al...@vp...> - 2013-01-11 07:26:35
|
On 10. 01. 2013 18:09, Raphael Neider wrote: ... > So code generation is ready (and a lot of things actually just fall > into place -- its magic :-D). > Did you also already use overlapping SFR definitions (TMR1 and TMR1L above)? ... As /sdcc/device/non-free/lib/pic14/libdev/* is created in sdcc build stage, and source files didn't contain sfr16 keyword, this work as without patch. Next in install stage we can change extern declaration __sfr TMR1 to __sfr16 TMR1 without touching TMR1L and TMR1H in device include files. Effects you see in example from previous post. And yes, user can use TMR1, TMR1L and TMR1H together. If we want exclude libdev and define SFR in user programs (want we?) TMR1 conflicts with TMR1H ( if use __sfr16 for TMR1) To avoid this we can patch file sdcc/src/pic14/glue.c (comment out instruction for size increment, line 210). But i'm not sure for all side effects now. And this is not necessary in my opinion. Albert |
From: Molnár K. <pr...@fr...> - 2013-01-13 09:32:42
|
On Fri, 11 Jan 2013 08:26:25 +0100 al_bin <al...@vp...> wrote: > On 10. 01. 2013 18:09, Raphael Neider wrote: > ... > > So code generation is ready (and a lot of things actually just fall > > into place -- its magic :-D). > > Did you also already use overlapping SFR definitions (TMR1 and TMR1L above)? > ... > > As /sdcc/device/non-free/lib/pic14/libdev/* is created in sdcc build stage, > and source files didn't contain sfr16 keyword, this work as without patch. > Next in install stage we can change extern declaration __sfr TMR1 to __sfr16 TMR1 > without touching TMR1L and TMR1H in device include files. > Effects you see in example from previous post. And yes, user can use TMR1, > TMR1L and TMR1H together. > > If we want exclude libdev and define SFR in user programs (want we?) > TMR1 conflicts with TMR1H ( if use __sfr16 for TMR1) > To avoid this we can patch file sdcc/src/pic14/glue.c (comment out instruction for size > increment, line 210). But i'm not sure for all side effects now. > And this is not necessary in my opinion. > > Albert Something left out of the calculation. In the case of TMRx, differ the read and write sequence. write: first TMRxH, second TMRxL read: first TMRxL, second TMRxH The sdcc can handle this exceptional case? Károly |
From: al_bin <al...@vp...> - 2013-01-13 12:23:33
|
W dniu 2013-01-13 09:53:00 użytkownik Molnár Károly <pr...@fr...> napisał: ... > Something left out of the calculation. In the case of TMRx, differ the read and write sequence. > write: first TMRxH, second TMRxL > read: first TMRxL, second TMRxH > :-) As for mcs51: SDCC manual (http://sdcc.sourceforge.net/doc/sdccman.pdf) ch. 3.4.1.7 "16 Bit and 32 bit special function register combinations which require a certain access order are better not declared using __sfr16 or __sfr32. Allthough SDCC usually accesses them Least Significant Byte (LSB) first, this is not guaranteed." Albert |
From: Molnár K. <pr...@fr...> - 2013-01-13 13:13:48
|
On Sun, 13 Jan 2013 13:23:23 +0100 al_bin <al...@vp...> wrote: > W dniu 2013-01-13 09:53:00 użytkownik Molnár Károly <pr...@fr...> napisał: > ... > > Something left out of the calculation. In the case of TMRx, differ the read and write sequence. > > write: first TMRxH, second TMRxL > > read: first TMRxL, second TMRxH > > > > :-) > > As for mcs51: > SDCC manual (http://sdcc.sourceforge.net/doc/sdccman.pdf) ch. 3.4.1.7 > > > "16 Bit and 32 bit special function register combinations which require a certain access order are better not declared > using __sfr16 or __sfr32. Allthough SDCC usually accesses them Least Significant Byte (LSB) first, this is not > guaranteed." > > Albert > This would be good for a new type: __sfr16_t --- "t" -> timer Perhaps even these will: __sfr16_be --- "be" -> big-endian __sfr16_le --- "le" -> little-endian Otherwise, I now updated the PIC16 branch (device and include files). From now on use the __ sfr16 type. Due to the above to TMRx registers, no sfr16. Two examples of the realization: ..................................... __at(0x0FC3) __sfr ADRES; __at(0x0FC3) __sfr ADRESL; __at(0x0FC3) __sfr16 ADRESw; __at(0x0FC4) __sfr ADRESH; ..................................... __at(0x0FD9) __sfr FSR2L; __at(0x0FD9) __sfr16 FSR2w; __at(0x0FDA) __sfr FSR2H; ..................................... Károly |
From: Raphael N. <rn...@we...> - 2013-01-13 16:37:08
|
Hi, > This would be good for a new type: __sfr16_t --- "t" -> timer > > Perhaps even these will: __sfr16_be --- "be" -> big-endian > __sfr16_le --- "le" -> little-endian > > Otherwise, I now updated the PIC16 branch (device and include files). From now on use the __ sfr16 > type. Due to the above to TMRx registers, no sfr16. What are the consequences to code generation functions if you introduce a new type / new types in order to guarantee LSB first/MSB first access? Are sfr/__sfr16 data first read via some volatile read fuction (iCode?), then processed, and then written back via some volatile write function (iCode?)? Otherwise, probably all code generation functions need to be revised to guarantee proper behaviour [1]. Do you plan to introduce all four probably variations of 16-bit SFRs (read order LSB or MSB first vs. write order LSB or MSB first)? I am not even sure if all (PIC14/PIC16) functions properly honor volatile -- they might read/assign their operands multiple times, possibly even depending on runtime conditions. Are we sure not to introduce additional, subtle bugs here? Do not get me wrong: I'd like direct support for multi-bype SFRs, I just want to avoid special cases throughout the backend. > __at(0x0FC3) __sfr ADRES; > > __at(0x0FC3) __sfr ADRESL; > __at(0x0FC3) __sfr16 ADRESw; > > __at(0x0FC4) __sfr ADRESH; Do gputils (gplink) allow this? We end up with ADRESL and ADRESw occupying 0xFC3 as well as ADRSH and ADRESw occupying 0xFC4. *Should* the linker allow this?!? Best regards Raphael [1]: I assume that SFRs should be usable in statements such as ADRESw += 0x0101; even though as user one *should* probably use unsigned temp = ADRESw; temp += 0x0101; ADRESw = temp; which would then only require assignments from / to SFR types to be handled correctly. |
From: Molnár K. <pr...@fr...> - 2013-01-13 18:26:56
|
On Sun, 13 Jan 2013 17:37:00 +0100 Raphael Neider <rn...@we...> wrote: > Hi, > > > This would be good for a new type: __sfr16_t --- "t" -> timer > > > > Perhaps even these will: __sfr16_be --- "be" -> big-endian > > __sfr16_le --- "le" -> little-endian > > > > Otherwise, I now updated the PIC16 branch (device and include files). From now on use the __ sfr16 > > type. Due to the above to TMRx registers, no sfr16. > > What are the consequences to code generation functions if you > introduce a new type / new types in order to guarantee LSB first/MSB > first access? Only so register pairs were given the 16-bit access, where indifferent the order of access. The TMR registers are therefore I let out of this. The newer models were only ideas. Only I have created an opportunity for testing, if it already exists in the __sfr16 type. > Are sfr/__sfr16 data first read via some volatile read > fuction (iCode?), then processed, and then written back via some > volatile write function (iCode?)? Otherwise, probably all code > generation functions need to be revised to guarantee proper behaviour > [1]. > Do you plan to introduce all four probably variations of 16-bit SFRs > (read order LSB or MSB first vs. write order LSB or MSB first)? > > I am not even sure if all (PIC14/PIC16) functions properly honor > volatile -- they might read/assign their operands multiple times, > possibly even depending on runtime conditions. > > Are we sure not to introduce additional, subtle bugs here? > > Do not get me wrong: I'd like direct support for multi-bype SFRs, I > just want to avoid special cases throughout the backend. > > > __at(0x0FC3) __sfr ADRES; > > > > __at(0x0FC3) __sfr ADRESL; > > __at(0x0FC3) __sfr16 ADRESw; > > > > __at(0x0FC4) __sfr ADRESH; > > Do gputils (gplink) allow this? I performed a test. The compilation of all pic18xxx.c device files succeeded without error. > We end up with ADRESL and ADRESw > occupying 0xFC3 as well as ADRSH and ADRESw occupying 0xFC4. *Should* > the linker allow this?!? > But, unfortunately, I has just discovered that the gplink produce errors. :-(( > Best regards > Raphael > > > [1]: I assume that SFRs should be usable in statements such as > > ADRESw += 0x0101; > > even though as user one *should* probably use > > unsigned temp = ADRESw; > temp += 0x0101; > ADRESw = temp; > > which would then only require assignments from / to SFR types to be > handled correctly. I restore the previous state. I wanted to help, but I was careless. Excuse me. Károly |
From: Borut R. <bor...@gm...> - 2013-01-13 20:34:54
|
On 13. 01. 2013 19:26, Molnár Károly wrote: > On Sun, 13 Jan 2013 17:37:00 +0100 > Raphael Neider <rn...@we...> wrote: > >> We end up with ADRESL and ADRESw >> occupying 0xFC3 as well as ADRSH and ADRESw occupying 0xFC4. *Should* >> the linker allow this?!? >> > But, unfortunately, I has just discovered that the gplink produce errors. :-(( What kind of errors? Aren't sfr definitions just mapped to EQUs in asm? Can you please provide a short example so that I can analyze it from the gplink perspective? Borut |
From: Raphael N. <rn...@we...> - 2013-01-13 23:15:16
|
Hi Borut, On Sun, 13 Jan 2013 21:34:45 +0100, Borut Ražem <bor...@gm...> wrote: > On 13. 01. 2013 19:26, Molnár Károly wrote: >> On Sun, 13 Jan 2013 17:37:00 +0100 >> Raphael Neider <rn...@we...> wrote: >> >>> We end up with ADRESL and ADRESw >>> occupying 0xFC3 as well as ADRSH and ADRESw occupying 0xFC4. *Should* >>> the linker allow this?!? >>> >> But, unfortunately, I has just discovered that the gplink produce >> errors. :-(( > > What kind of errors? Aren't sfr definitions just mapped to EQUs in asm? > Can you please provide a short example so that I can analyze it from the > gplink perspective? sfr definitions are turned into udata / udata_ovr sections (see below) in order to properly allocate the memory for/to the register. neider@neider-virtual:/media/daten/neider/test/sfr16$ cat main.c __at(0x40) unsigned foo; __at(0x41) __sfr bar; void main () {} neider@neider-virtual:/media/daten/neider/test/sfr16$ ../../sdcc.svn/build/bin/sdcc -mpic14 -p16f877a main.c --use-non-free message: using default linker script "/home/neider/local.svn/share/gputils/lkr/16f877a_g.lkr" error: multiple sections using address 0x41 main.asm contains: UD_abs_main_40 udata_ovr 0x0040 _foo res 2 UD_abs_main_41 udata_ovr 0x0041 _bar res 1 From my point of view, this is exactly the right thing to do for the linker. If we want the overlap, we should use a union. Otherwise, the linker should ensure that apparently separate data objects are in fact separate, i.e., they reside in disjoint memory regions. Kind regards Raphael |
From: Borut R. <bor...@gm...> - 2013-01-14 19:54:33
|
Raphael, thanks for explanation. Would generating symbols instead of sections including res instructions solve the problem? For example: _foo EQU 0x0040 _bar EQU 0x0041 instead of UD_abs_main_40 udata_ovr 0x0040 _foo res 2 UD_abs_main_41 udata_ovr 0x0041 _bar res 1 Borut On 13. 01. 2013 23:59, Raphael Neider wrote: > Hi Borut, > > On Sun, 13 Jan 2013 21:34:45 +0100, Borut Ražem <bor...@gm...> > wrote: > >> On 13. 01. 2013 19:26, Molnár Károly wrote: >>> On Sun, 13 Jan 2013 17:37:00 +0100 >>> Raphael Neider <rn...@we...> wrote: >>> >>>> We end up with ADRESL and ADRESw >>>> occupying 0xFC3 as well as ADRSH and ADRESw occupying 0xFC4. *Should* >>>> the linker allow this?!? >>>> >>> But, unfortunately, I has just discovered that the gplink produce >>> errors. :-(( >> What kind of errors? Aren't sfr definitions just mapped to EQUs in asm? >> Can you please provide a short example so that I can analyze it from the >> gplink perspective? > sfr definitions are turned into udata / udata_ovr sections (see below) in > order to properly allocate the memory for/to the register. > > neider@neider-virtual:/media/daten/neider/test/sfr16$ cat main.c > __at(0x40) unsigned foo; > __at(0x41) __sfr bar; > > void main () {} > neider@neider-virtual:/media/daten/neider/test/sfr16$ > ../../sdcc.svn/build/bin/sdcc -mpic14 -p16f877a main.c --use-non-free > message: using default linker script > "/home/neider/local.svn/share/gputils/lkr/16f877a_g.lkr" > error: multiple sections using address 0x41 > > main.asm contains: > UD_abs_main_40 udata_ovr 0x0040 > _foo > res 2 > UD_abs_main_41 udata_ovr 0x0041 > _bar > res 1 > > > From my point of view, this is exactly the right thing to do for the > linker. If we want the overlap, we should use a union. Otherwise, the > linker should ensure that apparently separate data objects are in fact > separate, i.e., they reside in disjoint memory regions. > > Kind regards > Raphael > > ------------------------------------------------------------------------------ > Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS, > MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current > with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft > MVPs and experts. ON SALE this month only -- learn more at: > http://p.sf.net/sfu/learnmore_123012 > _______________________________________________ > Sdcc-user mailing list > Sdc...@li... > https://lists.sourceforge.net/lists/listinfo/sdcc-user > > |
From: al_bin <al...@vp...> - 2013-01-14 13:09:15
|
OMG. My point of view: Current wersion PIC16 is OK. We have sfr16 keyword and proper libdev. In libdev is space allocated for REGL and REGH. Thus for REG too. In user header files we have declarations: extern __at(addr) __sfr REGL; extern __at(addr+1) __sfr REGH; extern __at(addr) __sfr REG; //3 In current state REG duplicates REGL Change declaration 3 (for user programs, NOT for create libdev ) to extern __at(addr) __sfr16 REG; break nothing and allow use word registers. The same is for PIC14, except sfr16 is not keyword. Albert |
From: Raphael N. <rn...@we...> - 2013-01-10 21:31:11
|
Hi, >> It does? Well, that would be news to me ... Does it work?!? > > Yes, and works fine. As pic14 with patch from my previous post. > ... Now, that's cool ... I'll probably uncomment it then. Not sure what the code transformations (aka optimizations in the backend) make of it, but one could surely give it a try. >> I agree that this feature would be nice. However, this needs support >> in the code generator and (equally important) in the linker (gputils' >> gplink): Currently gputils do not like two symbols (TMR1 and TMR1L) >> occupying the same memory location (0x000F). >> Unless someone volunteers to implement both sides (or multiple >> volunteers step forward, together covering both sides ...), I do not >> see this being implemented any time soon. >> It is not sufficient to just "allow" the keyword and hope that >> everything else just magically falls into place. >> >> Sorry to disappoint you. >> > > No problem, for me it work well with only allow the keyword. > Just want to share. So code generation is ready (and a lot of things actually just fall into place -- its magic :-D). Did you also already use overlapping SFR definitions (TMR1 and TMR1L above)? > See: > ----------------------------file flash.c ----------------------------- Looks good indeed. Thanks for the report! Raphael |