[Bug middle-end/57955] New: Uniquization of constants breaks constant alignment

[Bug middle-end/57955] New: Uniquization of constants breaks constant alignment

[dje at gcc dot gnu.org]

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

            Bug ID: 57955
           Summary: Uniquization of constants breaks constant alignment
           Product: gcc
           Version: 4.6.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: dje at gcc dot gnu.org

A patch to improve uniquization of constants at the tree level broke alignment
of constants and vectorized copies.

gcc.target/powerpc/ppc-vector-memcpy.c

[Bug middle-end/57955] Uniquization of constants breaks constant alignment

[dje at gcc dot gnu.org]

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

David Edelsohn <dje at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |missed-optimization
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2013-07-22
                 CC|                            |bergner at gcc dot gnu.org,
                   |                            |ebotcazou at gcc dot gnu.org,
                   |                            |meissner at gcc dot gnu.org,
                   |                            |pthaugen at gcc dot gnu.org
      Known to work|                            |4.3.0, 4.4.0, 4.5.0
     Ever confirmed|0                           |1
      Known to fail|                            |4.6.0, 4.7.0, 4.8.0

--- Comment #1 from David Edelsohn <dje at gcc dot gnu.org> ---
Confirmed.

[Bug middle-end/57955] Uniquization of constants breaks constant alignment

[ebotcazou at gcc dot gnu.org]

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

--- Comment #2 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
The subject is very misleading, the alignment of constants is _not_ changed at
all, otherwise many things would have been broken.  The only change pertains to
the internal alignment of initializers and cannot break anything, as it at most
trades questionable optimization patterns for a smaller data footprint.

[Bug middle-end/57955] Uniquization of constants breaks constant alignment

[dje at gcc dot gnu.org]

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

--- Comment #3 from David Edelsohn <dje at gcc dot gnu.org> ---
The change caused a testcase failure, which was not investigated. I don't
understand the comment "questionable optimization patterns". Optimizing for
size should be controlled by -Os.

[Bug middle-end/57955] Uniquization of constants breaks constant alignment

[ebotcazou at gcc dot gnu.org]

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

--- Comment #4 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> I don't understand the comment "questionable optimization patterns".

I don't see much value in optimizing a memcpy to initialize a variable, it's
unlikely to be in a hot spot.  I'd suggest adding 'const' to the testcase.

> Optimizing for size should be controlled by -Os.

It's just classical constant pool technique though and constant pools are not
conditionalized on -Os in the compiler.

[Bug middle-end/57955] [4.6/4.7/4.8/4.9 Regression] Uniquization of constants reduces alignment of initializers

[dje at gcc dot gnu.org]

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

David Edelsohn <dje at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|Uniquization of constants   |[4.6/4.7/4.8/4.9
                   |breaks constant alignment   |Regression] Uniquization of
                   |                            |constants reduces alignment
                   |                            |of initializers

--- Comment #5 from David Edelsohn <dje at gcc dot gnu.org> ---
It was an undocumented change in GCC behavior.

I infer from the name that the testcase was trying to test that a block move of
appropriate alignment used VMX instructions. I will see if I can construct a
testcase with memcpy directly.

[Bug middle-end/57955] [4.7/4.8/4.9 Regression] Uniquization of constants reduces alignment of initializers

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.7.4
            Summary|[4.6/4.7/4.8/4.9            |[4.7/4.8/4.9 Regression]
                   |Regression] Uniquization of |Uniquization of constants
                   |constants reduces alignment |reduces alignment of
                   |of initializers             |initializers

[Bug middle-end/57955] [4.7/4.8/4.9 Regression] Uniquization of constants reduces alignment of initializers

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |WAITING

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
Can you point to a revision that broke this?  Can you verify if adding
-fno-common fixes it?

With a simple cross I do see lvx instructions both with -m32 and -m64.

[Bug middle-end/57955] [4.7/4.8/4.9/4.10 Regression] Uniquization of constants reduces alignment of initializers

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57955

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.7.4                       |4.8.4

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
The 4.7 branch is being closed, moving target milestone to 4.8.4.

[Bug middle-end/57955] [4.8/4.9/5 Regression] Uniquization of constants reduces alignment of initializers

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57955

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.8.4                       |4.8.5

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 4.8.4 has been released.

[Bug middle-end/57955] [4.8/4.9/5 Regression] Uniquization of constants reduces alignment of initializers

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57955

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I'll note that (not obvious from the comments) this PR talks about the
r158838 change affecting the r201264 revision of the ppc-vector-memcpy.c test,
r201265 changed the test again, so that it doesn't fail.

[Bug middle-end/57955] [4.8/4.9/5 Regression] Uniquization of constants reduces alignment of initializers

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57955

--- Comment #10 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
So, it seems the .LC0 VAR_DECL that is newly created undergoes
DATA_ABI_ALIGNMENT, DATA_ALIGNMENT and CONSTANT_ALIGNMENT treatment, but the
rs6000 backend doesn't consider it worthwhile to bump the alignment.
Old gimplify used to bump the alignment for the ctors to the alignment of the
var being initialized:
                new_tree = create_tmp_var_raw (type, "C");

                gimple_add_tmp_var (new_tree);
                TREE_STATIC (new_tree) = 1;
                TREE_READONLY (new_tree) = 1;
                DECL_INITIAL (new_tree) = ctor;
                if (align > DECL_ALIGN (new_tree))
                  {
                    DECL_ALIGN (new_tree) = align;
                    DECL_USER_ALIGN (new_tree) = 1;
                  }
