[avr-gcc-list] Problems with skip patterns

[Andy H]

Hi,

Here is an issue I would like you to review.


avr-gcc uses the SBRS and SBRC  instruction to both test and branch for 
certain bit tests.
Similary with I/O bits SBIC SBIS

These were originally peephole optimizations that were converted to 
peephole2 optimizations (the latter produce RTL rather than assembler 
directly)

While looking at multi-mode arithmetic operations I note we produce 
sub-optimal code.

In particular, we substitute SBRS/C for  TST and JUMP combinations of 
the sign bit.

The code produced by gcc 4.4 (and perhaps 4.3) only requires a simple 
Qimode TST regardless of operand mode (size)

TST Rx
BRMI

So the only time SBRx will win as a substitute is when jump is only over 
one instruction. Otherwise we have no better code of:

SBRx
RJMP

However, TST instruction are replaced by  gcc if the preceding 
instruction sets the status correctly. So,  for example

DEC Rx
TST Tx
BRMI

becomes

DEC Rx
BRMI

Because SBR* is substituted we loose this common optimisation and end up 
with worse code:

DEC Rx
SBRC Rx, 7
RJMP

The simple solution to the problem is to disable or delete the SBR* 
peephole2 optimization for sign bit tests. This will  produce an  
overall improvement.  I can raise a patch to do this.

The one circumstance in which SBR* Rx,7 can be a win is where the 
preceding instruction does not set the status AND the jump required is 
over only one instruction.
If I remember correctly, the original peephole version performed a check 
using "jump_over_one_insn_p" and did not substitute unless only one 
instrcution was jumped.

LDS RX,foo
SBRC Rx,7
-some  instruction of 1 word.

Unfortunately, the  jump distance calculation  is  only possible at code 
generation - so that works for peephole optimizations - not peephole2 
used here.

The original "jump_over_one_insn_p" is still used for all sb** code 
generation to determine required jumps and does  a rather poor job since 
it assumes each word is a separate  instruction.

Now it so happens I came across this issue before and wrote a 
replacement and also  equivalent RTL version to work with peephole2. 
Unlike the existing version it is more aggressive and looks for common 
two word instructions, as well as single word instructions. Using these 
would provide better code. But it will need updating.


/* Check if dest label is ok for skip instruction destination in this 
instruction insn
  SBxx is only valid over one asm (not RTL) instruction. One asm insn 
can be 1,2 or 3 words.
  return number of asm words skipped as true, 0= false if we can't work 
it out
  or jump is too far
  Ideally we need addresses to find displacement - we can only do this 
with
  a code peephole not a peephole2 (as addresses are not yet 
calculated). To get around this
  we get the length of following instructions from  length attribute. 
If length is 1
  it is skippable. We also look for specific patterns we know are one 
instruction long
  this includes (call, r<=[mem], [mem]<=r and jump). Finally we look 
for the label.
  This wont catch all of the possibilities
  TODO use adjust_Ins_length for more accurate length?????
*/

int jump_over_one_insn_p2(rtx insn, rtx operands[], rtx dest )

{
/* This is wrapper to get around problem where get_attr_length clobbers 
operand list! */
 rtx ops[8];
 ops[0]=operands[0];
 ops[1]=operands[1];
 ops[2]=operands[2];
 int ret =jump_over_one_insn_pxx(insn,dest);
 operands[0]=ops[0];
 operands[1]=ops[1];
 operands[2]=ops[2];
return ret; 
}

