Bug 26197 - [4.2 regression] ICE in is_old_name with vectorizer
Summary: [4.2 regression] ICE in is_old_name with vectorizer
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 4.2.0
: P2 normal
Target Milestone: 4.2.0
Assignee: Not yet assigned to anyone
URL: http://gcc.gnu.org/ml/gcc-patches/200...
Keywords: ice-on-valid-code, monitored, patch
: 26518 26948 27474 (view as bug list)
Depends on:
Blocks: 26518 27474
  Show dependency treegraph
 
Reported: 2006-02-09 14:51 UTC by Volker Reichelt
Modified: 2006-08-29 17:04 UTC (History)
11 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2006-02-16 17:53:48


Attachments
tentative patch (1.26 KB, patch)
2006-02-28 08:26 UTC, Dorit Naishlos
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Volker Reichelt 2006-02-09 14:51:39 UTC
The following testcase triggers an ICE when compiled with
"-O2 -ftree-vectorize" on x86_64-unknown-linux-gnu (mainline):

=====================================================
struct A
{
  int* x[2];
  A() { for (int** p=x; p<x+2; ++p) *p = 0; }
};

struct B
{
  char c;
  A a0, a1, a2, a3, *p;

  B() {}
  B(const B& b) : c(), a0(b.a0), p(b.p) {}
  ~B() { delete p; }
};

inline void foo(const B& b) { new B(b); }

void bar()
{
  foo(B());
  foo(B());
}
=====================================================

bug.cc: In function 'void bar()':
bug.cc:19: internal compiler error: tree check: expected ssa_name, have struct_field_tag in is_old_name, at tree-into-ssa.c:466
Please submit a full bug report, [etc.]

This is similar to PR 26169, but hasn't been fixed yet.
Comment 1 Andrew Pinski 2006-02-09 15:10:13 UTC
Confirmed.  (there is a missed optimization here also which Richard G. and I am looking at right now).

Anyways,
Before:
  # p_247 = PHI <&D.2477.a0.x[0](2)>;
<L183>:;
  #   TMT.32_246 = V_MAY_DEF <TMT.32_255>;
  *p_247 = 0B;

After:
#   SFT.12 = V_MAY_DEF <SFT.12>;
#   TMT.32_246 = V_MAY_DEF <TMT.32_255>;
D.2477.a0.x[0] = 0B;