which is definitely very excessive (128-byte alignment is bigger than
MAX_ALIGNMENT and hardly useful here).
I'd say if you think this is worth doing anything at all, then the best choice
might be to increase CONSTANT_ALIGNMENT for initializers larger than say 128
bits to 128 bits if Altivec or VSX is enabled - in current GCCs
CONSTANT_ALIGNMENT should be assumed only for decls that bind to the current
def, so the macro should be a pure optimization thing rather than an ABI thing,
only tweaking CONSTANT_ALIGNMENT would actually affect code that doesn't use
explicit align attribute on the vars.
The other possibility is to pass an extra align argument to
tree_output_constant_def and build_constant_def, that, if non-0, could bump
alignment of the created decl to what the caller would prefer to see.
We certainly should max that to BIGGEST_ALIGNMENT though, and in any case
wouldn't guarantee it, because the constant might have been already found in
the constant pool and at that point it has rtl created and it is too late for
bumping alignment.

[Bug middle-end/57955] [4.8/4.9/5/6 Regression] Uniquization of constants reduces alignment of initializers

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57955

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.8.5                       |4.9.3

--- Comment #11 from Richard Biener <rguenth at gcc dot gnu.org> ---
The gcc-4_8-branch is being closed, re-targeting regressions to 4.9.3.

[Bug middle-end/57955] [4.9/5/6 Regression] Uniquization of constants reduces alignment of initializers

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57955

--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 4.9.3 has been released.

[Bug middle-end/57955] [4.9/5/6 Regression] Uniquization of constants reduces alignment of initializers

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57955

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.9.3                       |4.9.4

[Bug middle-end/57955] [9/10/11/12 Regression] Uniquization of constants reduces alignment of initializers

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57955

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|8.5                         |9.4

--- Comment #22 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 8 branch is being closed.

[Bug middle-end/57955] [9/10/11/12 Regression] Uniquization of constants reduces alignment of initializers

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57955

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|9.4                         |9.5

--- Comment #23 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 9.4 is being released, retargeting bugs to GCC 9.5.

[Bug middle-end/57955] [9/10/11/12 Regression] Uniquization of constants reduces alignment of initializers

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57955

--- Comment #24 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Just to summarize this bug as far as I read it, please correct me if I am
wrong; note I am not proposing a change, just trying to summarize the back and
forth since it is not obvious right away of what the problem was.

This was the testcase:
 void foo(void)
 {
  int x[8] __attribute__((aligned(128))) = { 1, 1, 1, 1, 1, 1, 1, 1 };
  bar (x);
 }

Before the gimplification change the initializer {1,....} was promoted to a
static const and given an alignment of 128; due to this part of the code:

                if (align > DECL_ALIGN (new_tree))
                  {
                    DECL_ALIGN (new_tree) = align;
                    DECL_USER_ALIGN (new_tree) = 1;
                  }

But now it just uses DATA_ALIGNMENT (the code should be using
TARGET_CONSTANT_ALIGNMENT but does not right now, that was a proposal).

rs6000_constant_alignment (TARGET_CONSTANT_ALIGNMENT) only aligns strings csts
to word (32 or 64) aligned. rs6000_data_alignment (DATA_ALIGNMENT) only aligns
vectors to 128 and char arrays to word (32 or 64) align.

[Bug middle-end/57955] [9/10/11/12 Regression] Uniquization of constants reduces alignment of initializers

[ebotcazou at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57955

--- Comment #25 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> Before the gimplification change the initializer {1,....} was promoted to a
> static const and given an alignment of 128; due to this part of the code:
> 
>                 if (align > DECL_ALIGN (new_tree))
>                   {
>                     DECL_ALIGN (new_tree) = align;
>                     DECL_USER_ALIGN (new_tree) = 1;
>                   }
> 
> But now it just uses DATA_ALIGNMENT (the code should be using
> TARGET_CONSTANT_ALIGNMENT but does not right now, that was a proposal).

Yes, exactly.  The initializer is put into the "tree" constant pool, where its
alignment is uniformly set to DATA_ALIGNMENT.

[Bug middle-end/57955] [9/10/11/12 Regression] Uniquization of constants reduces alignment of initializers

[dje at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57955

--- Comment #26 from David Edelsohn <dje at gcc dot gnu.org> ---
As Bill mentioned, one can increase the alignment of a large constant, but
there is no way for the hooks that set alignment to recognize that the constant
will be assigned to variable with stricter alignment.

[Bug middle-end/57955] [10/11/12/13 Regression] Uniquization of constants reduces alignment of initializers

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57955

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|9.5                         |10.4

--- Comment #27 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 9 branch is being closed

[Bug middle-end/57955] [10/11/12/13 Regression] Uniquization of constants reduces alignment of initializers

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57955

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|10.4                        |10.5

--- Comment #28 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 10.4 is being released, retargeting bugs to GCC 10.5.

[Bug middle-end/57955] [11/12/13/14 Regression] Uniquization of constants reduces alignment of initializers

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57955

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|10.5                        |11.5

--- Comment #29 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 10 branch is being closed.