[sdcc-devel] [ sdcc-Feature Requests-1582704 ] constant bool returned inefficiently

[SourceForge.net <no...@so...>]

Feature Requests item #1582704, was opened at 2006-10-23 10:25
Message generated for change (Comment added) made by maartenbrock
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=350599&aid=1582704&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: None
Group: None
>Status: Closed
Priority: 5
Submitted By: Frieder Ferlemann (frief)
Assigned to: Maarten Brock (maartenbrock)
Summary: constant bool returned inefficiently

Initial Comment:
Instead of "setb C" or "clr C" an addition with the
accumulator is done.

Strangely return_zero() returns with acc set to 0xff.
(this suggests that the value in acc is nowhere in use:)


---------------8<--------------------------
unsigned char t;

bit return_zero()
{
 /* clr     a
    add     a,#0xff

    carry=0, why acc==0xff?
    (would have expected acc==0x00)
  */
 return 0;
}

bit return_nonzero()
{
 /* mov     a,#0x01
    add     a,#0xff

    carry=1, why acc==0x00? 
  */
 return 1;
}

/* if fixed this one could return bit */
unsigned char _sdcc_external_startup (void)
{
  return 0;
}

void main()
{
  if( return_zero() )
    t++;
  if( return_nonzero() )
    t++;
}
--------------->8--------------------------

----------------------------------------------------------------------

>Comment By: Maarten Brock (maartenbrock)
Date: 2006-10-23 16:53

Message:
Logged In: YES 
user_id=888171

My fix is along these lines yes. SDCC 2.6.1 #4432

I'm not sure what it will do when returning any other 
constant than 0 or 1. I hope the iCode generation already 
changes that to 1.

----------------------------------------------------------------------

Comment By: Bernhard Held (bernhardheld)
Date: 2006-10-23 15:19

Message:
Logged In: YES 
user_id=203539

No peephole, please. This little patch fixes your special
case. Here the string "#0x00" is emitted by
aopLiteralLong(), therefore movc() doesn't catch it. There
are some more occurences of "(s == zero)" resp. "(s == one)"
in gen.c. Maarten certainly knows best if these should be
modified too.

Index: sdcc/src/mcs51/gen.c
===================================================================
--- sdcc/src/mcs51/gen.c    (revision 4419)
+++ sdcc/src/mcs51/gen.c    (working copy)
@@ -244,9 +244,9 @@
 static void
 movc (const char *s)
 {
-  if (s == zero)
+  if (strcmp (s, zero) == 0)
     CLRC;
-  else if (s == one)
+  else if (strcmp (s, one) == 0)
     SETC;
   else if (strcmp (s, "c"))
     {/* it's not in carry already */


----------------------------------------------------------------------

Comment By: Maarten Brock (maartenbrock)
Date: 2006-10-23 14:38

Message:
Logged In: YES 
user_id=888171

When I implemented this my first concern was generating 
correct code. There are still several configurations that 
can use some improvements. Using a peephole will fix this, 
but I think it's better to improve the iCodes or code 
generation. This particular case is easy I guess. In gen.c 
movc() checks the address of a string s against the 
constant string one instead of the contents.

----------------------------------------------------------------------

Comment By: Frieder Ferlemann (frief)
Date: 2006-10-23 13:39

Message:
Logged In: YES 
user_id=589052

Ooops, should have been:

        ;       Peephole 265.b  return bitwise 1
        setb    c
        ret


----------------------------------------------------------------------

Comment By: Frieder Ferlemann (frief)
Date: 2006-10-23 13:30

Message:
Logged In: YES 
user_id=589052

Would you recommend fixing this with peephole rules?

(the rules include a check for "ret" so might not "catch
them all")

---8<-------------------------------

replace {
        clr     a
        add     a,#0xff
        ret
} by {
        ;       Peephole 265.a  return bitwise 0
        clr     c
        ret
}

replace {
        mov     a,#0x01
        add     a,#0xff
        ret
} by {
        ;       Peephole 265.b  return bitwise 1
        clr     c
        ret
}


----------------------------------------------------------------------

Comment By: Bernhard Held (bernhardheld)
Date: 2006-10-23 11:23

Message:
Logged In: YES 
user_id=203539

Just a short explanation: return values of type 'bit' are
returned in the carry-flag. The content of acc is ignored.

; load return value
clr a

; convert 'unsigned char' to bool:
; CY = (ACC == 0 ? 0 : 1);
add a,#0xff

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=350599&aid=1582704&group_id=599