From: SourceForge.net <no...@so...> - 2004-07-01 17:58:27
|
Bugs item #979599, was opened at 2004-06-25 10:44 Message generated for change (Comment added) made by jetset You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=979599&group_id=599 Category: None Group: None Status: Open Resolution: None Priority: 5 Submitted By: Josef Pavlik (jetset) Assigned to: Maarten Brock (maartenbrock) Summary: inefficent/wrong code for PORT&=~mask Initial Comment: hello I found, that the expression PORT &= ~mask is not compiled efficently, in some cases this code can make unpredictable result. I expect, that the resulting code will be anl PORT, #~mask but the value of the port is loaded to the register, then anded with mask and is reloaded to the port. If the port value change or if this is some input signals on the port, the 0 read from input signal will be written as 0 to output and will block the input port. The expression PORT |= mask is compiled OK I'm using the last version of compiler (15.6.04) with --model-large see the piece of resulting code in attachment #define RS485_POWER_PORT P3 #define RS485_POWER_MASK 0x08 ;rs485.c:321: RS485_POWER_PORT|=RS485_POWER_MASK; ; genOr orl _P3,#0x08 ; Peephole 112.b changed ljmp to sjmp ; Peephole 251.b replaced sjmp to ret with ret ret 00102$: ;rs485.c:325: RS485_POWER_PORT&=~RS485_POWER_MASK; ; genAssign mov r2,_P3 ; genAnd mov a,#0xF7 anl a,r2 mov _P3,a 00104$: ---------------------------------------------------------------------- >Comment By: Josef Pavlik (jetset) Date: 2004-07-01 19:58 Message: Logged In: YES user_id=756222 I mean the bitwise complement of the mask which is evaluated by the compiler. Example: the expression port|=~mask versus port|=mask ---------------------------------------------------------------------- Comment By: Bernhard Held (bernhardheld) Date: 2004-07-01 16:46 Message: Logged In: YES user_id=203539 ~ is the "bitwise complement". It still can't see any problem. The only opcode of the 8051 with complement is "cpl a", therefore SDCC can never generate a read-modify-write code for the complement operation. ---------------------------------------------------------------------- Comment By: Josef Pavlik (jetset) Date: 2004-07-01 00:44 Message: Logged In: YES user_id=756222 sorry, I mean the ~ operator. You're right. 'not' is '!'. what is the tilde? Complement? Negate? Unfortunately, English is not my best known language :-((( ---------------------------------------------------------------------- Comment By: Bernhard Held (bernhardheld) Date: 2004-06-30 22:42 Message: Logged In: YES user_id=203539 Both bitwise or and xor are already included in my patch. What tests did you do with the "NOT statement"? There's no read-modify-write operation with logical negation, neither in C nor in 8051 assembler. '!=' looks similiar to '^=' but the semantic is somewhat different ;-) ---------------------------------------------------------------------- Comment By: Josef Pavlik (jetset) Date: 2004-06-30 00:13 Message: Logged In: YES user_id=756222 Reading the comments and making additional test I found, that the problem is not bound to the AND statement, but to the NOT statement and is present in the following two cases too: port|=~mask port^=~mask ---------------------------------------------------------------------- Comment By: Bernhard Held (bernhardheld) Date: 2004-06-29 22:41 Message: Logged In: YES user_id=203539 The answer is easy: there's no code for this optimization. I've attached my fix. The optimization itself is ok, but I'm in doubt about the liferange-part. I never touched the LR code and I wasn't able to test it. I would be happy if somebody could review my code. ---------------------------------------------------------------------- Comment By: Bernhard Held (bernhardheld) Date: 2004-06-29 22:41 Message: Logged In: YES user_id=203539 The answer is easy: there's no code for this optimization. I've attached my fix. The optimization itself is ok, but I'm in doubt about the liferange-part. I never touched the LR code and I wasn't able to test it. I would be happy if somebody could review my code. ---------------------------------------------------------------------- Comment By: Maarten Brock (maartenbrock) Date: 2004-06-29 20:58 Message: Logged In: YES user_id=888171 The fix for bug 524195 is partly responsible for this 'bug'. See SDCCval.c 1.52. Essentially the result from ~ is always signed. Now the question remains why (signed)iTemp0 = (unsigned)c (signed)iTemp1 = (signed)iTemp0 & (signed)0xF7 (unsigned)c = (signed)iTemp0 is not optimized. ---------------------------------------------------------------------- Comment By: Maarten Brock (maartenbrock) Date: 2004-06-29 10:30 Message: Logged In: YES user_id=888171 The volatileness of P3 has nothing to do with it. As you found out P3 &= (unsigned char)~0x08 does give optimized output. Replacing P3 by 'volatile unsigned char c' gives the same result as well as 'unsigned char c'. Even this does not generate optimal code: c &= ~0x08U So it must be some conversion from unsigned to signed by ~ that gets in the way. When c is signed it always generates optimal code, volatile or not. ---------------------------------------------------------------------- Comment By: Bernhard Held (bernhardheld) Date: 2004-06-28 09:33 Message: Logged In: YES user_id=203539 Another quick fix is PORT &= (unsigned char) ~mask; I've already spent some time on this issue. The beginning of the problem is a cast from 'signed char' to 'unsigned char' added in iCode. This cast should be eliminated by the optimizer (findCheaperOp()). The problem here is, that P3 is volatile, and the optimizer refuses to touch this iCode (usigned char c; c &= ~mask; // isn't optimized too!?). It's not (yet) possible to avoid the addition of the cast, because it's important to keep the signedness of all operands. Even in mcs51/gen.c the signedness of an operation is still determined by the signedness of the operands. IMO the signedness of the operation should be determined in AST and used throughout the iCode and code generators. This would generally enable more aggressive optimizations of operands. Finally I also planed to fix this issue with the peephole optimizer, but Frieder pointed out an important problem: the register and acc isn't updated. It seems to be necessary to add another special case to the optimizer (mcs51/packRegsForAssign()). ---------------------------------------------------------------------- Comment By: Josef Pavlik (jetset) Date: 2004-06-28 01:15 Message: Logged In: YES user_id=756222 Hi Maarten, the behaviour, that read-modify-write instructions reads the output register instead of real state of the port is the standard behaviour of the original 8051 (c) Intel. Thanx to Frief for the ad-hoc work around Jet ---------------------------------------------------------------------- Comment By: Frieder Ferlemann (frief) Date: 2004-06-27 19:40 Message: Logged In: YES user_id=589052 The attached file has a temporary fix for the problem to get things going until it is fixed properly To make use of this additionally specify "--peep-file peeph_979599.def " on the sdcc command line. Note: this peephole still updates r2 and acc although both of them probably won't hold the expected values (as you and Maarten pointed out). ---------------------------------------------------------------------- Comment By: Maarten Brock (maartenbrock) Date: 2004-06-27 14:13 Message: Logged In: YES user_id=888171 I'm not sure what an original 8051 does but at least some Silabs derivatives behave differently between ANL _P3,#0x08 and MOV A,_P3, ANL A,#0x08, MOV _P3,A. This is from their documentation: A Read of a Port Data register (or Port bit) will always return the logic state present at the pin itself, regardless of whether the Crossbar has allocated the pin for peripheral use or not. An exception to this occurs during the execution of a read- modify-write instruction (ANL, ORL, XRL, CPL, INC, DEC, DJNZ, JBC, CLR, SET, and the bitwise MOV operation). During the read cycle of the read-modify-write instruction, it is the contents of the Port Data register, not the state of the Port pins themselves, which is read. And you're right IMHO that you may assume the direct code to be produced. I know I would. Greets, Maarten ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=979599&group_id=599 |