From: SourceForge.net <no...@so...> - 2004-07-08 08:18:19
|
Bugs item #979599, was opened at 2004-06-25 10:44 Message generated for change (Comment added) made by maartenbrock 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: Maarten Brock (maartenbrock) Date: 2004-07-08 10:18 Message: Logged In: YES user_id=888171 1) (unsigned) port &= unsigned is completely in the unsigned domain, but (unsigned) port &= signed needs a cast from (unsigned) port to signed port. It's this cast that stands in the way of optimization. 2) According to the C standard the constant should be signed int. But we have an 8-bit cpu and would like to keep as much arithmatic in a char. So the constant is (signed) char. 3) In section 6.5.3.3 the word (promoted) worries me most. To what should we promote? To the type of the operand? (ridiculous) To int (for char or bitfield)? To the type the result will be promoted to? (which seems best to me, but is harder to achieve) I hope you agree this standard is a pain to read. I'll go and have a look if I can manage the last one. ---------------------------------------------------------------------- Comment By: Josef Pavlik (jetset) Date: 2004-07-07 02:32 Message: Logged In: YES user_id=756222 one question: Why the code of port &= unsigned is different of port &= signed? another question: the constant should be signed or unsigned? reading the http://www.open-std.org/jtc1/sc22/wg14/www/docs/n843.htm section 6.4.4.1 I found the following: type of an decimal constant w/o suffix is the first enought from the following list: signed int, signed long int, signed long long int type of hex and oct constant w/o suffix is the first enought from the following list: signed int, unsigned int, signed long int, unsigned long int, signed ll int, unsigned ll int (interesting thing: the constants 32768 and 0x8000 are not equal in 16bit CPU! ) Seems that the constat should be signed. before the aritmetic operation is done, the operands should be converted using the following rules: http://www.open-std.org/jtc1/sc22/wg14/www/docs/n843.htm section 6.3.1.7 The integer promotions are performed on both operands. Then the following rules are applied to the promoted operands: If both operands have the same type, then no further conversion is needed. Otherwise, if both operands have signed integer types or both have unsigned integer types, the operand with the type of lesser integer conversion rank is converted to the type of the operand with greater rank. Otherwise, if the operand that has unsigned integer type has rank greater or equal to the rank of the type of the other operand, then the operand with signed integer type is converted to the type of the operand with unsigned integer type. Otherwise, if the type of the operand with signed integer type can represent all of the values of the type of the operand with unsigned integer type, then the operand with unsigned integer type is converted to the type of the operand with signed integer type. Otherwise, both operands are converted to the unsigned integer type corresponding to the type of the operand with signed integer type. And finally, section 6.5.3.3 says: [#4] The result of the ~ operator is the bitwise complement | of its (promoted) operand (that is, each bit in the result is set if and only if the corresponding bit in the converted operand is not set). The integer promotions are performed on the operand, and the result has the promoted type. If | the promoted type is an unsigned type, the expression ~E is | equivalent to the maximum value representable in that type | minus E. I hope, that those informations can help you to resolve the problem. Jet ---------------------------------------------------------------------- Comment By: Maarten Brock (maartenbrock) Date: 2004-07-06 22:06 Message: Logged In: YES user_id=888171 That should have been: unsigned long int d; ---------------------------------------------------------------------- Comment By: Maarten Brock (maartenbrock) Date: 2004-07-06 21:59 Message: Logged In: YES user_id=888171 Josef, You're right. It's all about the bitwise complement operator which renders the constant signed. And somehow it's never changed to unsigned again. Bernhard's fix helps, but it feels a bit like a hack. What if something else comes in between... And why does "unsigned int c; c &= ~1" generate highly compact code? In this case ~1 IS unsigned although it started out signed. I'm thinking of changing the patch for bug 524195. Instead of hard casting ~ on literals signed, I suggest hard casting of char to int. This seems to generate compact and correct code, even when applied to int's. But if used with long ints it generates this code: long int d; d &= ~1; anl _d,#0xFE mov (_d + 2),#0x00 mov (_d + 3),#0x00 So the next question is if this is according to the C-standard or not? Of course ~1L gives the expected result. Maarten ---------------------------------------------------------------------- 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 |