int
jump_over_one_insn_pxx (rtx insn, rtx dest)
{
 /* get dest instruction uid */
 int uid = INSN_UID (GET_CODE (dest) == LABEL_REF
             ? XEXP (dest, 0)
             : dest);
            
 /* skip next real instruction  - should be the jump*/
 rtx nexti=next_real_insn(insn);

 /*return false if there isn't one (bad md file)*/   
 if (!nexti) return 0;

 /* next real instruction is perhaps one we can skip*/
 nexti=next_real_insn(nexti);

 /*return false if there isn't one */   
 if (!nexti) return 0;

 /* length in words of skippable instruction */
 int len =0;

 rtx next=    PATTERN(nexti);

    
 /* ok skipping byte stores of zero or register into direct memory STS */
 if (GET_CODE(next)==SET)
   if (GET_MODE(XEXP(next,0))==QImode)
     if (GET_CODE(XEXP(next,0))==MEM)
       if (CONSTANT_P(XEXP(XEXP(next,0),0)))
         if ((GET_CODE(XEXP(next,1))==REG)||(XEXP(next,1)==const0_rtx))
           len=2;

        
 /* ok skipping byte loads of register from direct mem LDS */
 if (GET_CODE(next)==SET)
   if (GET_MODE(XEXP(next,1))==QImode)
     if (GET_CODE(XEXP(next,1))==MEM)
       if (CONSTANT_P(XEXP(XEXP(next,1),0)))     
         if ((GET_CODE(XEXP(next,0))==REG))
           len= 2;
  
 /*ok skipping over any non conditional jump - JUMP */
 if (GET_CODE(next)==SET)
   if (GET_CODE(XEXP(next,0))==PC)
     if (GET_CODE(XEXP(next,1))==LABEL_REF)
       len=2;

 /* ok skipping calls which are always ONE instruction but sometimes 2 
words*/
 if (GET_CODE(nexti)==CALL_INSN) len=2;


 /*finally single word RTL instructions are one asm instruction */
 int l=get_attr_length(nexti);
 l=adjust_insn_length(nexti,l);
 if (l==1) len=1;


 /* exit if instruction is not skippable */
 if (!len) return 0;

 /* jump target must be next non-note instruction */
 while ((nexti=NEXT_INSN(nexti)))
 {
   if (uid==INSN_UID(nexti))
       return  len;
   else if INSN_P (nexti)
       return 0;
 }

 /* lable target was not found  */
 return 0;
}
/* This is the same function but uses instruction addresses */
int
jump_over_one_insn_p (rtx insn, rtx dest)
{
 /* return false if we dont know instruction addresses*/
 if (!INSN_ADDRESSES_SET_P()) return 0;
 int uid = INSN_UID (GET_CODE (dest) == LABEL_REF
             ? XEXP (dest, 0)
             : dest);
        
 /* skip notes and look at next real instruction */
 rtx nexti=next_real_insn(insn);

 /*return false if there isn't one */   
 if (!nexti) return 0;

 /* Use this as baseline for displacment calculation */         
 int jump_addr = INSN_ADDRESSES (INSN_UID (nexti));
 int dest_addr = INSN_ADDRESSES (uid);
 int disp = dest_addr - jump_addr;
  
 /* valid displacments are 1,2 or 3 words */
 if ((disp <= 0)||(disp>3)) return 0;

 /* one word displacment always means only one asm instruction */
 if (disp == 1) return 1;

 /* ok skipping calls which are always ONE instruction but sometimes 2 
words*/
 if (GET_CODE(nexti)==CALL_INSN)
   if (disp == 2) return 2;
  
 rtx next=    PATTERN(next_real_insn(insn));     
 /* ok skipping byte stores of zero or register into direct memory STS */
 if (GET_CODE(next)==SET)
   if (GET_MODE(XEXP(next,0))==QImode)
     if (GET_CODE(XEXP(next,0))==MEM)
       if (CONSTANT_P(XEXP(XEXP(next,0),0)))
         if ((GET_CODE(XEXP(next,1))==REG)||(XEXP(next,1)==const0_rtx))
           if (disp == 2) return 2;
        
 /* ok skipping byte loads of register from direct mem LDS */
 if (GET_CODE(next)==SET)
   if (GET_MODE(XEXP(next,1))==QImode)
     if (GET_CODE(XEXP(next,1))==MEM)
       if (CONSTANT_P(XEXP(XEXP(next,1),0)))     
         if ((GET_CODE(XEXP(next,0))==REG))
           if (disp == 2) return 2;
          
 /*ok skipping over any non conditional jump to label - JUMP */
 /*ok skipping over any non conditional jump - JUMP */
 if (GET_CODE(next)==SET)
   if (GET_CODE(XEXP(next,0))==PC)
     if (GET_CODE(XEXP(next,1))==LABEL_REF)
       if (disp == 2) return 2;
    
 return 0;
}