Bug 17234 - if-conversion problem
Summary: if-conversion problem
Status: NEW
Alias: None
Product: gcc
Classification: Unclassified
Component: rtl-optimization (show other bugs)
Version: 4.0.0
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: missed-optimization
Depends on:
Blocks:
 
Reported: 2004-08-30 05:26 UTC by Dan Nicolaescu
Modified: 2021-06-08 09:04 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2005-12-24 20:45:20


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Dan Nicolaescu 2004-08-30 05:26:12 UTC
Compiling 

void foo (unsigned *, unsigned *, unsigned);
unsigned *baz (unsigned) __attribute__ ((const));

struct COST
{
  unsigned *cost;

  unsigned maxWeakConstraintLevel;
};

unsigned * bar(struct COST *c)
{
  unsigned *valp;
  if(c->maxWeakConstraintLevel == 0)
    valp =0;
  else
    {
      valp = baz (4 * 33);
      foo (valp, c->cost, c->maxWeakConstraintLevel * sizeof(unsigned));
    }
  return valp;
}

with -O2 -fomit-frame-pointer on x86 generates:

bar:
        subl    $28, %esp
        movl    %edi, 24(%esp)
        movl    32(%esp), %edi
        movl    %esi, 20(%esp)
        xorl    %esi, %esi            <- [1] this should not be here
        movl    %ebx, 16(%esp)
        movl    4(%edi), %ebx
        testl   %ebx, %ebx
        jne     .L6                 
        movl    %esi, %eax             <- it should be before this instruction
        movl    16(%esp), %ebx
        movl    20(%esp), %esi
        movl    24(%esp), %edi
        addl    $28, %esp
        ret
        .p2align 4,,7
.L6:
        movl    $132, (%esp)
        call    baz
        movl    %eax, %esi         <- %esi is not used on this branch, 
                                    so [1] is partially dead code

The instruction [1] is put there by the if-conversion pass, when compiling
with -fno-ifconversion [1] appears only on one branch. 

gcc-3.3.3 does not have this problem, so this is a regression.
Comment 1 Wolfgang Bangerth 2004-08-30 11:33:13 UTC
Confirmed. However, I can see the same problem with 3.2.3 and 3.3.4, so 
this doesn't seem to be a regression. Do I miss something, Dan? 
 
W. 
Comment 2 Dan Nicolaescu 2004-08-30 15:25:28 UTC
(In reply to comment #1)
> Confirmed. However, I can see the same problem with 3.2.3 and 3.3.4, so 
> this doesn't seem to be a regression. Do I miss something, Dan? 

I don't know, I only have access to one version of gcc on x86 at this moment,
here is what I get: 

bar:
        pushl   %esi
        pushl   %ebx
        pushl   %ebx
        movl    16(%esp), %ebx
        movl    4(%ebx), %ecx
        testl   %ecx, %ecx
        jne     .L2
        xorl    %esi, %esi   <- this is in the right place
.L3:
[snip]
        .ident  "GCC: (GNU) 3.3.3 20040412 (Red Hat Linux 3.3.3-7)"

Comment 3 roger 2004-09-05 00:50:54 UTC
This transformation is internally known as IF-CASE-1 and is performed during
the second if-conversion pass, .20.ce2.  From the CFG perspective this change
appears to be a win (at the point it is performed); by speculatively executing
a single instruction we reduce the CFG from four basic blocks to just three.

i.e.     0        0'
        / \       | \
       1   2  =>  |  2
        \ /       | /
         3        3

Normally, this simplification reduces the of unconditional branches in the
code, and allows for improved basic block reordering optimizations.
Unfortunately, in this case, basic block 3 marks the end of the function,
and we subsequently duplicate the function epilogue.  This later duplication
ends up turning both forms of the above CGF into 3 basic blocks, but the
if-conversion has pessimized the code with a partially dead assignment in
0' that could have remained in 3' (in the diagram below, reflecting the final CFG).

i.e.     0           0'
        / \    =>   / \
      1+3 2+3      3'  2+3

One solution may be to disable IF-CASE-1 is the join block is the last
basic block in the function, contains no additional instructions, and we're
expecting to duplicate the epilogue.

Jan, what do you think?  Is there an easy way to determine if the join block
of an IF-THEN-ELSE is a candidate for basic block duplication?
Comment 4 Steven Bosscher 2005-01-23 18:55:03 UTC
Honza, Roger asked you something in comment #3, but you're not in the 
CC: list, so you probably missed it.  Care to comment? 
Comment 5 Steven Bosscher 2005-06-25 13:17:07 UTC
Honza, ping ping ping !!!! 
Comment 6 Jan Hubicka 2006-04-30 14:24:08 UTC
Sorry, I've must missed the two pings and noticed the problem only now while housekeeping.
There is no easy way to peek cfglayout about what decisions it will do in future, so there is no easy
way to decide whether to duplicate or not.  I am not sure if this is important enought, but if we really care,
we might consider if-conversion to do the duplication itself for really small blocks.
Comment 7 Steven Bosscher 2007-07-25 19:43:21 UTC
This may actually have been fixed by my cfglayout work.
Comment 8 Andrew Pinski 2021-06-08 09:04:14 UTC
I can no longer reproduce this issue with a more recent compiler.  Also The order of the basic block changed.
So we have now:
entry
je L3
calls
L3:
ret

Most likely due to the heurstics changes.
Anyways I force the other order using __builtin_expect and I get:
bar:
        pushl   %esi
        xorl    %eax, %eax
        pushl   %ebx
        subl    $20, %esp
        movl    32(%esp), %esi
        movl    4(%esi), %ebx
        testl   %ebx, %ebx
        jne     .L8
        addl    $20, %esp
        popl    %ebx
        popl    %esi
        ret
        .p2align 4,,10
        .p2align 3
.L8:
        subl    $12, %esp
        sall    $2, %ebx
        pushl   $132
        call    baz
        addl    $12, %esp
        pushl   %ebx
        pushl   (%esi)
        pushl   %eax
        movl    %eax, 28(%esp)
        call    foo
        addl    $16, %esp
        movl    12(%esp), %eax
        addl    $20, %esp
        popl    %ebx
        popl    %esi
        ret

---- CUT ---
We duplicate the ret here even.