[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[gary at intrepid dot com]

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

--- Comment #9 from Gary Funck <gary at intrepid dot com> 2011-07-08 16:20:50 UTC ---


This note is both a ping for this rather old bug report, as well as a follow up
with some additional information.

For the ping side of things, we have been using this patch for the past 5/so
years in GCC/UPC without problems (in both the build 'gcc' and 'upc'
compilers).

+++ ./gcc/config/i386/i386.h    Wed Jul  6 10:11:16 2011
@@ -1755,6 +1755,10 @@
 #define BRANCH_COST(speed_p, predictable_p) \
   (!(speed_p) ? 2 : (predictable_p) ? 0 : ix86_branch_cost)

+/* An integer expression for the size in bits of the largest integer machine
+   mode that should actually be used.  We allow pairs of registers.  */
+#define MAX_FIXED_MODE_SIZE GET_MODE_BITSIZE (TARGET_64BIT ? TImode : DImode)
+
 /* Define this macro as a C expression which is nonzero if accessing
    less than a word of memory (i.e. a `char' or a `short') is no
    faster than accessing a word of memory, i.e., if such access

Although I haven't run any benchmarks, I think that this patch will improve
performance for some applications (that make use of 128 bit structures), and
recommend that it be applied to the GCC trunk.

Recently, however, we ran the torture tests against the 'gcc' built from the
GUPC branch with those built from the trunk (checks enabled and bootstrap
disabled, CFLAGS='-O0 -g').  We found a regression.  This test produced an ICE:

FAIL: tmpdir-gcc.dg-struct-layout-1/t001 c_compat_x_tst.o compile, (internal
compiler error)
FAIL: tmpdir-gcc.dg-struct-layout-1/t001 c_compat_y_tst.o compile, (internal
compiler error)

This can be reproduced with the following command, run in the built 'gcc'
directory:
make check-gcc RUNTESTFLAGS="struct-layout-1.exp=*t001*"

The following is a boiled down test case:

union S160
{
  long double a;
};
union S160 s160;
extern union S160 a160[5];
extern union S160 check160 (union S160, union S160 *, union S160);
extern void checkx160 (union S160);
void
test160 (void)
{
  checkx160 (check160 (s160, &a160[1], a160[2]));
}

The assertion check occurred at line 3097 below:

 0x00000000006390fe in expand_call (exp=0x7ffff14a51e0, target=0x7ffff107b980,
ignore=0) at gcc/calls.c:3097
3097              gcc_assert (GET_MODE (target) == pmode);
(gdb) p pmode
$1 = TImode
(gdb) l
3092              enum machine_mode pmode;
3093
3094              /* Ensure we promote as expected, and get the new
unsignedness.  */
3095              pmode = promote_function_mode (type, TYPE_MODE (type),
&unsignedp,
3096                                             funtype, 1);
3097              gcc_assert (GET_MODE (target) == pmode);
3098
3099              if ((WORDS_BIG_ENDIAN || BYTES_BIG_ENDIAN)
3100                  && (GET_MODE_SIZE (GET_MODE (target))
3101                      > GET_MODE_SIZE (TYPE_MODE (type))))
(gdb) p GET_MODE (target)
$2 = XFmode
(gdb) p expand_location(input_location)->file
$3 = 0x1ab5b40 "gcc/testsuite/gcc/gcc.dg-struct-layout-1/t001_test.h"
(gdb) p expand_location(input_location)->line
$4 = 161

The gist of what happens here is:
1) Because of the change in the definition of MAX_FIXED_MODE_SIZE, the compiler
assigned TImode (rather than BLKmode) as the mode for the union type (it
contains only a single field, a long double, of mode XFmode).
2) The expand_call logic decided that the target mode of the value to be
returned is equal to that of the first (and only) field: XFmode.
3) The assertion (GET_MODE (target) == pmode) fails.

As far as we know, this is the only (x86_64) failure introduced by the
MAX_FIXED_MODE_SIZE patch.  It seems that the logic in expand_call() will need
to be adjusted to handle this test case, if the patch is applied.

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[gary at intrepid dot com]

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

--- Comment #10 from Gary Funck <gary at intrepid dot com> 2011-07-08 16:58:55 UTC ---
Note: I don't know how this fix fits in with the x86_64 ABI, and obviously once
this fix is in place, the binary call interface will change for 128 bit
struct/union arguments.  As noted in the previous reply, expand_call tried
passing the argument (of TImode) in an XFmode (SSE) register.  That may or may
not be permitted by the call ABI.

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[gary at intrepid dot com]

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

