[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode
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=gccview=revrev=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
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=gccview=revrev=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
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
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 movqS+8(%rip), %rdx movqS(%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
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
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
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 movqS+8(%rip), %rdx movqS(%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
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
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
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
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
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
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
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
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 textdata 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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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.ang...@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
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.hWed 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:
0x006390fe in expand_call (exp=0x714a51e0, target=0x7107b980,
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
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
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
--- 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] x86_64 - 128 bit structs not targeted to TImode
--- 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|-00-00 00:00:00 |2005-06-25 11:56:35 date|| http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20020
[Bug target/20020] x86_64 - 128 bit structs not targeted to TImode
-- 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
-- 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
--- 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
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
--- 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
--- 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
--- 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
--- 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
--- 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
