From: Maarten B. <sou...@ds...> - 2004-08-21 09:35:18
|
Stan, > I tried accessing an xdata location on an EZ-USB AN2131 two different ways, > as an unsigned char and as a bit field structure. Here are the declarations: > > struct bitsUSBCS > { > unsigned SIGRSUME : 1; // bit 0 > unsigned RENUM : 1; // bit 1 > unsigned DISCOE : 1; // bit 2 > unsigned DISCON : 1; // bit 3 > unsigned Reserved : 3; // bits 6-4 > unsigned WAKESRC : 1; // bit 7 > }; > > xdata at 0x7fd6 unsigned char byteUSBCS; > xdata at 0x7fd6 struct bitsUSBCS USBCS; > > Here is the code I get when clearing bit 2 using the two different methods: > > byteUSBCS &= ~0x04; > > ; genAssign > ; genAnd > ; Peephole 248.b optimized and to xdata > mov dptr,#_byteUSBCS > movx a,@dptr > mov r2,a > anl a,#0xFB > movx @dptr,a > > USBCS.DISCOE = 0; > > ; genPointerSet > ; genFarPointerSet > mov dptr,#_USBCS > ; genPackBits > movx a,@dptr > anl a,#0xfb > movx @dptr,a > > Here is the code I get when setting bit 2 using the two different methods: > > byteUSBCS |= 0x04; > > ; genAssign > ; genOr > ; Peephole 248.a optimized or to xdata > mov dptr,#_byteUSBCS > movx a,@dptr > mov r2,a > orl a,#0x04 > movx @dptr,a > > USBCS.DISCOE = 1; > > ; genPointerSet > ; genFarPointerSet > mov dptr,#_USBCS > ; genPackBits > movx a,@dptr > orl a,#0x04 > movx @dptr,a > -------------------------------------- > > The only difference I see is the temporary storage of the content of @dptr > before the ANDing or ORing operation. Does this cause any problems in the > evaluation of expressions containing bit fields or are bit fields safe to > use? I'd much prefer using USBCS.DISCOE = 0 rather than byteUSBCS &= ~0x04. > > -Stan I see no reason why you could not use bitfields. All I see is an opportunity for better optimization. |
From: Frieder F. <fri...@we...> - 2004-08-21 11:40:56
Attachments:
peeph_248_x.def
|
Maarten Brock wrote: >> >>The only difference I see is the temporary storage of the content of @dptr >>before the ANDing or ORing operation. Does this cause any problems in the >>evaluation of expressions containing bit fields or are bit fields safe to >>use? I'd much prefer using USBCS.DISCOE = 0 rather than byteUSBCS &= ~0x04. >> >>-Stan > > > I see no reason why you could not use bitfields. All I see is an > opportunity for better optimization. Some redundant loads of dptr currently are only removed for repeated ANDing and ORing operations. You'll notice that if you use an xdata port pin to output a strobe like xport.strobe=1; xport.strobe=0; xport.strobe=1; The appended file should fix that (append --peep-file peeph_248_x.def to the SDCC command line, or cut'n paste this into src/mcs51/peeph.def and recompile SDCC). @Maarten: I won't commit this unless you give OK. Frieder |
From: Stan M. <sta...@ya...> - 2004-08-21 23:21:06
|
Maarten & Frieder: Thanks to you both for your insights. I had noticed the redundant loads of dptr when using xdata bit fields. Thanks for the patch but so far I have been using Windows binaries. When I get further along I may start building from sources and will then use your changes. -Stan |
From: Maarten B. <sou...@ds...> - 2004-08-22 09:55:12
|
> Maarten & Frieder: > > Thanks to you both for your insights. > I had noticed the redundant loads of dptr when using xdata bit fields. > Thanks for the patch but so far I have been using Windows binaries. When I > get further along I may start building from sources and will then use your > changes. > > -Stan Stan, If you don't build SDCC you can still use the peephole file Frieder provided with SDCC's --peep-file option, see the manual on how to use it. Frieder, This looks ok to me, no volatile problems either. But it's too bad we can't do this in reverse. Is there any way to "remember" the contents of DPTR? Hide it in a comment or something like that? Maybe we must invent a new opcode and remove it later on? Just an idea: replace { mov dptr,%1 movx a,@dptr orl a,%2 movx @dptr,a mov dptr,%1 movx a,@dptr anl a,%3 movx @dptr,a } by { mov dptr,%1 movx a,@dptr orl a,%2 movx @dptr,a keep dptr,%1 ;this line will be removed later movx a,@dptr anl a,%3 movx @dptr,a } replace { keep dptr,%1 ;this line will be removed later movx a,@dptr anl a,%3 movx @dptr,a mov dptr,%1 movx a,@dptr orl a,%2 movx @dptr,a } by { keep dptr,%1 ;this line will be removed later movx a,@dptr anl a,%3 movx @dptr,a keep dptr,%1 ;this line will be removed later movx a,@dptr orl a,%2 movx @dptr,a } replace { keep dptr,%1 ;this line will be removed later } by { ;removed redundant load } And some variations on this theme of course. |
From: Frieder F. <fri...@we...> - 2004-08-22 14:18:33
|
Hi Maarten, Maarten Brock wrote: > > This looks ok to me, no volatile problems either. But it's too bad we > can't do this in reverse. Is there any way to "remember" the contents of > DPTR? Hide it in a comment or something like that? Maybe we must > invent a new opcode and remove it later on? Just an idea: > > replace { > mov dptr,%1 > movx a,@dptr > orl a,%2 > movx @dptr,a > > mov dptr,%1 > movx a,@dptr > anl a,%3 > movx @dptr,a > } by { > mov dptr,%1 > movx a,@dptr > orl a,%2 > movx @dptr,a > > keep dptr,%1 ;this line will be removed later > movx a,@dptr > anl a,%3 > movx @dptr,a > } > > replace { > keep dptr,%1 ;this line will be removed later > movx a,@dptr > anl a,%3 > movx @dptr,a > > mov dptr,%1 > movx a,@dptr > orl a,%2 > movx @dptr,a > } by { > keep dptr,%1 ;this line will be removed later > movx a,@dptr > anl a,%3 > movx @dptr,a > > keep dptr,%1 ;this line will be removed later > movx a,@dptr > orl a,%2 > movx @dptr,a > } > > replace { > keep dptr,%1 ;this line will be removed later > } by { > ;removed redundant load > } > > And some variations on this theme of course. Yes, you'd need quite a few... and I don't think the effort is spent too well there, Stan's example is one of the few examples where peepholes can do optimizations which include 2 or 3 lines of c-code. (And although the peephole 248.x rules are already relatively long they only cover |,&,^ operators) I guess if the compiler could safely issue a "keep" instruction it could have removed this line prior to peephole optimization. And because it has a better overview than our peephole optimizations it can be much better at it! How to get there? There is something in SDCC which might be extended for that: If you compile the _naked interrupt example in http://sdcc.sourceforge.net/doc/sdccman.html/node76.html with a recent SDCC you'll note comments like ; eliminated unneeded push/pop b ; eliminated unneeded push/pop acc in the .asm file. These optimizations are possible by the use of functions getRegsWritten() asmLineNodeFromLineNode() in src/mcs51/main.c which Erik committed these Spring this year, iirc. They currently are 'only' used to detect whether a register is read/written or not. If these functions would track whether a literal/symbol/symbolwithoffset was assigned to a register then redundant loads could be easily detected and removed (if within the same basic block). This would make peepholes like 248.x (and others of its kind we'd potentially add:) immediately obsolete and it would also optimize cases where the idata/pdata addressing modes (like mov @r0,a or movx @r0,a) are used (or where a register is used for temporary storage and is not read later). Cheers, Frieder PS: if you respond to this maybe open a thread on sdcc-devel? |
From: Maarten B. <sou...@ds...> - 2004-08-22 19:09:27
|
> PS: if you respond to this maybe open a thread on sdcc-devel? Discussion will have to be postponed on my side. I'm going on a holiday this week. B.t.w. I think Stan wanted to know if the line containing R2 was needed or that the other version was safe to use. And as stated, I think he can. |