--- Comment #11 from Gary Funck <gary at intrepid dot com> 2011-07-08 17:20:32 UTC ---
(In reply to comment #10)
> Note: I don't know how this fix fits in with the x86_64 ABI, and obviously once
> this fix is in place, the binary call interface will change for 128 bit
> struct/union arguments.

Actually, the call ABI can be preserved, and this fix/improvement can still be
made in order to gain some potential benefit in copying struct's and perhaps
accessing their fields.  The main reason we made the fix however, was to insure
that 128 arguments (and function return values) fit into TImode registers on
x86_64 targets.

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[ebotcazou at gcc dot gnu.org]

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

--- Comment #12 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2011-07-09 10:27:42 UTC ---
Since the PR was opened, a new usage of MAX_FIXED_MODE_SIZE has been
introduced:

2006-11-18  John David Anglin  <dave.anglin@nrc-cnrc.gc.ca>

    PR fortran/27885
    PR middle-end/28176
    * stor-layout.c (set_sizetype): Limit precision of *bitsizetypes types
    to MAX_FIXED_MODE_SIZE.

so bumping MAX_FIXED_MODE_SIZE to 128 will also bump the size of these types.
That's very likely OK anyway.  FWIW I'm evaluating the change for SPARC64.

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[chip at pobox dot com]

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

Chip Salzenberg <chip at pobox dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |chip at pobox dot com

--- Comment #13 from Chip Salzenberg <chip at pobox dot com> 2012-08-06 22:52:41 UTC ---
Is this bug obsolete now?

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[gary at intrepid dot com]

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

--- Comment #14 from Gary Funck <gary at intrepid dot com> 2012-08-11 03:22:34 UTC ---
(In reply to comment #13)
> Is this bug obsolete now?

Comment #7 (2005-06-25) states that this is a valid bug, and near as I can tell
the current compiler still does not target 16-byte aligned (or potentially
aligned) struct's into TImode values.  Thus, I don't see any technical reason
mentioned in the subsequent comments that describe why this bug should be
marked  as obsolete.

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[hjl.tools at gmail dot com]

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

--- Comment #15 from H.J. Lu <hjl.tools at gmail dot com> 2012-08-11 14:37:30 UTC ---
Do we have a run-time testcase?

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[gary at intrepid dot com]

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

--- Comment #16 from Gary Funck <gary at intrepid dot com> 2012-08-12 18:08:05 UTC ---
Created attachment 27995
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27995
test case #1 - struct targeted to TImode

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[gary at intrepid dot com]

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

--- Comment #17 from Gary Funck <gary at intrepid dot com> 2012-08-12 18:11:25 UTC ---
Created attachment 27996
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27996
test case #2 - struct targeted to TImode

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[gary at intrepid dot com]

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

--- Comment #18 from Gary Funck <gary at intrepid dot com> 2012-08-12 18:17:19 UTC ---
Created attachment 27997
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=27997
test case #3 - struct targeted to TImode

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[gary at intrepid dot com]

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

--- Comment #19 from Gary Funck <gary at intrepid dot com> 2012-08-12 18:30:25 UTC ---
(In reply to comment #15)
> Do we have a run-time testcase?

I attached three compile-time test cases that check if the generated RTL refers
to TImode values.  The test cases are set up to only run on x86_64 and x86
targets. I confirmed that they work as expected on an x86_64 target (with and
without the MAX_FIXED_MODE_SIZE patch).  Not sure if it will work on x86 or
not.  The test case may have to be tweaked for x86.

Is this sufficient?  I don't know how a run-time test would demonstrate the use
of TImode, except in the form of a benchmark.

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[hjl.tools at gmail dot com]

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

--- Comment #20 from H.J. Lu <hjl.tools at gmail dot com> 2012-08-12 19:50:08 UTC ---
(In reply to comment #19)
> (In reply to comment #15)
> > Do we have a run-time testcase?
> 
> I attached three compile-time test cases that check if the generated RTL refers
> to TImode values.  The test cases are set up to only run on x86_64 and x86
> targets. I confirmed that they work as expected on an x86_64 target (with and
> without the MAX_FIXED_MODE_SIZE patch).  Not sure if it will work on x86 or
> not.  The test case may have to be tweaked for x86.

X86 doesn't support __int128 and requires SSE for TImode.
We may need to limit those testcases for int128 target.

> Is this sufficient?  I don't know how a run-time test would demonstrate the use
> of TImode, except in the form of a benchmark.

If it is an ABI, we can add a testcase to gcc.target/x86_64/abi
for x86-64.  If there is no additional issues for x86-64, I think
we can close it after checking in those testcases.

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[gary at intrepid dot com]

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

--- Comment #21 from Gary Funck <gary at intrepid dot com> 2012-08-12 21:24:40 UTC ---
(In reply to comment #20) 
> X86 doesn't support __int128 and requires SSE for TImode.
> We may need to limit those testcases for int128 target.

OK, I'll add: /* { dg-require-effective-target int128 } */.

> If it is an ABI, we can add a testcase to gcc.target/x86_64/abi
> for x86-64.  If there is no additional issues for x86-64, I think
> we can close it after checking in those testcases.

This issue is performance-related, not ABI-related, per the discussion replies. 

The tests will FAIL on the current compiler because it does not assign TI mode
to 128 bit struct's.  The tests PASS if the suggested MAX_FIXED_MODE_SIZE
definition is added to config/i386/i386.h.

Note: Applying the new MAX_FIXED_MODE_SIZE definition patch will result in at
least one GCC test regression (see comment #9).

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[gary at intrepid dot com]

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

Gary Funck <gary at intrepid dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #27995|0                           |1
        is obsolete|                            |

--- Comment #22 from Gary Funck <gary at intrepid dot com> 2012-08-12 21:45:09 UTC ---
Created attachment 28000
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=28000
test case #1 - struct targeted to TImode

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[gary at intrepid dot com]

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

Gary Funck <gary at intrepid dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #27996|0                           |1
        is obsolete|                            |

--- Comment #23 from Gary Funck <gary at intrepid dot com> 2012-08-12 21:47:47 UTC ---
Created attachment 28001
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=28001
test case #2 - struct targeted to TImode

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[gary at intrepid dot com]

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

Gary Funck <gary at intrepid dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #27997|0                           |1
        is obsolete|                            |

--- Comment #24 from Gary Funck <gary at intrepid dot com> 2012-08-12 21:50:33 UTC ---
Created attachment 28002
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=28002
test case #3 - struct targeted to TImode

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[gary at intrepid dot com]

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

--- Comment #25 from Gary Funck <gary at intrepid dot com> 2012-08-12 22:08:50 UTC ---
(In reply to comment #20)
> X86 doesn't support __int128 and requires SSE for TImode.
> We may need to limit those testcases for int128 target.

If targeting struct's to TImode is supported on x86 targets then perhaps this
definition will need to be refer to 128 bit target support?

#define MAX_FIXED_MODE_SIZE GET_MODE_BITSIZE (TARGET_64BIT ? TImode : DImode)

Off-hand, I don't know the target macro that does this.

something like this?

#define MAX_FIXED_MODE_SIZE targetm.scalar_mode_supported_p (TImode) ? TImode :
DImoe

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[gary at intrepid dot com]

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

--- Comment #26 from Gary Funck <gary at intrepid dot com> 2012-08-12 22:14:56 UTC ---
Typo fixed below:

#define MAX_FIXED_MODE_SIZE targetm.scalar_mode_supported_p (TImode) ? TImode :
DImode

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[hjl.tools at gmail dot com]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ubizjak at gmail dot com

--- Comment #27 from H.J. Lu <hjl.tools at gmail dot com> 2012-08-12 22:30:32 UTC ---
(In reply to comment #25)
> (In reply to comment #20)
> > X86 doesn't support __int128 and requires SSE for TImode.
> > We may need to limit those testcases for int128 target.
> 
> If targeting struct's to TImode is supported on x86 targets then perhaps this
> definition will need to be refer to 128 bit target support?
> 
> #define MAX_FIXED_MODE_SIZE GET_MODE_BITSIZE (TARGET_64BIT ? TImode : DImode)
> 
> Off-hand, I don't know the target macro that does this.

There are:

rs6000/rs6000.h:#define MAX_FIXED_MODE_SIZE GET_MODE_BITSIZE (TARGET_POWERPC64
? TImode : DImode)
s390/s390.h:#define MAX_FIXED_MODE_SIZE GET_MODE_BITSIZE (TARGET_64BIT ? TImode
: DImode)

Please try this patch:

diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h
index c4d85b7..6c4c2ce 100644
--- a/gcc/config/i386/i386.h
+++ b/gcc/config/i386/i386.h
@@ -1853,6 +1853,10 @@ do {                            \

 /* #define SLOW_UNALIGNED_ACCESS(MODE, ALIGN) 0 */

+/* An integer expression for the size in bits of the largest integer machine
+   mode that should actually be used.  We allow pairs of registers.  */
+#define MAX_FIXED_MODE_SIZE GET_MODE_BITSIZE (TARGET_64BIT ? TImode : DImode)
+
 /* Define this macro if it is as good or better to call a constant
    function address than to call an address kept in a register.

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[gary at intrepid dot com]

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

--- Comment #28 from Gary Funck <gary at intrepid dot com> 2012-08-12 22:43:16 UTC ---
(In reply to comment #27)
> Please try this patch:
> 
> diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h
> index c4d85b7..6c4c2ce 100644
> --- a/gcc/config/i386/i386.h
> +++ b/gcc/config/i386/i386.h
> @@ -1853,6 +1853,10 @@ do {                            \
> 
>  /* #define SLOW_UNALIGNED_ACCESS(MODE, ALIGN) 0 */
> 
> +/* An integer expression for the size in bits of the largest integer machine
> +   mode that should actually be used.  We allow pairs of registers.  */
> +#define MAX_FIXED_MODE_SIZE GET_MODE_BITSIZE (TARGET_64BIT ? TImode : DImode)
> +
>  /* Define this macro if it is as good or better to call a constant
>     function address than to call an address kept in a register.

That's the patch that I have been using and testing against (ref: comment #9).

I suggested this:
#define MAX_FIXED_MODE_SIZE targetm.scalar_mode_supported_p (TImode) ? TImode :
DImode
because it might allow x86 targets with SSE to also target TIMode for 128 bit
struct's (on the assumption that TImode will be more efficient).

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[hjl.tools at gmail dot com]

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

--- Comment #29 from H.J. Lu <hjl.tools at gmail dot com> 2012-08-13 02:17:28 UTC ---
(In reply to comment #28)
> (In reply to comment #27)
> > Please try this patch:
> > 
> > diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h
> > index c4d85b7..6c4c2ce 100644
> > --- a/gcc/config/i386/i386.h
> > +++ b/gcc/config/i386/i386.h
> > @@ -1853,6 +1853,10 @@ do {                            \
> > 
> >  /* #define SLOW_UNALIGNED_ACCESS(MODE, ALIGN) 0 */
> > 
> > +/* An integer expression for the size in bits of the largest integer machine
> > +   mode that should actually be used.  We allow pairs of registers.  */
> > +#define MAX_FIXED_MODE_SIZE GET_MODE_BITSIZE (TARGET_64BIT ? TImode : DImode)
> > +
> >  /* Define this macro if it is as good or better to call a constant
> >     function address than to call an address kept in a register.
> 
> That's the patch that I have been using and testing against (ref: comment #9).
> 
> I suggested this:
> #define MAX_FIXED_MODE_SIZE targetm.scalar_mode_supported_p (TImode) ? TImode :
> DImode
> because it might allow x86 targets with SSE to also target TIMode for 128 bit
> struct's (on the assumption that TImode will be more efficient).

ix86_scalar_mode_supported_p (TImode) returns false for -m32
even when SSE is enabled. You can submit a patch with

#define MAX_FIXED_MODE_SIZE GET_MODE_BITSIZE (TARGET_64BIT ? TImode : DImode)

plus some testcases.

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[gary at intrepid dot com]

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

--- Comment #30 from Gary Funck <gary at intrepid dot com> 2012-08-14 04:24:54 UTC ---
Patch posted:
http://gcc.gnu.org/ml/gcc-patches/2012-08/msg00839.html

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[chip at pobox dot com]

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

--- Comment #31 from Chip Salzenberg <chip at pobox dot com> 2012-08-14 22:46:12 UTC ---
I've tested the attached patch, and I find that it succeeds in preventing the
current missed optimizations in structs passed by value from affecting 128-bit
structs.

IOW: Works for me.  Thanks!

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[chip at pobox dot com]

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

--- Comment #32 from Chip Salzenberg <chip at pobox dot com> 2012-08-14 23:09:01 UTC ---
More good data: this patch reduces the size of libstdc++.so by .5%

$ size usr/lib/libstdc++.so.6.0.17 /usr/lib/libstdc++.so.6.0.17
   text    data     bss     dec     hex filename
 949608   36200   85088 1070896  105730 usr/lib/libstdc++.so.6.0.17
 955484   36200   85088 1076772  106e24 /usr/lib/libstdc++.so.6.0.17

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[hjl.tools at gmail dot com]

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

--- Comment #33 from H.J. Lu <hjl.tools at gmail dot com> 2012-08-14 23:43:24 UTC ---
We must make sure that

---
union S160
{
  long double a;
};
extern union S160 check160 (void);
extern void checkx160 (union S160);
void
test160 (void)
{
  checkx160 (check160 ());
}
---

compiles.

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[gary at intrepid dot com]

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

--- Comment #34 from Gary Funck <gary at intrepid dot com> 2012-08-14 23:55:57 UTC ---
(In reply to comment #33)
> We must make sure that
> 
> ---
> union S160
> {
>   long double a;
> };
> extern union S160 check160 (void);
> extern void checkx160 (union S160);
> void
> test160 (void)
> {
>   checkx160 (check160 ());
> }
> ---
> 
> compiles.

I agree.  I looked into this briefly, but was unsure how to fix the issue.

What I can do is incorporate whatever fix is agreed upon into the patch, and
add this as an explicit test case.  Even though it is slightly duplicative, it
will be clearer what's going on if a regression occurs later on.

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[gary at intrepid dot com]

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

--- Comment #35 from Gary Funck <gary at intrepid dot com> 2012-08-15 00:00:43 UTC ---
Note that for the test case in comment #34 (and comment #9) to fail that the
MAX_FIXED_MODE_SIZE patch has to be applied, and likely GCC internal checking
has to be enabled.

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[hjl.tools at gmail dot com]

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

--- Comment #36 from H.J. Lu <hjl.tools at gmail dot com> 2012-08-15 01:23:54 UTC ---
(In reply to comment #35)
> Note that for the test case in comment #34 (and comment #9) to fail that the
> MAX_FIXED_MODE_SIZE patch has to be applied, and likely GCC internal checking
> has to be enabled.

This patch seems to work:

diff --git a/gcc/calls.c b/gcc/calls.c
index 5bc1b1e..8b95baf 100644
--- a/gcc/calls.c
+++ b/gcc/calls.c
@@ -3345,7 +3345,13 @@ expand_call (tree exp, rtx target, int ignore)
       sibcall_failure = 1;
     }
       else
-    target = copy_to_reg (avoid_likely_spilled_reg (valreg));
+    {
+      rtx val = avoid_likely_spilled_reg (valreg);
+      if (target && GET_MODE (val) != GET_MODE (target))
+        target = copy_from_reg (target, val, rettype);
+      else
+        target = copy_to_reg (val);
+    }

       /* If we promoted this return value, make the proper SUBREG.
          TARGET might be const0_rtx here, so be careful.  */
diff --git a/gcc/expr.c b/gcc/expr.c
index 4e7eb5f..8bd57eb 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -2084,32 +2084,20 @@ emit_group_store (rtx orig_dst, rtx src, tree type
ATTRIBUTE_UNUSED, int ssize)
     emit_move_insn (orig_dst, dst);
 }

-/* Generate code to copy a BLKmode object of TYPE out of a
-   set of registers starting with SRCREG into TGTBLK.  If TGTBLK
-   is null, a stack temporary is created.  TGTBLK is returned.
+/* Generate code to copy an object of TYPE out of a set of registers
+   starting with SRCREG into TGTBLK.  TGTBLK is returned.

    The purpose of this routine is to handle functions that return
-   BLKmode structures in registers.  Some machines (the PA for example)
-   want to return all small structures in registers regardless of the
-   structure's alignment.  */
+   structures in registers.  */

 rtx
-copy_blkmode_from_reg (rtx tgtblk, rtx srcreg, tree type)
+copy_from_reg (rtx tgtblk, rtx srcreg, tree type)
 {
   unsigned HOST_WIDE_INT bytes = int_size_in_bytes (type);
   rtx src = NULL, dst = NULL;
   unsigned HOST_WIDE_INT bitsize = MIN (TYPE_ALIGN (type), BITS_PER_WORD);
   unsigned HOST_WIDE_INT bitpos, xbitpos, padding_correction = 0;
-  enum machine_mode copy_mode;
-
-  if (tgtblk == 0)
-    {
-      tgtblk = assign_temp (build_qualified_type (type,
-                          (TYPE_QUALS (type)
-                           | TYPE_QUAL_CONST)),
-                1, 1);
-      preserve_temp_slots (tgtblk);
-    }
+  enum machine_mode copy_mode, mode = TYPE_MODE (type);

   /* This code assumes srcreg is at least a full word.  If it isn't, copy it
      into a new pseudo which is a full word.  */
@@ -2166,7 +2154,7 @@ copy_blkmode_from_reg (rtx tgtblk, rtx srcreg, tree type)
       /* We need a new destination operand each time bitpos is on
      a word boundary.  */
       if (bitpos % BITS_PER_WORD == 0)
-    dst = operand_subword (tgtblk, bitpos / BITS_PER_WORD, 1, BLKmode);
+    dst = operand_subword (tgtblk, bitpos / BITS_PER_WORD, 1, mode);

       /* Use xbitpos for the source extraction (right justified) and
      bitpos for the destination store (left justified).  */
@@ -2179,6 +2167,29 @@ copy_blkmode_from_reg (rtx tgtblk, rtx srcreg, tree
type)
   return tgtblk;
 }

+/* Generate code to copy a BLKmode object of TYPE out of a
+   set of registers starting with SRCREG into TGTBLK.  If TGTBLK
+   is null, a stack temporary is created.  TGTBLK is returned.
+
+   The purpose of this routine is to handle functions that return
+   BLKmode structures in registers.  Some machines (the PA for example)
+   want to return all small structures in registers regardless of the
+   structure's alignment.  */
+
+rtx
+copy_blkmode_from_reg (rtx tgtblk, rtx srcreg, tree type)
+{
+  if (tgtblk == 0)
+    {
+      tgtblk = assign_temp (build_qualified_type (type,
+                          (TYPE_QUALS (type)
+                           | TYPE_QUAL_CONST)),
+                1, 1);
+      preserve_temp_slots (tgtblk);
+    }
+  return copy_from_reg (tgtblk, srcreg, type);
+}
+
 /* Copy BLKmode value SRC into a register of mode MODE.  Return the
    register if it contains any data, otherwise return null.

diff --git a/gcc/expr.h b/gcc/expr.h
index 68cdb8d..f029671 100644
--- a/gcc/expr.h
+++ b/gcc/expr.h
@@ -334,6 +334,9 @@ extern rtx emit_group_move_into_temps (rtx);
    PARALLEL.  */
 extern void emit_group_store (rtx, rtx, tree, int);

+/* Copy object from a set of registers.  */
+extern rtx copy_from_reg (rtx, rtx, tree);
+
 /* Copy BLKmode object from a set of registers.  */
 extern rtx copy_blkmode_from_reg (rtx, rtx, tree);

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[gary at intrepid dot com]

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

--- Comment #37 from Gary Funck <gary at intrepid dot com> 2012-08-15 03:34:55 UTC ---
(In reply to comment #36)
> (In reply to comment #35)
> > Note that for the test case in comment #34 (and comment #9) to fail that the
> > MAX_FIXED_MODE_SIZE patch has to be applied, and likely GCC internal checking
> > has to be enabled.
> 
> This patch seems to work:
[...]

Thanks, I couldn't figure out how to successfully apply that git-svn formatted
patch to my svn based working copy.

Would you like to first apply your patch, update the trunk, and then I'll test
it against my patch, and then RFA if all goes well?

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[rguenth at gcc dot gnu.org]

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

--- Comment #38 from Richard Guenther <rguenth at gcc dot gnu.org> 2012-08-15 08:57:51 UTC ---
What are the code generation deficiencies you are targeting with this?  For
testcase #1 I get

sptr_result:
.LFB0:
        .cfi_startproc
        movq    S+8(%rip), %rdx
        movq    S(%rip), %rax
        ret

what would you expect instead?

I don't think we should change MAX_FIXED_MODE_SIZE, nor make use of
TImode unconditionally.

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[chip at pobox dot com]

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

--- Comment #39 from Chip Salzenberg <chip at pobox dot com> 2012-08-15 09:13:36 UTC ---
avoiding BLKmode avoids unnecessary spills to memory.  See Bug 28831 and Bug
41194 for examples.

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[rguenther at suse dot de]

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

--- Comment #40 from rguenther at suse dot de <rguenther at suse dot de> 2012-08-15 09:29:02 UTC ---
On Wed, 15 Aug 2012, chip at pobox dot com wrote:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20020
> 
> --- Comment #39 from Chip Salzenberg <chip at pobox dot com> 2012-08-15 09:13:36 UTC ---
> avoiding BLKmode avoids unnecessary spills to memory.  See Bug 28831 and Bug
> 41194 for examples.

It seems to me that the issue should be addressed in other ways than
fixing just the case that can be fixed using TImode

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[gary at intrepid dot com]

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

--- Comment #41 from Gary Funck <gary at intrepid dot com> 2012-08-15 13:47:37 UTC ---
(In reply to comment #38)
> What are the code generation deficiencies you are targeting with this?  For
> testcase #1 I get
> 
> sptr_result:
> .LFB0:
>         .cfi_startproc
>         movq    S+8(%rip), %rdx
>         movq    S(%rip), %rax
>         ret
> 
> what would you expect instead?
> 
> I don't think we should change MAX_FIXED_MODE_SIZE, nor make use of
> TImode unconditionally.

All three test cases were designed simply to make them easy to check via an RTL
scan for the presence/use of TImode.  I chose three arbitrary small test cases
with the only criteria that they used the struct's differently.

As far as the motivation for suggesting the change, I noted that several other
targets assign 128-bit struct's into TImode values.  Therefore, I assumed that
there must be some benefit, and that this was an oversight in the x86_64
implementation.

In the GUPC implementation of the UPC programming language, a pointer into
shared space has three components (virtual-offset,thread,phase). This
pointer-to-shared (PTS) can be represented in a "packed" mode, where it uses 64
bits, but gives up some range for each of the fields.  The more general form is
the "struct" representation which provides full range for the fields and is
128-bits -- as the name implies, the underlying PTS representation manipulated
by the compiler is a struct.  Note that the packed representation could be a
struct also (with bit fields), but back when this project was started code
generation for structs and bit-fields wasn't very good; the code quality was
better if the compiler hand-crafted the necessary bit field manipulations.

Since UPC programs use PTS's frequently, we found that targeting them to TImode
containers improved various micro-benchmarks.  We noted that other targets like
MIPS and PPC did this and assumed it would be a good idea for x86_64 to follow
suit.  We don't have any hard data on the level of performance improvement,
though as Chip noted, some modest space savings were gained in libstdc++.

I can re-run some UPC tests, if that helps, but they have a rather specific
usage pattern.  Maybe something like the SPEC benchmarks would be more
compelling, but I don't have access to them.

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[hjl.tools at gmail dot com]

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

--- Comment #42 from H.J. Lu <hjl.tools at gmail dot com> 2012-08-15 13:58:16 UTC ---
(In reply to comment #37)
> (In reply to comment #36)
> > (In reply to comment #35)
> > > Note that for the test case in comment #34 (and comment #9) to fail that the
> > > MAX_FIXED_MODE_SIZE patch has to be applied, and likely GCC internal checking
> > > has to be enabled.
> > 
> > This patch seems to work:
> [...]
> 
> Thanks, I couldn't figure out how to successfully apply that git-svn formatted
> patch to my svn based working copy.
> 
> Would you like to first apply your patch, update the trunk, and then I'll test
> it against my patch, and then RFA if all goes well?

I tried your patch + mine.  There are 2 problems:

1. Testcases should have

/* { dg-do compile } */
/* { dg-require-effective-target int128 } */

in this order.  Otherwise, dg-require-effective-target is ignored.
Or you combine them into

/* { dg-do compile { target int128 } } */

2. I got

FAIL: tmpdir-gcc.dg-struct-layout-1/t001 c_compat_y_tst.o compile,  (internal
compiler error)
FAIL: tmpdir-g++.dg-struct-layout-1/t025 cp_compat_y_tst.o compile,  (internal
compiler error)
FAIL: tmpdir-g++.dg-struct-layout-1/t027 cp_compat_y_tst.o compile,  (internal
compiler error)

In file included from
/export/build/gnu/gcc/build-x86_64-linux/gcc/testsuite/g++/g++.dg-struct-layout-1//t027_y.C:8:0:^M
/export/build/gnu/gcc/build-x86_64-linux/gcc/testsuite/g++/g++.dg-struct-layout-1//t027_test.h:
In function 'S2609 check2609(S2609, S2609*, S2609)':^M
/export/gnu/import/git/gcc/gcc/testsuite/g++.dg/compat/struct-layout-1_y1.h:75:10:
internal compiler error: in emit_move_insn, at expr.c:3469^M 
/export/gnu/import/git/gcc/gcc/testsuite/g++.dg/compat/struct-layout-1.h:502:27:
note: in expansion of macro 'TX'^M
/export/build/gnu/gcc/build-x86_64-linux/gcc/testsuite/g++/g++.dg-struct-layout-1//t027_test.h:10:1:
note: in expansion of macro 'U'^M
Please submit a full bug report,^M
with preprocessed source if appropriate.^M
See <http://gcc.gnu.org/bugs.html> for instructions.^M

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[hjl.tools at gmail dot com]

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

--- Comment #43 from H.J. Lu <hjl.tools at gmail dot com> 2012-08-15 14:21:05 UTC ---
The problem is we return a TI union in XF register
because the x86-64 psABI.

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[gary at intrepid dot com]

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

--- Comment #44 from Gary Funck <gary at intrepid dot com> 2012-08-15 14:45:42 UTC ---
(In reply to comment #43)
> The problem is we return a TI union in XF register
> because the x86-64 psABI.

Is this the same problem documented in comment #9?

The patch that you suggested in comment #36 doesn't fix it?

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #45 from Jakub Jelinek <jakub at gcc dot gnu.org> 2012-08-15 14:55:01 UTC ---
Changing this is generally very risky for ABI incompatibilities, many targets
base some of the decisions how to pass parameters or return values on the type
mode.

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[hjl.tools at gmail dot com]

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

--- Comment #46 from H.J. Lu <hjl.tools at gmail dot com> 2012-08-15 16:01:15 UTC ---
(In reply to comment #45)
> Changing this is generally very risky for ABI incompatibilities, many targets
> base some of the decisions how to pass parameters or return values on the type
> mode.

ABI is tightly controlled by i386 backend and shouldn't be a problem.
The main issue is middle-end isn't prepared to deal with it.

compute_record_mode has

  /* 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
      && host_integerp (TYPE_SIZE (type), 1)
      && GET_MODE_BITSIZE (mode) == TREE_INT_CST_LOW (TYPE_SIZE (type)))
    SET_TYPE_MODE (type, mode);
  else 
    SET_TYPE_MODE (type, mode_for_size_tree (TYPE_SIZE (type), MODE_INT, 1)); 

  /* If structure's known alignment is less than what the scalar
     mode would need, and it matters, then stick with BLKmode.  */
  if (TYPE_MODE (type) != BLKmode
      && STRICT_ALIGNMENT
      && ! (TYPE_ALIGN (type) >= BIGGEST_ALIGNMENT
            || TYPE_ALIGN (type) >= GET_MODE_ALIGNMENT (TYPE_MODE (type))))
    {            
      /* If this is the only reason this type is BLKmode, then
         don't force containing types to be BLKmode.  */
      TYPE_NO_FORCE_BLK (type) = 1;
      SET_TYPE_MODE (type, BLKmode);
    }

We need a target hook to proper handle union with a single XFmode.

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[hjl.tools at gmail dot com]

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

--- Comment #47 from H.J. Lu <hjl.tools at gmail dot com> 2012-08-16 00:00:25 UTC ---
Created attachment 28028
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=28028
A patch

Here is a patch which should be applied on top of

http://gcc.gnu.org/ml/gcc-patches/2012-08/msg01021.html

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[hjl at gcc dot gnu.org]

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

--- Comment #48 from hjl at gcc dot gnu.org <hjl at gcc dot gnu.org> 2012-08-17 19:59:51 UTC ---
Author: hjl
Date: Fri Aug 17 19:59:46 2012
New Revision: 190492

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=190492
Log:
Define MAX_FIXED_MODE_SIZE/TARGET_MEMBER_TYPE_FORCES_BLK for i386

gcc/

    PR target/20020
    * config/i386/i386.c (ix86_member_type_forces_blk): New function.
    (TARGET_MEMBER_TYPE_FORCES_BLK): New macro.

    * config/i386/i386.h (MAX_FIXED_MODE_SIZE): New macro.

gcc/testsuite/

    PR target/20020
    * gcc.target/i386/pr20020-1.c: New test.
    * gcc.target/i386/pr20020-2.c: Likewise.
    * gcc.target/i386/pr20020-3.c: Likewise.

Added:
    trunk/gcc/testsuite/gcc.target/i386/pr20020-1.c
    trunk/gcc/testsuite/gcc.target/i386/pr20020-2.c
    trunk/gcc/testsuite/gcc.target/i386/pr20020-3.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/i386/i386.c
    trunk/gcc/config/i386/i386.h
    trunk/gcc/testsuite/ChangeLog

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[hjl.tools at gmail dot com]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED
   Target Milestone|---                         |4.8.0

--- Comment #48 from hjl at gcc dot gnu.org <hjl at gcc dot gnu.org> 2012-08-17 19:59:51 UTC ---
Author: hjl
Date: Fri Aug 17 19:59:46 2012
New Revision: 190492

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=190492
Log:
Define MAX_FIXED_MODE_SIZE/TARGET_MEMBER_TYPE_FORCES_BLK for i386

gcc/

    PR target/20020
    * config/i386/i386.c (ix86_member_type_forces_blk): New function.
    (TARGET_MEMBER_TYPE_FORCES_BLK): New macro.

    * config/i386/i386.h (MAX_FIXED_MODE_SIZE): New macro.

gcc/testsuite/

    PR target/20020
    * gcc.target/i386/pr20020-1.c: New test.
    * gcc.target/i386/pr20020-2.c: Likewise.
    * gcc.target/i386/pr20020-3.c: Likewise.

Added:
    trunk/gcc/testsuite/gcc.target/i386/pr20020-1.c
    trunk/gcc/testsuite/gcc.target/i386/pr20020-2.c
    trunk/gcc/testsuite/gcc.target/i386/pr20020-3.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/i386/i386.c
    trunk/gcc/config/i386/i386.h
    trunk/gcc/testsuite/ChangeLog

--- Comment #49 from H.J. Lu <hjl.tools at gmail dot com> 2012-08-17 20:00:41 UTC ---
Fixed.

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[hjl at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED
   Target Milestone|---                         |4.8.0

--- Comment #49 from H.J. Lu <hjl.tools at gmail dot com> 2012-08-17 20:00:41 UTC ---
Fixed.

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[ubizjak at gmail dot com]

------- Comment #8 from ubizjak at gmail dot com  2008-08-03 17:44 -------
CCing HJ for ABI issue.


-- 

ubizjak at gmail dot com changed:

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


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

[Bug target/20020] New: x86_64 - 128 bit structs not targeted to TImode

[gary at intrepid dot com]

[Also posted to the GCC mailing list, http://gcc.gnu.org/ml/gcc/2005-
02/msg00692.html]


Given,

struct shared_ptr_struct
  {
    unsigned int phase  : 24;
    unsigned short thread : 16;
    void *addr;
  };

On the x86_64 (ie, Opteron[tm]) platform, GCC appears to designate the
underlying mode of this type as a BLKmode, instead of a TImode.  This
has implications in terms of the quality of the code that is generated
to copy and manipulate 128 bit structures (as defined in the example
above).

The decision to commit this type to a BLKmode value, originates
in this logic in mode_for_size():

      if (limit && size > MAX_FIXED_MODE_SIZE)
         return BLKmode;

On the x86 platform, there appears to be no target definition for
MAX_FIELD_SIZE. Thus, the default in stor-layout.c applies:

#ifndef MAX_FIXED_MODE_SIZE
#define MAX_FIXED_MODE_SIZE GET_MODE_BITSIZE (DImode)
#endif

Other 64 bit targets define MAX_FIXED_MODE_SIZE along these lines
(some line wrapping may occur below):

config/i960/i960.h:#define    MAX_FIXED_MODE_SIZE GET_MODE_BITSIZE
(TImode)
config/ia64/ia64.h:#define MAX_FIXED_MODE_SIZE GET_MODE_BITSIZE (TImode)
config/mips/mips.h:#define MAX_FIXED_MODE_SIZE LONG_DOUBLE_TYPE_SIZE
config/sh/sh.h:#define MAX_FIXED_MODE_SIZE (TARGET_SH5 ? 128 : 64)

on MIPS, LONG_DOUBLE_TYPE_SIZE is defined as follows:

/* A C expression for the size in bits of the type `long double' on
   the target machine.  If you don't define this, the default is two
   words.  */
#define LONG_DOUBLE_TYPE_SIZE \
  (mips_abi == ABI_N32 || mips_abi == ABI_64 ? 128 : 64)

In the 'dev' tree, the s390 defines MAX_FIXED_MODE_SIZE as follows:

config/s390/s390.h:#define MAX_FIXED_MODE_SIZE GET_MODE_BITSIZE
(TARGET_64BIT ? TImode : DImode)

(Arguably, the s390 variant might be a better default value to be defined
in stor-layout.c)

-- 
           Summary: x86_64 - 128 bit structs not targeted to TImode
           Product: gcc
           Version: 4.0.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: target
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: gary at intrepid dot com
                CC: gcc-bugs at gcc dot gnu dot org
  GCC host triplet: x86_64-unknown-linux-gnu


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

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2005-02-17 04:02 -------
You missed another important 64 bit target PPC (aka rs6000):
#define MAX_FIXED_MODE_SIZE GET_MODE_BITSIZE (TARGET_POWERPC64 ? TImode : DImode)

But note:
"MAX_FIXED_MODE_SIZE should be deprecated.  About the only thing that
it should be used for right now is mode assigment for aggregates."

See <http://gcc.gnu.org/ml/gcc/2005-01/msg00160.html>.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |enhancement
           Keywords|                            |missed-optimization


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

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[matz at suse dot de]

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |matz at suse dot de


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

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[gary at intrepid dot com]

------- Additional Comments From gary at intrepid dot com  2005-02-17 19:35 -------
The MAX_FIXED_REC_SIZE defintion is a relatively recent addition to 
config/rs6000.h (and from a quick review of the cvs log, it seems that this 
change hasn't yet been incorporated into a release.) Just fyi.

revision 1.330 
date: 2004/08/11 19:18:14;  author: dje;  state: Exp;  lines: +6 -0
        * config/rs6000/rs6000.h (MAX_FIXED_MODE_SIZE): Define.
         
        * config/rs6000/rs6000.md (mfcr rlwinm patterns): Set length to 8.
        (mfcr rlwinm rlwinm patterns): Set length to 12.




-- 


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

Re: [Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[Andrew Pinski]

On Feb 17, 2005, at 2:35 PM, gary at intrepid dot com wrote:

> The MAX_FIXED_REC_SIZE defintion is a relatively recent addition to
> config/rs6000.h (and from a quick review of the cvs log, it seems that 
> this
> change hasn't yet been incorporated into a release.) Just fyi.

Yes so, just a note the patch to fix this if it is approved will always 
have
to go on the mainline.  Also note MAX_FIXED_MODE_SIZE was changed on 
rs6000
for a reason and not for an optimization issue.

-- Pinski

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[pinskia at physics dot uc dot edu]

------- Additional Comments From pinskia at physics dot uc dot edu  2005-02-17 20:05 -------
Subject: Re:  x86_64 - 128 bit structs not targeted to TImode


On Feb 17, 2005, at 2:35 PM, gary at intrepid dot com wrote:

> The MAX_FIXED_REC_SIZE defintion is a relatively recent addition to
> config/rs6000.h (and from a quick review of the cvs log, it seems that 
> this
> change hasn't yet been incorporated into a release.) Just fyi.

Yes so, just a note the patch to fix this if it is approved will always 
have
to go on the mainline.  Also note MAX_FIXED_MODE_SIZE was changed on 
rs6000
for a reason and not for an optimization issue.

-- Pinski



-- 


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

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[gary at intrepid dot com]

------- Additional Comments From gary at intrepid dot com  2005-02-17 21:41 -------

> Also note MAX_FIXED_MODE_SIZE was changed on 
> rs6000 for a reason and not for an optimization issue.

Based on my interpretaion of the procedure call ABI, both this problem and a 
related Bug #19566 demonstrate a variance from the AMD 64 bit ABI (see "System 
V Application Binary Interface AMD64 Architecture Processor Supplement", Draft 
Version 0.95, January 14, 2005, section 3.2.3).

If this is a correct interpretation of the ABI, then both bugs are not just 
performance related.

-- 


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

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[matz at suse dot de]

------- Additional Comments From matz at suse dot de  2005-02-17 22:06 -------
I think that #19566 is a real bug.  The ABI specifies to pass 16byte 
structs in registers.  Anyway MAX_FIXED_MODE_SIZE doesn't influence 
the calling convention, only how such struct is handled by transforming 
code.  I.e. changing MAX_FIXED_MODE_SIZE shouldn't fix any ABI bug 
(in fact shouldn't change how parameters are passed at all). 
At least from my understanding and if there aren't other bugs making this 
false ;) 

-- 


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

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[gary at intrepid dot com]

------- Additional Comments From gary at intrepid dot com  2005-02-17 22:39 -------

> I think that Bug #19566 is a real bug.  The ABI specifies to pass 16byte 
> structs in registers.  Anyway MAX_FIXED_MODE_SIZE doesn't influence 
> the calling convention ...

Thanks for the clarification.  After re-reading classify_argument()
in config/i386/i386.c, I agree.  I had thought that for all BLKmode
values that it would target the argument into memory, however, for
aggregate types (records/unions, etc) it ignores the mode and looks
at each field (per the ABI). Thus, I agree this Bug #20020 is
performance-related only, and Bug #19566 illustrates a variance from
the procedure call ABI.


-- 


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

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[pinskia at gcc dot gnu dot org]

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
   GCC host triplet|x86_64-unknown-linux-gnu    |
 GCC target triplet|                            |x86_64-unknown-linux-gnu


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

[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode

[steven at gcc dot gnu dot org]

------- Additional Comments From steven at gcc dot gnu dot org  2005-06-25 11:56 -------
So it's agreed this is a valid bug...  Now, what are we going to do about it? 

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|                            |1
   Last reconfirmed|0000-00-00 00:00:00         |2005-06-25 11:56:35
               date|                            |


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