Bug 78035 - Inconsistency between address comparison and alias analysis
Summary: Inconsistency between address comparison and alias analysis
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 7.0
: P3 normal
Target Milestone: ---
Assignee: Richard Biener
URL:
Keywords: missed-optimization, wrong-code
Depends on:
Blocks:
 
Reported: 2016-10-19 08:50 UTC by Richard Biener
Modified: 2016-11-03 08:02 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2016-10-19 00:00:00


Attachments
prototype patch for the address comparison inconsistency (1.97 KB, patch)
2016-10-19 09:35 UTC, Richard Biener
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Biener 2016-10-19 08:50:10 UTC
Consider

extern int a;
extern int b;
extern int c;

int foo(int choose_a)
{
  int *p;
  if (choose_a)
    p = &a;
  else
    p = &b;
  return p != &c;
}

int bar ()
{
  return &a != &c;
}

where trunk optimizes foo to return 1 while leaving the address comparison
in bar alone.  This is because for bar we end up invoking symtab_node::equal_address_to which considers symbol interposition while
for simplifying foo we look at alias info (points-to) which considers
a, b and c distinct objects.

Either we are being too conservative in bar or we have a possible wrong-code
issue in foo considering the very same points-to information is also used
for alias analysis (and obviously direct accesses to a and b are not considered
aliasing either).
Comment 1 Richard Biener 2016-10-19 08:53:20 UTC
To ignore the memory disambiguation side and "fix" the address comparison side
only we could introduce a "points-to-interposable" flag and bail out whenever
we see that comparing two pointers using points-to.
Comment 2 Richard Biener 2016-10-19 08:54:55 UTC
It would be nice to split out a predicate for this from symtab_node::equal_address_to that I can invoke on a single symtab node
(or is there already one?  The code suggests that
->analyzed && decl_binds_to_current_def_p () is conservatively correct?
Comment 3 Richard Biener 2016-10-19 09:07:43 UTC
Adding Joseph for the address comparison optimization validity (with regarding to standard C but also other standards that may be relevant here)
Comment 4 Richard Biener 2016-10-19 09:35:06 UTC
Created attachment 39835 [details]
prototype patch for the address comparison inconsistency

Prototype patch.  Note it's too conservative (doesn't consider vtables specially, etc., as said, I'd like to have a symtab helper for this).
Comment 5 Alexander Monakov 2016-10-19 12:42:25 UTC
Note that for external function symbols from the standard library there's an old DR that clarifies that their addresses may compare equal, since the library may be implemented in a language other than C. See point c) in http://www.open-std.org/jtc1/sc22/wg14/www/docs/dr_078.html

But that is only for function declarations, I don't know if for object declarations it's the same or different. Credit goes to Szabolcs Nagy for pointing me to this DR.
Comment 6 rguenther@suse.de 2016-10-19 13:20:42 UTC
On Wed, 19 Oct 2016, amonakov at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78035
> 
> Alexander Monakov <amonakov at gcc dot gnu.org> changed:
> 
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>                  CC|                            |amonakov at gcc dot gnu.org
> 
> --- Comment #5 from Alexander Monakov <amonakov at gcc dot gnu.org> ---
> Note that for external function symbols from the standard library there's an
> old DR that clarifies that their addresses may compare equal, since the library
> may be implemented in a language other than C. See point c) in
> http://www.open-std.org/jtc1/sc22/wg14/www/docs/dr_078.html

"Not implemented in C" would apply to any extern function pair as the
compiler does not know wheter it's part of a "standard library"
(I think restricting the answer to standard library function is
misguided).

> But that is only for function declarations, I don't know if for object
> declarations it's the same or different. Credit goes to Szabolcs Nagy for
> pointing me to this DR.

Usually it's out of the scope of the C standard but we generally try
to honor ELF possibilities.
Comment 7 Richard Biener 2016-11-02 08:30:20 UTC
Author: rguenth
Date: Wed Nov  2 08:29:48 2016
New Revision: 241776

URL: https://gcc.gnu.org/viewcvs?rev=241776&root=gcc&view=rev
Log:
2016-11-02  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/78035
	PR tree-optimization/77964
	* gimple-pretty-print.c (pp_points_to_solution): Print
	vars_contains_interposable.
	* tree-ssa-alias.c: Include varasm.h.
	(ptrs_compare_unequal): Check vars_contains_interposable and
	decl_binds_to_current_def_p.
	(dump_points_to_solution): Dump vars_contains_interposable.
	* tree-ssa-alias.h (struct pt_solution): Add vars_contains_interposable
	flag.
	* tree-ssa-structalias.c: Include varasm.h.
	(set_uids_in_ptset): Record whether vars contains a
	not decl_binds_to_current_def_p variable in vars_contains_interposable.
	(ipa_escaped_pt): Update initializer.

	* gcc.target/i386/pr78035.c: New testcase.

Added:
    trunk/gcc/testsuite/gcc.target/i386/pr78035.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/gimple-pretty-print.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-alias.c
    trunk/gcc/tree-ssa-alias.h
    trunk/gcc/tree-ssa-structalias.c
Comment 8 Richard Biener 2016-11-02 08:30:53 UTC
Fixed.
Comment 9 krister.walfridsson 2016-11-02 21:36:37 UTC
Doesn't this just introduce more inconsistencies in the compiler? For example

    extern int a;
    extern int b;

    int foo(void)
    {
        a = 1;
        b = 5;
        a++;
        return &a != &b;
    }

optimizes to

    foo:
        movl    $a, %eax
        movl    $5, b(%rip)
        movl    $2, a(%rip)
        cmpq    $b, %rax
        setne   %al
        movzbl  %al, %eax
        ret

That is, the accesses to a and b are optimized as if they are distinct, even though the compiler keeps the comparison of the addresses.

I cannot think of a reasonable use case where you must handle comparisons of the addresses as currently implemented while allowing other optimizations as if the objects are distinct, so I'd say the bug from the original description is that we were "being too conservative in bar"...
Comment 10 rguenther@suse.de 2016-11-03 08:02:21 UTC
On Wed, 2 Nov 2016, krister.walfridsson at gmail dot com wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78035
> 
> krister.walfridsson at gmail dot com changed:
> 
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>                  CC|                            |krister.walfridsson at gmail dot c
>                    |                            |om
> 
> --- Comment #9 from krister.walfridsson at gmail dot com ---
> Doesn't this just introduce more inconsistencies in the compiler? For example
> 
>     extern int a;
>     extern int b;
> 
>     int foo(void)
>     {
>         a = 1;
>         b = 5;
>         a++;
>         return &a != &b;
>     }
> 
> optimizes to
> 
>     foo:
>         movl    $a, %eax
>         movl    $5, b(%rip)
>         movl    $2, a(%rip)
>         cmpq    $b, %rax
>         setne   %al
>         movzbl  %al, %eax
>         ret
> 
> That is, the accesses to a and b are optimized as if they are distinct, even
> though the compiler keeps the comparison of the addresses.
> 
> I cannot think of a reasonable use case where you must handle comparisons of
> the addresses as currently implemented while allowing other optimizations as if
> the objects are distinct, so I'd say the bug from the original description is
> that we were "being too conservative in bar"...

I opened this bug for the inconsistency in address comparisons which
now can utilize points-to analysis.  This part is fixed now.

Yes, for alias analysis we are less conservative than when optimizing
address comparisons ... but it appears that this is on purpose.