Bug 24689 - operand_equal_p does not return true for some equivalent ARRAY_REF
Summary: operand_equal_p does not return true for some equivalent ARRAY_REF
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: unknown
: P3 enhancement
Target Milestone: 4.3.0
Assignee: Richard Biener
URL:
Keywords: missed-optimization
Depends on: 31307
Blocks:
  Show dependency treegraph
 
Reported: 2005-11-06 02:26 UTC by Kazu Hirata
Modified: 2007-04-12 10:28 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2007-04-11 11:13:33


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Kazu Hirata 2005-11-06 02:26:51 UTC
Consider:

extern void bar (unsigned int);

int
foo (void)
{
  char buf[1] = { 3 };
  const char *p = buf;
  const char **q = &p;
  unsigned int ch;
  switch (**q)
    {
    case 1:  ch = 5; break;
    default: ch = 0; break;
    }

#if 1
  bar (ch);
#endif
  return ch;
}

This function should be optimized to "return 0;", but it isn't.

Interestingly, if you change "#if 1" to "#if 0", you are going to get
this optimized.

The testcase is derived from PR 20583.
Comment 1 Andrew Pinski 2005-11-06 04:34:22 UTC
Looks like another one of the call clobbered bugs:
Variable: buf, UID 1612, char[1], is an alias tag, is addressable, call clobbered, default def: buf_2

Though that should not matter.
Here is a testcase where call clobberness is correct but it should not matter as there is no way for the change to happen:
extern void bar (unsigned int);
extern void bar1 (const char **);

int
foo (void)
{
  char buf[1] = { 3 };
  const char *p = buf;
  const char **q = &p;
  unsigned int ch;
  switch (**q)
    {
    case 1:  ch = 5; break;
    default: ch = 0; break;
    }

  bar (ch);
  bar1 (q);
  return ch;
}
Comment 2 Andrew Pinski 2005-11-06 04:47:12 UTC
Note switch statements here have nothing to do with the real issue:
extern void bar (unsigned int);
extern void bar1 (const char **);

