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.

Reply via email to