• State Not Answered
  • Locked Locked
  • Replies 9 replies
  • Subscribers 21 subscribers
  • Views 1684 views
  • Users 0 members are here
2025 survey

We are running a survey to help us improve the experience for all of our members. If you see the survey appear, please take the time to tell us about your experience if you can.

Options
Related
How was your experience today?
This discussion has been locked.
You can no longer post new replies to this discussion. If you have a question you can start a new discussion

unnecessary code generation

Heinz Saathoff

Hello,

I'm relatively new in using Keil C166 development system. First tests are successfully so far using the Keil MCB167NET board. As a long time C programmer I'm interested in the quality of generated code. I noticed that the generated code sometimes has unnecessary code lines that I thought an optimizer should be able to find. Here is a example:

#include <C167CS.H>
#include "comtype.h"
#include "timer.h"
void timer_init()
{  struct T01_CON {
      uint  T0L :3;
      uint  T0M :1;
      uint  un00:2;
      uint  T0R :1;
      uint  un01:1;
      uint  T1L :3;
      uint  T1M :1;
      uint  un10:2;
      uint  T1R :1;
      uint  un11:1;
   };
   union {
      struct T01_CON tcon;
      uint   reg;
   } u;
   u.reg = T01CON;
   u.tcon.T0L = 2;    // FCPU / 2^(3+T0L) = 2 µs at 16MHz
   u.tcon.T0M = 0;    // Timer Mode
   u.tcon.T0R = 1;    // run Timer
   T01CON = u.reg;
}//timer_init
The genearated code looks for speed optimizer, Level 7 is:
   ; FUNCTION timer_init (BEGIN  RMASK = @0x4030)
0000 2802          SUB       R0,#02H
0002 F2F550FF      MOV       R5,T01CON
0006 B850          MOV       [R0],R5       ; u
0008 A840          MOV       R4,[R0]       ; u
000A 0AF54F42      BFLDL     R5,#04FH,#042H
000E B850          MOV       [R0],R5       ; u
0010 A840          MOV       R4,[R0]       ; u
0012 F6F550FF      MOV       T01CON,R5
0016 0802          ADD       R0,#02H
0018 CB00          RET
   ; FUNCTION timer_init (END    RMASK = @0x4030)

At first I see that at Offset 0008 and 0010 the mov to R4 is unnecessary because R4 is never read again. Removing this moves a second look noticed that also the moves at offset 0006 and 000E are unnecessary.

Should this kind of optimization be doable by a peephole optimizer or by data flow analysis?

NB: This is not critical, just as a hint for future improvements.
The most important thing with optimizers is still the correctness of generated code.

- Heinz (from Delmenhorst, Germany)

Parents
  • 0

    It is typical to directly modify the bits in an SFR. For example, the following:

    void timer_init (void)
{
T01CON &= ~(0x0007 | 0x0008 | 0x0040);
T01CON |=  (0x0002 |          0x0040);
}

    Initializes the timer exactly as your example does but without the overhead of reading the SFR into a variable, changing the bits, and writing the SFR back.

    The code generated for the above is:

    0000 66A8B0FF      AND       T01CON,#0FFB0H
0004 76A84200      OR        T01CON,#042H
0008 CB00          RET

    Another problem with your example is that if it is interrupted and if any bits in T01CON are changed, then when the interrupt exits, your example undoes those changes.

    Jon

Reply
  • 0

    It is typical to directly modify the bits in an SFR. For example, the following:

    void timer_init (void)
{
T01CON &= ~(0x0007 | 0x0008 | 0x0040);
T01CON |=  (0x0002 |          0x0040);
}

    Initializes the timer exactly as your example does but without the overhead of reading the SFR into a variable, changing the bits, and writing the SFR back.

    The code generated for the above is:

    0000 66A8B0FF      AND       T01CON,#0FFB0H
0004 76A84200      OR        T01CON,#042H
0008 CB00          RET

    Another problem with your example is that if it is interrupted and if any bits in T01CON are changed, then when the interrupt exits, your example undoes those changes.

    Jon

Children
  • 0 in reply to Jon Ward

    Hello Jon,


    It is typical to directly modify the bits in an SFR. For example, the following:

    void timer_init (void)
{
T01CON &= ~(0x0007 | 0x0008 | 0x0040);
T01CON |=  (0x0002 |          0x0040);
}

    Yes, I changed it in a similar way now. Your hint with the interrupt in between read and write wouldn't be a problem as this initialization only happens ones at startup. It can be a problem in other situations, however.
    My original idea was to have field names in the sfr as found in the device data sheet.

    - Heinz