We forgot to rename SFT.12.
Comment 2 Andrew Pinski 2006-02-09 15:45:27 UTC
(In reply to comment #1)
> Confirmed.  (there is a missed optimization here also which Richard G. and I am
> looking at right now).

This missed optimization is filed as PR 26198.
Comment 3 Jeffrey A. Law 2006-02-09 16:03:51 UTC
Subject: Re:  [4.2 regression] ICE in
	is_old_name, at tree-into-ssa.c:466

On Thu, 2006-02-09 at 15:10 +0000, pinskia at gcc dot gnu dot org wrote:
> 
> ------- Comment #1 from pinskia at gcc dot gnu dot org  2006-02-09 15:10 -------
> Confirmed.  (there is a missed optimization here also which Richard G. and I am
> looking at right now).
> 
> Anyways,
> Before:
>   # p_247 = PHI <&D.2477.a0.x[0](2)>;
> <L183>:;
>   #   TMT.32_246 = V_MAY_DEF <TMT.32_255>;
>   *p_247 = 0B;
> 
> After:
> #   SFT.12 = V_MAY_DEF <SFT.12>;
> #   TMT.32_246 = V_MAY_DEF <TMT.32_255>;
> D.2477.a0.x[0] = 0B;
> 
> We forgot to rename SFT.12.
When does the unnamed SFT.12 first appear?

jeff


Comment 4 Jeffrey A. Law 2006-02-10 20:24:36 UTC
This is a bug in the vectorizer and has absolutely nothing to do with PR26169.

The vectorizer is twiddling things such that the set of virtual operands changes
for the statement in question.  ie, if you look at the statement before and
after the vectorizer runs it has the form:
#   TMT.32D.2670_172 = V_MAY_DEF <TMT.32D.2670_17>;
D.2524_41->a0D.2385 = D.2477.a0D.2385;

However, if you were to call update_stmt on the statement immediately after
the vectorizer is complete, then dumped the statement again, you'd have:
#   TMT.32D.2670_172 = V_MAY_DEF <TMT.32D.2670_17>;
#   VUSE <SFT.11D.2649>;
#   VUSE <SFT.12D.2650>;
D.2524_41->a0D.2385 = D.2477.a0D.2385;

That's a clear indication that this bug is actually in the vectorizer.  Not
VRP or the SSA updating code.

When a pass twiddles aliasing information such that the set of virtual operands
would change on a statement that is a bug if the statement is not properly
updated.

Jeff
Comment 5 Zdenek Dvorak 2006-02-12 19:24:03 UTC
Probably related to

http://gcc.gnu.org/ml/gcc-patches/2006-01/msg00446.html
Comment 6 Dorit Naishlos 2006-02-13 16:23:16 UTC
(In reply to comment #5)
> Probably related to
> http://gcc.gnu.org/ml/gcc-patches/2006-01/msg00446.html

Would you expect then that calling mark_new_vars_to_rename, like you did in your patch, will fix this problem?

I wasn't able to reproduce this error on powerpc-linux and i686-pc-linux-gnu. I do realize that there's a problem with the setting of virtual operands in the vectorizer. The over conservativeness in the vectorizer with respect to setting aliasing information for vector pointers when accessing struct fields may be responssible for this. I will try to look into this issue. In the meantime, could someone that can reproduce this problem try out the mark_new_vars_to_rename patch that Zdenek suggested in the link?

Comment 7 Zdenek Dvorak 2006-02-16 12:52:50 UTC
No, that patch does not fix the problem.  It is however the same type of problem -- vectorizer not updating virtual operands properly (I had tried to find all places in vectorizer where this could happen in that patch, but I obviously missed some).
Comment 8 Zdenek Dvorak 2006-02-16 17:53:48 UTC
This seems somewhat more complicated than I thought -- vectorizer actually does not touch the statement with changed vops, it just clobbers alias info somehow so that we are adding vops for it we did not before. I will have a look why this happens.
Comment 9 Zdenek Dvorak 2006-02-17 19:54:08 UTC
SFT for D.2477.a is SFT.12. Before vectorizer is run, SFT.12 has is_alias_tag == false and may_aliases = {TMT.32}; thus, we would only add a vuse for TMT.32 to the statement, and this is subsumed by the VDEF.

Vectorizer calls new_type_alias for D.2477.  This causes SFT.12 to be added to the alias set of this new tag, and SFT.12->is_alias_tag is set to true (by add_may_alias).  This however means that since that moment on, add_virtual_operand will be also adding vuses for SFT.12; i.e., although we haven't touched the statement at all, its virtual operands will change.

I haven't any idea how this might be fixed (the more I learn about the way alias info translates to virtual operands, the less I understand it :-( ). I am CCing someone who might know.
Comment 10 Dorit Naishlos 2006-02-19 16:10:02 UTC
so maybe if an SFT has may-aliases then new_type_alias should add the may-aliases of the SFT as may-aliases of the new tag, instead of adding the SFT as a may-alias of the new tag. ?

There's a comment in new_type_alias that's quite worrying:
"/* The following is based on code in add_stmt_operand to ensure that the
         same defs/uses/vdefs/vuses will be found...."
So this code depends on code in add_stmt_operand, that may have changed by now... 

Related or not, there's another bug in new_type_alias in PR26359.
Comment 11 Dorit Naishlos 2006-02-28 08:26:06 UTC
Created attachment 10935 [details]
tentative patch

I get a similar error message when trying to bootstrap mainline with vectorization enabled:

/home/dorit/mainline_svn/build2/./prev-gcc/xgcc -B/home/dorit/mainline_svn/build2/./prev-gcc/ -B/home/dorit/mainline_svn2/ppc64-yellowdog-linux/bin/ -c   -g -O2 -ftree-vectorize -maltivec -DIN_GCC   -W -Wall -Wwrite-strings -Wstrict-prototypes -Wmissing-prototypes -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -Wold-style-definition -Wmissing-format-attribute -Werror -fno-common   -DHAVE_CONFIG_H -I. -I. -I../../gcc/gcc -I../../gcc/gcc/. -I../../gcc/gcc/../include -I../../gcc/gcc/../libcpp/include  -I../../gcc/gcc/../libdecnumber -I../libdecnumber    ../../gcc/gcc/recog.c -o recog.o
../../gcc/gcc/recog.c: In function âconstrain_operandsâ:
../../gcc/gcc/recog.c:2270: internal compiler error: tree check: expected ssa_name, have struct_field_tag in verify_ssa, at tree-ssa.c:735
make[3]: *** [recog.o] Error 1
make[3]: Leaving directory `/home/dorit/mainline_svn/build2/gcc'
make[2]: *** [all-stage2-gcc] Error 2
make[2]: Leaving directory `/home/dorit/mainline_svn/build2'
make[1]: *** [stage2-bubble] Error 2
make[1]: Leaving directory `/home/dorit/mainline_svn/build2'
make: *** [bootstrap] Error 2

Following Zdenek's observations, I tried the attached patch. It solves this failure above in recog.c, but it fails bootstrap with vectorization enabled later on. (It does pass regular bootstrap on ppc-linux). So this patch needs to be further examined, but I wonder if it fixes this PR (I can't reproduce it)?

About the patch: 

new_type_alias() originally looked like this:

TAG <- new tag for ptr;
if (var has subvars){
   foreach subvar
	add the subvar as may-alias of TAG.
}
else{
   get the may-aliases of var;
   if (|may-aliases| == 1)
	set the (single) may-alias of var as the new tag of ptr;
   else if (|may-aliases| == 0)
	add var as may-alias of the TAG;
   else /* |may-aliases| > 1 */
	add the may-aliases of var as may-aliases of TAG;
}

What I did is basically factored out the 'else' part into a separate function, and called that function also in the 'if' part, for each subvar; this way, we don't add the subvar as may-alias of TAG if the subvar itself has may-aliases, but add its may-aliases instead:

new version of new_type_alias():

TAG <- new tag for ptr;
if (var has subvars){
   foreach subvar
     add_may_aliases_for_new_tag (TAG, subvar)
}
else{
   add_may_aliases_for_new_tag (TAG, var)
}

add_may_aliases_for_new_tag (TAG, var)
{
   get the may-aliases of var;
   if (|may-aliases| == 1)
	set the (single) may-alias of var as the new tag of ptr;
   else if (|may-aliases| == 0)
	add var as may-alias of the TAG;
   else /* |may-aliases| > 1 */
	add the may-aliases of var as may-aliases of TAG;
}

Makes sense to anyone?
Comment 12 Diego Novillo 2006-02-28 14:38:23 UTC
(In reply to comment #11)

> Makes sense to anyone?
> 
Unfortunately yes.  I am hoping that we will soon be able to get rid of all this nonsense.  The vectorizer is running against a very recalcitrant alias pass.  In particular, the alias grouping heuristic is getting in your way, turning around the roles of memory tag and aliased symbols.

The approach you outline is fine.
Comment 13 Dorit Naishlos 2006-03-01 12:35:33 UTC
So I'll submit the patch to gcc-patches for approval. Can someone please check if this patch actually solves this PR?
Comment 14 Andrew Pinski 2006-03-02 04:44:03 UTC
here is an ever more reduced testcase:
void g(const void*);
struct B
{
  int* x[2];
  int *p;
  B()
  {
     for (int** p=x; p<x+2; ++p)
      *p = 0;
  }
  ~B()
   {
      g(p);
   }
};
void bar()
{
  const B &b = B();
  g(&b);
}
--------
Compile with "-O2 -ftree-vectorize --param max-aliased-vops=0 "
--param max-aliased-vops=0 is needed so that aliaing grouping is done always.
Comment 15 Andrew Pinski 2006-03-03 18:00:26 UTC
*** Bug 26518 has been marked as a duplicate of this bug. ***
Comment 16 Andrew Pinski 2006-04-05 18:38:00 UTC
*** Bug 26948 has been marked as a duplicate of this bug. ***
Comment 17 Mark Mitchell 2006-06-04 17:40:26 UTC
Has anyone tested Dorit's patch?  (IIUC, Dorit is on maternity leave.)
Comment 18 patchapp@dberlin.org 2006-07-24 19:50:18 UTC
Subject: Bug number PR26197

A patch for this bug has been added to the patch tracker.
The mailing list url for the patch is http://gcc.gnu.org/ml/gcc-patches/2006-07/msg01043.html
Comment 19 dorit 2006-08-10 12:07:31 UTC
Subject: Bug 26197

Author: dorit
Date: Thu Aug 10 12:07:22 2006
New Revision: 116060

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=116060
Log:
        PR tree-optimization/26197
        * tree-ssa-alias.c (new_type_alias): Takes additional argument. Calls
        get_ref_base_and_extent and overlap_subvar to add only relevant
        subvars as may-aliases.
        (add_may_alias_for_new_tag): New function, factored out of
        new_type_alias.
        * tree-vect-transform.c (vect_create_data_ref_ptr): Call new_type_alias
        with additional argument.
        * tree-flow.h (new_type_alias): Takes additional argument.


Added:
    trunk/gcc/testsuite/g++.dg/vect/param-max-aliased-pr26197.cc
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/g++.dg/vect/vect.exp
    trunk/gcc/tree-flow.h
    trunk/gcc/tree-ssa-alias.c
    trunk/gcc/tree-vect-transform.c

Comment 20 Volker Reichelt 2006-08-11 14:53:15 UTC
Fixed on mainline.
Comment 21 Andrew Pinski 2006-08-29 17:04:00 UTC
*** Bug 27474 has been marked as a duplicate of this bug. ***