User account creation filtered due to spam.

Bug 33315 - stores not commoned by sinking
Summary: stores not commoned by sinking
Status: ASSIGNED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 4.3.0
: P2 enhancement
Target Milestone: ---
Assignee: Richard Biener
URL:
Keywords: missed-optimization
Depends on: 23286 30905
Blocks: 11832 16996
  Show dependency treegraph
 
Reported: 2007-09-05 18:53 UTC by Pranav Bhandarkar
Modified: 2017-01-16 07:18 UTC (History)
10 users (show)

See Also:
Host:
Target: arm-none-eabi
Build:
Known to work:
Known to fail:
Last reconfirmed: 2007-09-05 20:34:09


Attachments
Sample Testcase (129 bytes, text/x-csrc)
2007-09-05 19:03 UTC, Pranav Bhandarkar
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Pranav Bhandarkar 2007-09-05 18:53:36 UTC
if ( x == 8 ) statement1
if ( x != 8 ) statement1
if ( x == 9 ) statement2
if ( x != 9 ) statement2
should be replaced by

statement1
statement2

However this doesnt happen and compare and jumps do get generated.
Comment 1 Pranav Bhandarkar 2007-09-05 19:03:05 UTC
Created attachment 14158 [details]
Sample Testcase
Comment 2 Richard Biener 2007-09-05 20:34:07 UTC
This is related to PR32306 and to the fact that we don't have a code hoisting pass.  And related to PR30905 because cross-jumping fixes this up on the
rtl-level for both gcc 4.1 and 4.2:

test:
        pushl   %ebp
        movl    %esp, %ebp
        popl    %ebp
        movl    $0, a
        movl    $0, a+4
        movl    $0, a+8
        ret
Comment 3 Steven Bosscher 2007-11-10 00:27:07 UTC
As a missed optimization, this bug adds new information.  But as a regression, this is a dup of bug 30905.
Comment 4 Joerg Wunsch 2007-11-23 10:57:45 UTC
Is the missed optimization in the following code the same?

volatile unsigned char *reg_a = (unsigned char *)42;
volatile unsigned char *reg_b = (unsigned char *)34;

extern void a(void);
extern void b(void);
extern void c(void);

void
decide(void)
{
        signed char diff;

        diff = *reg_a - *reg_b;

        if (diff < 0)
                a();
        else if (diff == 0)
                b();
        else if (diff > 0)
                c();
}

The third "if" statement is partially executed: it apparently remembered
that diff could not be less than 0, but it still tests against 0 even
though that test has just been done before.  Verified on both, the AVR
and i386 target.

Interestingly, by just reordering the code, the third condition will
be eliminated by GCC 4.x (but not by GCC 3.x):

volatile unsigned char *reg_a = (unsigned char *)42;
volatile unsigned char *reg_b = (unsigned char *)34;

extern void a(void);
extern void b(void);
extern void c(void);

void
decide(void)
{
        signed char diff;

        diff = *reg_a - *reg_b;

        if (diff < 0)
                a();
        else if (diff > 0)
                c();
        else if (diff == 0)
                b();
}

If someone thinks that's an entirely different thing than the subject of this
bug, please tell me so, and I'll submit a separate one.
Comment 5 Dave Edwards 2008-10-21 08:28:43 UTC
Subject: RE:  If condition not getting eliminated


Hi Ramana,

Please could you add sdkteam-gnu@icerasemi.com - then we all get to see
it ;-)

Cheers,


-----Original Message-----
From: ramana at icerasemi dot com [mailto:gcc-bugzilla@gcc.gnu.org] 
Sent: 21 October 2008 08:04
To: Dave Edwards
Subject: [Bug middle-end/33315] If condition not getting eliminated



-- 

ramana at icerasemi dot com changed:

           What    |Removed                     |Added
------------------------------------------------------------------------
----
                 CC|                            |ramana at icerasemi dot
com


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33315

------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
Comment 6 Andrew Pinski 2012-02-04 02:33:16 UTC
tail merge should be able to do this.  It currently does not though for the provided testcase in comment #1 but that is PR 52009.
Also note cselim should be able to do it too.
Comment 7 Richard Biener 2016-07-12 13:53:27 UTC
comment#4 is long fixed (in DOM).  In the original testcase the redundant
conditons are eliminated but we retain

test ()
{
  int i;

  <bb 2>:
  i_8 = num;
  if (i_8 == 1)
    goto <bb 3>;
  else
    goto <bb 6>;

  <bb 3>:
  MEM[(int *)&a] = 0;
  MEM[(int *)&a + 4B] = 0;
  goto <bb 5>;

  <bb 4>:
  MEM[(int *)&a + 4B] = 0;

  <bb 5>:
  MEM[(int *)&a + 8B] = 0;
  return;

  <bb 6>:
  MEM[(int *)&a] = 0;
  if (i_8 == 2)
    goto <bb 4>;
  else
    goto <bb 7>;

  <bb 7>:
  MEM[(int *)&a + 4B] = 0;
  goto <bb 5>;


because nothing sinks the common stores and tail-merging does only obfuscate
the CFG.  So this didn't really have sth to do with PR23286.  But the
complaint that the if condition is not eliminated is no longer true.

transmogrifying bug.
Comment 8 Martin Sebor 2017-01-13 17:23:54 UTC
With today's top of trunk (GCC 7.0) I see the following in the optimized dump.  The redundant stores seem to have been eliminated (they are still present in GCC 6).  Therefore resolving as fixed (please reopen if I missed something).

test ()
{
  int i;

  <bb 2> [100.00%]:
  i_8 = num;
  if (i_8 == 1)
    goto <bb 3>; [34.00%]
  else
    goto <bb 6>; [66.00%]

  <bb 3> [34.00%]:
  MEM[(int *)&a] = 0;
  goto <bb 5>; [100.00%]

  <bb 4> [34.00%]:
  MEM[(int *)&a + 4B] = 0;

  <bb 5> [100.00%]:
  MEM[(int *)&a + 8B] = 0;
  return;

  <bb 6> [66.00%]:
  MEM[(int *)&a] = 0;
  if (i_8 == 2)
    goto <bb 4>; [51.52%]
  else
    goto <bb 7>; [48.48%]

  <bb 7> [32.00%]:
  MEM[(int *)&a + 4B] = 0;
  goto <bb 5>; [100.00%]

}
Comment 9 Richard Biener 2017-01-16 07:14:52 UTC
There are still two stores to a[0] and two to a[1].
Comment 10 Richard Biener 2017-01-16 07:18:39 UTC
I've posted a patch for this in July (but it had fallout, as expected...).  IL after that fix:

test ()
{
  int i;

  <bb 2> [100.00%]:
  i_8 = num;
  MEM[(int *)&a] = 0;
  MEM[(int *)&a + 4B] = 0;
  MEM[(int *)&a + 8B] = 0;
  return;

}