From: SourceForge.net <no...@so...> - 2008-08-31 08:23:12
|
Bugs item #1444425, was opened at 2006-03-06 22:07 Message generated for change (Comment added) made by borutr You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1444425&group_id=599 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: pic16 target >Group: fixed >Status: Closed >Resolution: Fixed Priority: 5 Private: No Submitted By: Borut Raem (borutr) Assigned to: Raphael Neider (tecodev) Summary: onebyte.c regression tes fails on pic16 Initial Comment: 1 - sample code: ------------------------ void main(void) { char cL; volatile char cR; volatile char r16; cL = -1; cR = 1; r16 = cL * cR; } ------------------------ 2 - sdcc command: >sdcc -mpic16 -S t.c 3 - sdcc version: >sdcc -v SDCC : mcs51/gbz80/z80/avr/ds390/pic16/pic14/TININative/xa51/ds400/hc08 2.5.4 #1216 (Mar 5 2006) (MINGW32) 4 - error message: Internal error: validateOpType failed in OP_SYMBOL(IC_LEFT(ic)) @ main.c:918: expected symbol, got value This code is extracted from onebyte.c regression test. Borut ---------------------------------------------------------------------- >Comment By: Borut Raem (borutr) Date: 2008-08-31 10:23 Message: Logged In: YES user_id=568035 Originator: YES Fixed in svn revision #5224, probably by fixing 2048464: PIC16: fix genUminus - addresses not.c regression test. Borut ---------------------------------------------------------------------- Comment By: Bernhard Held (bernhardheld) Date: 2006-06-07 08:16 Message: Logged In: YES user_id=203539 >> 8 x 8 = 8 ? > No quite, 64 more likely ;-) :-)) > No, really: The PICs implement u8 x u8 --> u16 > multiplication via MULLW/MULWF Fine, this is exactly what mcs51 is doing too. Please note: - the result can be 8 or 16 bits wide - the operands can be signed and/or unsigned, even mixed signdness is possible! You can use genMultOneByte() in mcs51/gen.c as a template. ---------------------------------------------------------------------- Comment By: Raphael Neider (tecodev) Date: 2006-06-06 22:56 Message: Logged In: YES user_id=1115835 OK, I will stick to solution (1) from Bernhard. > Can the PIC14/PIC16 do any hardware multiplication? Yes it can, > 8 x 8 = 8 ? No quite, 64 more likely ;-) No, really: The PICs implement u8 x u8 --> u16 multiplication via MULLW/MULWF using either a literal or a register as operand1 and the accumualtor (WREG) as second operand. > And how about division? No HW support whatsoever. Well, they do provide shift instructions and bittests... Regards, Raphael ---------------------------------------------------------------------- Comment By: Maarten Brock (maartenbrock) Date: 2006-06-06 19:55 Message: Logged In: YES user_id=888171 A bit off-topic: In addition to "SDCC never asks for a 16 x 16 = 32 bit multiplication" I want to say that some day it might. SiLabs (and maybe others) has an 8051 derivative with a 16 x 16 bit multiplication unit. And for others it also might be interesting to implement this in order to save some cycles from a 32 x 32 = 32 multiplication. However C99 defines a 16 x 16 multiplication as a 16 bit result without upcasts. So maybe the solution in the middle is to implement optimized solutions for 8 x 8 = 16, 8 x 16 = 16, 16 x 32 = 32 The above probably might become an RFE someday. Can the PIC14/PIC16 do any hardware multiplication? 8 x 8 = 8 ? And how about division? Maarten ---------------------------------------------------------------------- Comment By: Bernhard Held (bernhardheld) Date: 2006-06-06 09:00 Message: Logged In: YES user_id=203539 You've got everything right: SDCC expects (at the moment) from the code generator a s8 x s8 = s16 multiplication. The reason is simple: the mcs51 comes with a hardware multiplication u8 x u8 = u16, and SDCC takes special care not to promote the operands in order to efficiently use this opcode. I see two possible solutions: 1) You implement u8 x u8 = u16 and s8 x s8 = s16 multiplication in the pic16 port. I claim this is used quite often, and will help to compile efficient + fast code. 2) Adding the possibility to disable this feature on a per-port basis. If it's disabled in the given example SDCC will promote the two operands to s16 and ask for a s16 x s16 = s16 multiplication. BTW: SDCC never asks for a 16 x 16 = 32 bit mulitplication. ---------------------------------------------------------------------- Comment By: Raphael Neider (tecodev) Date: 2006-06-06 08:34 Message: Logged In: YES user_id=1115835 Hmm, the given example is void: neither can -150 be represented in 8 bit using 2's complement nor is -300=112 (mod 256). It should read: -100 * 4 = -400 = 112 (mod 256). The conclusions remain the same, though. Probably multiplication should always be defined as an N x N -> 2N bit operation (except for long, obviously) and the result be cropped as required? Raphael ---------------------------------------------------------------------- Comment By: Raphael Neider (tecodev) Date: 2006-06-05 23:50 Message: Logged In: YES user_id=1115835 I wonder why SDCC tried to multiply two signed chars here... Should it not promote them to int first, as the result is 16 bit wide (hm, possibly promotion rules apply only to input operands?)!? -150 * 2 should be -300, but will yield -144=112 with the current _mulschar: s8 x s8 -> s8. Sign-extending the result to 16 bit does not suffice (MSB is clear)... Modifying _mulschar's signature to s8 x s8 -> s16 would help, but it is not curerntly declared so. Am I wrong? Is SDCC wrong? How I hate promotion ;-) If I were to implement _mulschar: s8 x s8 -> s8, would I have to throw away the upper byte and sign-extend the result or can I use the unsigned case, which produces a correct 16 bit wide result! Or do we have to clip u8 x u8 -> u16 to 8 bits and clear the upper byte as well?!? Hoping for illumination, Raphael ---------------------------------------------------------------------- Comment By: Borut Raem (borutr) Date: 2006-05-21 12:43 Message: Logged In: YES user_id=568035 Now the linker displays the folowing error: error: missing definition for symbol "__mulschar", required by "t.o" The following code compiles without errors: void main(void) { volatile char cL; volatile char cR; volatile char r16; cL = -1; cR = 1; r16 = cL * cR; } It uses the "MOVFF PRODL, _main_r16_1_1" istruction for multiplication. The same approach should be probably used for the failing case. Borut ---------------------------------------------------------------------- Comment By: Raphael Neider (tecodev) Date: 2006-03-08 11:00 Message: Logged In: YES user_id=1115835 Fixed in src/pic16/main.c 1.68, SDCC #1221. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1444425&group_id=599 |