int
foo (void)
{
  char buf[1] = { 3 };
  const char *p = buf;
  const char **q = &p;
  unsigned int ch;
  if(**q)
    {
            ch = 1;
    }
  else ch =2;

  bar (ch);
  bar1 (q);
  return ch;
}
Comment 3 Andrew Pinski 2005-11-06 04:50:28 UTC
(In reply to comment #0)
> This function should be optimized to "return 0;", but it isn't.
> Interestingly, if you change "#if 1" to "#if 0", you are going to get
> this optimized.
Not really because SRA works then, using -fno-tree-sra will show the same issue.
Comment 4 Andrew Pinski 2005-11-06 04:52:57 UTC
Here is another testcase:
extern void bar (unsigned int);
extern void bar1 (const char **);

char buf[1];
int
foo (void)
{
  const char *p = buf;
  const char **q = &p;
  buf[0] = 3;
  unsigned int ch;
  if(**q)
    {
            ch = 1;
    }
  else ch =2;

  return ch;
}

Comment 5 Andrew Pinski 2006-01-15 21:35:13 UTC
I am going to look into this soon but I don't think it is an aliasing issue as shown by:
  #   SFT.0_6 = V_MUST_DEF <SFT.0_5>;
  buf[0] = 3;
  #   VUSE <SFT.0_6>;
  D.1523_8 = buf[0];
Comment 6 Andrew Pinski 2006-01-16 06:02:49 UTC
The problem here is that operand_equal_p returns false for these two trees:
<array_ref 0x41da4b94
    type <integer_type 0x41d1b180 char sizes-gimplified public QI
        size <integer_cst 0x41d0a320 constant invariant 8>
        unit size <integer_cst 0x41d0a340 constant invariant 1>
        align 8 symtab 0 alias set -1 precision 8 min <integer_cst 0x41d0a3a0 -128> max <integer_cst 0x41d0a420 127>
        pointer_to_this <pointer_type 0x41d23120>>
   
    arg 0 <var_decl 0x41da2540 buf
        type <array_type 0x41da24e0 type <integer_type 0x41d1b180 char>
            QI size <integer_cst 0x41d0a320 8> unit size <integer_cst 0x41d0a340 1>
            align 8 symtab 0 alias set -1 domain <integer_type 0x41da2480>>
        addressable used public static common QI file t.c line 4 size <integer_cst 0x41d0a320 8> unit size <integer_cst 0x41d0a340 1>
        align 8
        chain <function_decl 0x41da3680 foo type <function_type 0x41d236c0>
            public static SI file t.c line 7 initial <block 0x41da4bfc> result <result_decl 0x41d12268 D.1518>
            (mem:SI (symbol_ref:SI ("foo") [flags 0x103] <function_decl 0x41da3680 foo>) [0 S4 A8])
            saved-insns 0x41d85c00>>
    arg 1 <integer_cst 0x41d0ab40 type <integer_type 0x41d1b2a0 int> constant invariant 0>>

and
 <array_ref 0x41dab6b4
    type <integer_type 0x41d23300 char readonly sizes-gimplified public QI
        size <integer_cst 0x41d0a320 constant invariant 8>
        unit size <integer_cst 0x41d0a340 constant invariant 1>
        align 8 symtab 0 alias set -1 precision 8 min <integer_cst 0x41d0a3a0 -128> max <integer_cst 0x41d0a420 127>
        pointer_to_this <pointer_type 0x41d23360>>
   
    arg 0 <var_decl 0x41da2540 buf
        type <array_type 0x41da24e0 type <integer_type 0x41d1b180 char>
            QI size <integer_cst 0x41d0a320 8> unit size <integer_cst 0x41d0a340 1>
            align 8 symtab 0 alias set -1 domain <integer_type 0x41da2480>>
        addressable used public static common QI file t.c line 4 size <integer_cst 0x41d0a320 8> unit size <integer_cst 0x41d0a340 1>
        align 8
        chain <function_decl 0x41da3680 foo type <function_type 0x41d236c0>
            public static SI file t.c line 7 initial <block 0x41da4bfc> result <result_decl 0x41d12268 D.1518>
            (mem:SI (symbol_ref:SI ("foo") [flags 0x103] <function_decl 0x41da3680 foo>) [0 S4 A8])
            saved-insns 0x41d85c00>>
    arg 1 <integer_cst 0x41d76600 type <integer_type 0x41da2480> constant invariant 0>
    arg 2 <integer_cst 0x41d0a2a0 type <integer_type 0x41d1b000 long unsigned int> constant invariant 0> arg 3 <integer_cst 0x41d0a340 1>>

Even they point to the same spot, it does not work as the two array reference have different arg2/arg3.

So Store_CCP is doing the correct thing, just the function it depends on is not.

And the reason for this problem is because when changing **q into buf[0], it does not fill out those two arguments to the tree.
Comment 7 Richard Biener 2007-04-11 11:13:33 UTC
I have a patch.
Comment 8 Andrew Pinski 2007-04-11 13:03:46 UTC
I should make a note here that if operand_equal_p returns true, iterative hash has to be the same also.
Comment 9 Richard Biener 2007-04-11 14:51:15 UTC
Luckily iterative hash hashes low/high of integer consts only, not the type.
Comment 10 Richard Biener 2007-04-12 10:16:12 UTC
Subject: Bug 24689

Author: rguenth
Date: Thu Apr 12 10:15:53 2007
New Revision: 123736

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=123736
Log:
2007-04-12  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/24689
	PR tree-optimization/31307
	* fold-const.c (operand_equal_p): Compare INTEGER_CST array
	indices by value.
	* gimplify.c (canonicalize_addr_expr): To be consistent with
	gimplify_compound_lval only set operands two and three of
	ARRAY_REFs if they are not gimple_min_invariant.  This makes
	it never at this place.
	* tree-ssa-ccp.c (maybe_fold_offset_to_array_ref): Likewise.

	* g++.dg/tree-ssa/pr31307.C: New testcase.
	* gcc.dg/tree-ssa/pr24689.c: Likewise.

Added:
    trunk/gcc/testsuite/g++.dg/tree-ssa/pr31307.C
    trunk/gcc/testsuite/gcc.dg/tree-ssa/pr24689.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/fold-const.c
    trunk/gcc/gimplify.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-ccp.c

Comment 11 Richard Biener 2007-04-12 10:28:28 UTC
Fixed.