public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug middle-end/114111] New: [avr] Expensive code instead of conditional branch.
@ 2024-02-26 11:42 gjl at gcc dot gnu.org
  2024-02-26 15:02 ` [Bug middle-end/114111] " rguenth at gcc dot gnu.org
  2024-02-26 16:49 ` pinskia at gcc dot gnu.org
  0 siblings, 2 replies; 3+ messages in thread
From: gjl at gcc dot gnu.org @ 2024-02-26 11:42 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114111

            Bug ID: 114111
           Summary: [avr] Expensive code instead of conditional branch.
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: gjl at gcc dot gnu.org
  Target Milestone: ---

Created attachment 57541
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57541&action=edit
addcc.c: C test case

Compile the code with avr-gcc -S -Os -dp:

int add_ge0 (int x, char c) {
    return x + (c >= 0);
}

int add_eq0 (int x, char c) {
    return x + (c == 0);
}

int add_le0 (int x, char c) {
    return x + (c <= 0);
}

int add_ge1 (int x, char c) {
    return x + (c >= 1);
}

int add_ltm3 (int x, char c) {
    return x + (c < -3);
}

int add_bit6 (int x, char c) {
    return x + !!(c & (1 << 6));
}

int add_nbit6 (int x, char c) {
    return x + !(c & (1 << 6));
}

All these could be performed by a test and the addition of x in an if-block. 
But what the compiler does is to extend the 8-bit value c to 16 bit, then
complement it, then shift the MSB to the LSB:

add_ge0:
        mov __tmp_reg__,r22      ;  23  [c=12 l=3]  *extendqihi2/0
        lsl r0  
        sbc r23,r23
        com r22  ;  24  [c=8 l=2]  *one_cmplhi2
        com r23
        bst r23,7        ;  31  [c=16 l=4]  *lshrhi3_const/3
        clr r22
        clr r23
        bld r22,0
        add r24,r22      ;  26  [c=8 l=2]  *addhi3/0
        adc r25,r23
        ret              ;  29  [c=0 l=1]  return

Even when it does a conditional to set the addend, it should rather have the
addition in the if-block (and moving x to R18 adds even more bloat):

add_eq0:
        mov r18,r24      ;  44  [c=4 l=1]  movqi_insn/0
        mov r19,r25      ;  45  [c=4 l=1]  movqi_insn/0
        ldi r24,lo8(1)   ;  46  [c=4 l=2]  *movhi/4
        ldi r25,0       
        cp r22, __zero_reg__     ;  47  [c=4 l=1]  cmpqi3/0
        breq .L3                 ;  48  [c=4 l=1]  branch
        ldi r24,0                ;  43  [c=4 l=2]  *movhi/1
        ldi r25,0       
.L3:
        add r24,r18      ;  42  [c=8 l=2]  *addhi3/0
        adc r25,r19
        ret              ;  51  [c=0 l=1]  return

...
        .ident  "GCC: (GNU) 14.0.1 20240212 (experimental)"

With avr-gcc 3.4.6 from around 2006, the generated code is as follows:

add_ge0:
        sbrs r22,7       ;  38  *sbrx_branch    [length = 2]
        adiw r24,1       ;  15  *addhi3/2       [length = 1]
.L2:
        ret      ;  37  return  [length = 1]

add_eq0:
        tst r22  ;  13  tstqi   [length = 1]
        brne .L4         ;  14  branch  [length = 1]
        adiw r24,1       ;  15  *addhi3/2       [length = 1]
.L4:
        ret      ;  35  return  [length = 1]

etc.  So at some point in time GCC lost all that smartness.

Appears to be around emit_stor_flag and friends; as far as I can see it doesn't
even try to work out costs.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [Bug middle-end/114111] [avr] Expensive code instead of conditional branch.
  2024-02-26 11:42 [Bug middle-end/114111] New: [avr] Expensive code instead of conditional branch gjl at gcc dot gnu.org
@ 2024-02-26 15:02 ` rguenth at gcc dot gnu.org
  2024-02-26 16:49 ` pinskia at gcc dot gnu.org
  1 sibling, 0 replies; 3+ messages in thread
From: rguenth at gcc dot gnu.org @ 2024-02-26 15:02 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114111

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
           Keywords|                            |missed-optimization
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2024-02-26
             Target|                            |avr

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
I think RTL expansion only (if even) considers BRANCH_COST.  I also think
that while we have if () to non-branchy code conversion we don't have the
reverse on GIMPLE so RTL expansion sees code like

  _7 = c_3(D) & 64;
  _1 = _7 != 0;
  _2 = (int) _1;
  _5 = _2 + x_4(D);
  return _5;

and when setcc is available it doesn't consider test & branch.  It would
only effectively do

  if (_7 != 0)
    _1 = 1;
  else
    _1 = 0;
  _2 = (int) _1;
  _5 = _2 + x_4(D);
  return _5;

so probably not help much in practice unless we move the computation
below back into the branch during RTL optimization.

This possibly asks for a better GIMPLE representation, at least for the
purpose of getting good code for AVR.  RTL expansion probably isn't the
best place to fix this.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [Bug middle-end/114111] [avr] Expensive code instead of conditional branch.
  2024-02-26 11:42 [Bug middle-end/114111] New: [avr] Expensive code instead of conditional branch gjl at gcc dot gnu.org
  2024-02-26 15:02 ` [Bug middle-end/114111] " rguenth at gcc dot gnu.org
@ 2024-02-26 16:49 ` pinskia at gcc dot gnu.org
  1 sibling, 0 replies; 3+ messages in thread
From: pinskia at gcc dot gnu.org @ 2024-02-26 16:49 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114111

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Maybe this is something that could be done during isel to undo what was done in
phiopt ...

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-02-26 16:49 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-26 11:42 [Bug middle-end/114111] New: [avr] Expensive code instead of conditional branch gjl at gcc dot gnu.org
2024-02-26 15:02 ` [Bug middle-end/114111] " rguenth at gcc dot gnu.org
2024-02-26 16:49 ` pinskia at gcc dot gnu.org

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).