Menu
▾
▴
#1420 explicit typecast is ineffective for unsigned char parameter
The compiler seems to ignore the explicit (unsigned char) typecast when it is applied to the actual parameter of a function with variable arguments. (Explicit (unsigned char) typecast is necessary even for unsigned char type otherwise the compiler would automatically push a two byte integer onto the stack.) The bug for example blocks the use of printf() function with "%bu" format specifier.
C:\>sdcc -v
SDCC : mcs51/gbz80/z80/avr/ds390/pic16/pic14/TININative/xa51/ds400/hc08 2.7.4 #4972 (Nov 23 2007) (MINGW32)
C:\>ver
Microsoft Windows XP [Version 5.1.2600]
Logged In: YES
user_id=568035
Originator: NO
Maarten, what do you think about the command-line switch proposed by Laszlo? I have to say that I'm not very favorable to it...
I've found the piece of code which disables the integer promotion of varargs in case of explicit type cast: it is in SDCCast.c, function processParms(), condition statement around the comment /* Parameter was explicitly typecast; don't touch it. */. If the condition is removed the integer propagation is done even in case of explicit type casting.
And now a question: as far as I could find, the %b and %B printf modifiers, which take one byte parameter, are available only in device/lib/printf_large.c. Is this correct?
BTW: I found the %b modifier also in pic libraries: device/lib/pic16/libc/stdio, files printf_tiny.c and vfprintf.c, but the meaning is totally different: it means output in binary format.
Borut
Logged In: YES
user_id=888171
Originator: NO
I don't like to add another command-line switch for this.
I looked at the history of that piece that disables the promotion and it was added together with varargs support in revision #541. I also seem to remember that I once bumped into this issue of a lost cast when changing some code, but I can't find it anymore. Somehow I have the feeling that I broke this behaviour by removing 'unnecessary' casts in favour of better optimization. But this might just as well have been during a local test and was never commited.
Even though this is non-standard behaviour I would somehow like to keep it for SDCC. SDCC is targeted for small (8 bit) devices and I think this is a very useful extension.
Yes %b modifiers are only in printf_large as far as I know. And they have been there since the initial revision. See
http://sdcc.svn.sourceforge.net/viewvc/sdcc/trunk/sdcc/device/lib/vprintf.c?revision=3&view=markup
The pics use %b for something else as you found out. Maybe we should split and merge them. How about %B for bytes and %b for bits?
Maarten
Logged In: YES
user_id=568035
Originator: NO
Maarten, if I understand you correctly, you are more favorable to the solution proposed by Lazslo. In this case the following condition should be removed from file SDCCasc.c, function decoreteType(), case CAST:
/* if 'from' and 'to' are the same remove the superfluous cast, */
/* this helps other optimizations */
if (compareTypeExact (LTYPE(tree), RTYPE(tree), -1) == 1)
{
return tree->right;
}
This lines were added in svn revision #4124 by Bernhard Held on 2006-04-25 with the comment: "partial fix for RFE 1475769, remove cast to same type". If we revert this change, we will probably have other problems and lose some optimizations, as you wrote, and we would not be compliant with the ISO C standard.
Maybe introduction of a command line switch is not so bad idea after all...
Borut
Logged In: YES
user_id=568035
Originator: NO
The better solution would be to execute the if (compareTypeExact(...) == 1) condition only in case if the left operand of cast is not actual function parameter:
if (!ACTUAL_FUNCTION_PARAMETER (tree->right) && compareTypeExact (LTYPE(tree), RTYPE(tree), -1) == 1)
{
return tree->right;
}
Now the problem is how the ACTUAL_FUNCTION_PARAMETER() should be implemented: the CAST is evaluated before the CALL, so we probably don't know that the operand is a function parameter?
Any idea?
Borut
Logged In: YES
user_id=888171
Originator: NO
Apart from introducing a new command line switch I don't see a solution by Laszlo. SDCC has had special behaviour for this from the beginning and allthough non-standard I don't see a reason to reject it now.
But I'm really not in favour of yet another switch. And thinking of it now, why not use --std-cXX vs. --std-sdccXX switches. If this is a SDCC specific language extension then it should be enabled by --std-sdccXX and disabled by --std-cXX.
Is this code in decorateType() the part that removes the cast in this case? Maybe we should find a way to remember that it was an explicit or implicit cast. And also use that in processParms like literalFromCast.
Maarten
Logged In: YES
user_id=568035
Originator: NO
Fixed as proposed in my previous post: casts of the same type of actual function parameters aren't removed (optimized). The ISO C standard nonconformity (or sdcc extension) remains and backward compatibility is not broken.
Borut
Logged In: YES
user_id=888171
Originator: NO
Borut,
Unless I am mistaken this holding on to the cast is only relevant for varargs functions. Looking at your patch is it not possible to change .actualArgument to .actualVarArg and only set it for a varargs function? This could keep the loss of other optimizations low.
Maarten
Logged In: YES
user_id=568035
Originator: NO
Maarten, you are right. I committed the version which implements the values.removedCast flag, as you proposed. Please review the fix. I'm not sure if testing for IS_AST_SYM_VALUE (*actParm) is correct and needed when testing for AST_VALUES (*actParm, removedCast).
Borut