[Bug rtl-optimization/54342] New: [4.8 Regression] Wrong mode of call argument

[Bug rtl-optimization/54342] New: [4.8 Regression] Wrong mode of call argument

[vbyakovl23 at gmail dot com]

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

             Bug #: 54342
           Summary: [4.8 Regression] Wrong mode of call argument
    Classification: Unclassified
           Product: gcc
           Version: 4.8.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: rtl-optimization
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: vbyakovl23@gmail.com


The argument of call has mode OI rather than V8SF. Test case

-------------------------------------------------
#include <immintrin.h>

typedef union un1
{
    __m256 x;
    float f;
} UN1;
UN1 u;
extern __m256 y;
extern int bar2(UN1);

int foo2 ()
{
    u.x = y;
    return bar2(u);
}
----------------------------------------------

Dump after expand

(note 4 2 5 3 [bb 3] NOTE_INSN_BASIC_BLOCK) 

(insn 5 4 6 3 (set (reg/f:DI 62) 
        (symbol_ref:DI ("u")  <var_decl 0x7f312f270a00 u>)) er.c:13 -1 
     (nil)) 

(insn 6 5 7 3 (set (reg:V8SF 63) 
        (mem/c:V8SF (symbol_ref:DI ("y") [flags 0x40]  <var_decl 0x7f312f270aa0
y>) [2 y+0 S32 A256])) er.c:13 -1 
     (nil)) 

(insn 7 6 8 3 (set (mem/c:V8SF (reg/f:DI 62) [0 u.x+0 S32 A256]) 
        (reg:V8SF 63)) er.c:13 -1 
     (nil)) 

(insn 8 7 9 3 (set (reg/f:DI 65) 
        (symbol_ref:DI ("u")  <var_decl 0x7f312f270a00 u>)) er.c:14 -1 
     (nil)) 

(insn 9 8 10 3 (set (reg:OI 64) 
        (mem/c:OI (reg/f:DI 65) [3 u+0 S32 A256])) er.c:14 -1 
     (nil)) 

(insn 10 9 11 3 (set (reg:OI 21 xmm0) 
        (reg:OI 64)) er.c:14 -1 
     (nil)) 

(call_insn/j 11 10 12 3 (parallel [ 
            (set (reg:SI 0 ax) 
                (call (mem:QI (symbol_ref:DI ("bar2") [flags 0x41] 
<function_decl 0x7f312f27b300 bar2>) [0 bar2 S1 A8]) 
                    (const_int 0 [0]))) 
            (unspec [ 
                    (const_int 1 [0x1]) 
                ] UNSPEC_CALL_NEEDS_VZEROUPPER) 
        ]) er.c:14 -1 
     (nil) 
    (expr_list:REG_DEP_TRUE (use (reg:OI 21 xmm0)) 
        (nil)))

Following patch fixes that.

diff --git a/gcc/stor-layout.c b/gcc/stor-layout.c
index 53554a9..bb39e7f 100644
--- a/gcc/stor-layout.c
+++ b/gcc/stor-layout.c
@@ -1639,7 +1639,8 @@ compute_record_mode (tree type)
   /* If we only have one real field; use its mode if that mode's size
      matches the type's size.  This only applies to RECORD_TYPE.  This
      does not apply to unions.  */
-  if (TREE_CODE (type) == RECORD_TYPE && mode != VOIDmode
+  if ((TREE_CODE (type) == RECORD_TYPE || TREE_CODE (type) == UNION_TYPE)
+      && mode != VOIDmode
       && host_integerp (TYPE_SIZE (type), 1)
       && GET_MODE_BITSIZE (mode) == TREE_INT_CST_LOW (TYPE_SIZE (type)))
     SET_TYPE_MODE (type, mode);

[Bug rtl-optimization/54342] [4.8 Regression] Wrong mode of call argument

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.8.0

--- Comment #1 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-08-21 11:08:27 UTC ---
> diff --git a/gcc/stor-layout.c b/gcc/stor-layout.c
> index 53554a9..bb39e7f 100644
> --- a/gcc/stor-layout.c
> +++ b/gcc/stor-layout.c
> @@ -1639,7 +1639,8 @@ compute_record_mode (tree type)
>    /* If we only have one real field; use its mode if that mode's size
>       matches the type's size.  This only applies to RECORD_TYPE.  This
>       does not apply to unions.  */
> -  if (TREE_CODE (type) == RECORD_TYPE && mode != VOIDmode
> +  if ((TREE_CODE (type) == RECORD_TYPE || TREE_CODE (type) == UNION_TYPE)
> +      && mode != VOIDmode
>        && host_integerp (TYPE_SIZE (type), 1)
>        && GET_MODE_BITSIZE (mode) == TREE_INT_CST_LOW (TYPE_SIZE (type)))
>      SET_TYPE_MODE (type, mode);

Reading the comment immediately makes the patch suspicious.

[Bug rtl-optimization/54342] [4.8 Regression] Wrong mode of call argument

[rguenth at gcc dot gnu.org]

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

--- Comment #2 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-08-21 11:10:13 UTC ---
Btw, please elaborate on why you consider this a bug.

[Bug rtl-optimization/54342] [4.8 Regression] Wrong mode of call argument

[vbyakovl23 at gmail dot com]

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

--- Comment #3 from Vladimir Yakovlev <vbyakovl23 at gmail dot com> 2012-08-21 11:18:39 UTC ---
I'm working on vzeroupper insertion and my implementation inserts vzeroupper
before the call because VALID_AVX256_REG_MODE returns false.

[Bug rtl-optimization/54342] [4.8 Regression] Wrong mode of call argument

[ubizjak at gmail dot com]

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

Uros Bizjak <ubizjak at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hjl.tools at gmail dot com

--- Comment #4 from Uros Bizjak <ubizjak at gmail dot com> 2012-08-22 22:01:52 UTC ---
I believe that OImode is currently handled inconsistently in the compiler, and
should be handled exactly in the way TImode is handled for xmm registers.

There are some examples:

-function_pass_avx256_p: OImode is handled together with VALID_AVX256_REG_MODE
in parallels, but not in registers.
-construct_container: OImode is created for container member
-ix86_hard_regno_mode_ok: OImode is handled together with
VALID_AVX256_REG_MODE.

I think that OImode should be a member of VALID_AVX256_REG_MODE.

[Bug rtl-optimization/54342] [4.8 Regression] Wrong mode of call argument

[hjl.tools at gmail dot com]

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

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |WAITING
   Last reconfirmed|                            |2012-08-22
     Ever Confirmed|0                           |1

--- Comment #5 from H.J. Lu <hjl.tools at gmail dot com> 2012-08-22 22:08:54 UTC ---
Is an ABI issue? Why isn't shown up in gcc.target/x86_64/abi/avx
tests?

[Bug rtl-optimization/54342] [4.8 Regression] Wrong mode of call argument

[hjl.tools at gmail dot com]

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

--- Comment #6 from H.J. Lu <hjl.tools at gmail dot com> 2012-08-22 22:10:51 UTC ---
This may be related to PR 54315.

[Bug rtl-optimization/54342] [4.8 Regression] Wrong mode of call argument

[ubizjak at gmail dot com]

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

--- Comment #7 from Uros Bizjak <ubizjak at gmail dot com> 2012-08-22 22:26:22 UTC ---
Please note that there is currently no way to generate OImode directly:

$ more oi.c
typedef int OItype __attribute__ ((mode (OI)));

OItype test (OItype a)
{
  return a;
}
$ gcc -O2 -mavx oi.c
oi.c:1:1: error: unable to emulate ‘OI’

[Bug rtl-optimization/54342] [4.8 Regression] Wrong mode of call argument

[hjl.tools at gmail dot com]

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

--- Comment #8 from H.J. Lu <hjl.tools at gmail dot com> 2012-08-23 04:04:36 UTC ---
I don't think AVX supports true 256-bit integer. On the other hand, I was
also puzzled by compute_record_mode, which excludes UNION_TYPE and
QUAL_UNION_TYPE.  Will including them break union?

[Bug rtl-optimization/54342] [4.8 Regression] Wrong mode of call argument

[hjl.tools at gmail dot com]

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

--- Comment #9 from H.J. Lu <hjl.tools at gmail dot com> 2012-08-23 13:58:15 UTC ---
(In reply to comment #4)
> I believe that OImode is currently handled inconsistently in the compiler, and
> should be handled exactly in the way TImode is handled for xmm registers.
> 
> There are some examples:
> 
> -function_pass_avx256_p: OImode is handled together with VALID_AVX256_REG_MODE
> in parallels, but not in registers.
> -construct_container: OImode is created for container member
> -ix86_hard_regno_mode_ok: OImode is handled together with
> VALID_AVX256_REG_MODE.

It is done on purpose.  GCC needs an integer mode which
is as wide as vector size for internal usage.

> I think that OImode should be a member of VALID_AVX256_REG_MODE.

If we want to do that, we need first to update psABI to specify how
to pass and return OImode.

[Bug rtl-optimization/54342] [4.8 Regression] Wrong mode of call argument

[hjl.tools at gmail dot com]

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

--- Comment #10 from H.J. Lu <hjl.tools at gmail dot com> 2012-08-23 14:26:53 UTC ---
There are

     /* We implement the move patterns for all vector modes into and
         out of SSE registers, even when no operation instructions
         are available.  OImode move is available only when AVX is
         enabled.  */
      return ((TARGET_AVX && mode == OImode)
              || VALID_AVX256_REG_MODE (mode)

to deal with OImode and AVX.  Please find out what you need to
do to support vzeroupper insertion.

[Bug rtl-optimization/54342] [4.8 Regression] Wrong mode of call argument

[hjl.tools at gmail dot com]

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

H.J. Lu <hjl.tools at gmail dot com> changed:

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

--- Comment #11 from H.J. Lu <hjl.tools at gmail dot com> 2012-08-23 17:16:21 UTC ---
(In reply to comment #8)
> I don't think AVX supports true 256-bit integer. On the other hand, I was
> also puzzled by compute_record_mode, which excludes UNION_TYPE and
> QUAL_UNION_TYPE.  Will including them break union?

It breaks gcc.c-torture/compile/pr42196-2.c:

union U
{
  __complex__ int ci;
  __complex__ float cf;
};

float gd;
extern float bar (union U);

float foo (int b, double f1, double f2, int c1, int c2)
{
  union U u;
  double r;

  if (b)
    {
      __real__ u.cf = f1;
      __imag__ u.cf = f2;
    }
  else
    {
      __real__ u.ci = c1;
      __imag__ u.ci = c2;
    }

  r = bar (u);
  return r;
}

The proper fix is to allow OImode in your case without adding it to
VALID_AVX256_REG_MODE.

[Bug rtl-optimization/54342] [4.8 Regression] Wrong mode of call argument

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-11-09 12:19:52 UTC ---
Why is this marked [4.8 Regression]?  From what I can see, gcc has been using
OImode in this testcase since it was changed to pass 32-byte vectors in ymm
registers (somewhen during 4.4 development, and 4.3 didn't have avx support at
all).  Thus this doesn't look any kind of regression to me.  We risk ABI
regressions if we change this though.

[Bug target/54342] OImode is used for _m256 types when using unions in a function call.

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|rtl-optimization            |target
   Target Milestone|4.8.0                       |---
            Summary|[4.8 Regression] Wrong mode |OImode is used for _m256
                   |of call argument            |types when using unions in
                   |                            |a function call.

--- Comment #13 from Andrew Pinski <pinskia at gcc dot gnu.org> 2012-11-16 22:58:46 UTC ---
>(insn 10 9 11 3 (set (reg:OI 21 xmm0) 
        (reg:OI 64)) er.c:14 -1 
     (nil)) 


This above is generated by the back-end's call by construct_container.  So this
is not a middle-end issue even if the middle-end says the mode on the union is
BLKmode.  It is up to the back-end to do the correct thing when it is doing a
function call and converting the BLKmode of the union to the correct mode.

              case 4:
                gcc_assert (i == 0
                            && regclass[1] == X86_64_SSEUP_CLASS
                            && regclass[2] == X86_64_SSEUP_CLASS
                            && regclass[3] == X86_64_SSEUP_CLASS);
                tmpmode = OImode;

Is where the OImode is being generated.

The comment in ix86_hard_regno_mode_ok says:
      /* We implement the move patterns for all vector modes into and
         out of SSE registers, even when no operation instructions
         are available.  OImode move is available only when AVX is
         enabled.  */

So really I don't think this is a bug in the code itself but rather the patch
which you are working on which did not take into account OImode being an AVX
mode.

[Bug target/54342] OImode is used for _m256 types when using unions in a function call.

[vbyakovl23 at gmail dot com]

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

Vladimir Yakovlev <vbyakovl23 at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED

--- Comment #14 from Vladimir Yakovlev <vbyakovl23 at gmail dot com> 2012-11-22 07:06:12 UTC ---
The vzeroupper implementation is in trunk now. To fix the problem I made use of
the proposes if HJ. So the issue can be closed.