Bug 53647 - [4.8 Regression] gcc.c-torture/compile/20011229-1.c and gcc.c-torture/compile/pr25311.c
Summary: [4.8 Regression] gcc.c-torture/compile/20011229-1.c and gcc.c-torture/compile...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 4.8.0
: P3 normal
Target Milestone: 4.8.0
Assignee: Bill Schmidt
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-06-12 15:43 UTC by H.J. Lu
Modified: 2012-06-17 19:35 UTC (History)
3 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2012-06-12 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description H.J. Lu 2012-06-12 15:43:43 UTC
On Linux/x86, revision 188457 gave

FAIL: gcc.c-torture/compile/20011229-1.c  -Os  (test for excess errors)
FAIL: gcc.c-torture/compile/pr25311.c  -Os  (internal compiler error)

Revision 188451 is OK.
Comment 1 Bill Schmidt 2012-06-12 17:27:18 UTC
Can you please post the symptom of the ICE?  These errors don't occur on ppc so will be some work for me to replicate.
Comment 2 H.J. Lu 2012-06-12 17:45:40 UTC
/export/build/gnu/gcc-x32/build-x86_64-linux/gcc/xgcc -B/export/build/gnu/gcc-x32/build-x86_64-linux/gcc/ -fno-diagnostics-show-caret -Os -w -c -o pr25311.o /export/gnu/import/git/gcc/gcc/testsuite/gcc.c-torture/compile/pr25311.c
/export/gnu/import/git/gcc/gcc/testsuite/gcc.c-torture/compile/pr25311.c: In function ‘set_size’:
/export/gnu/import/git/gcc/gcc/testsuite/gcc.c-torture/compile/pr25311.c:16:1: internal compiler error: Floating point exception
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.

Program received signal SIGFPE, Arithmetic exception.
0x0000000000cdefb5 in hoist_adjacent_loads (bb0=0x7ffff1326d90, 
    bb1=0x7ffff1326e00, bb2=0x7ffff1326e70, bb3=0x7ffff1326ee0)
    at /export/gnu/import/git/gcc/gcc/tree-ssa-phiopt.c:1941
1941	      align1 = DECL_ALIGN (field1) % param_align_bits;
(gdb) p param_align_bits
$1 = 0
(gdb)
Comment 3 H.J. Lu 2012-06-12 17:56:04 UTC
i386.c has

  maybe_set_param_value (PARAM_L1_CACHE_LINE_SIZE, ix86_cost->prefetch_block,
                         global_options.x_param_values,
                         global_options_set.x_param_values);

But some costs have

  0,                                    /* size of l1 cache  */
  0,                                    /* size of l2 cache  */
  0,                                    /* size of prefetch block */
  0,                                    /* number of parallel prefetches */

and we get param_align == 0:

+  int param_align = PARAM_VALUE (PARAM_L1_CACHE_LINE_SIZE);
+  unsigned param_align_bits = (unsigned) (param_align * BITS_PER_UNIT);
+  gimple_stmt_iterator gsi;
Comment 4 Bill Schmidt 2012-06-12 19:05:55 UTC
So, appears to be a target issue.  The only other use of L1_CACHE_SIZE in the top-level gcc directory is in tree-ssa-loop-prefetch.c, where 0 is apparently undetected but causes prefetching to be quietly disabled.  Specifying 0 doesn't appear to be intentionally valid.
Comment 5 Bill Schmidt 2012-06-12 19:09:48 UTC
If this is incorrect, and zero is supposed to indicate a cacheless memory (do they exist anymore?), then I can disable the adjacent-loads hoisting optimization for that case.  Someone know the answer for this?
Comment 6 H.J. Lu 2012-06-12 19:26:12 UTC
We should update i386.c to have reasonable values for
size of l1 cache, size of l2 cache, size of prefetch block
and number of parallel prefetches, instead of 0s.
Comment 7 Uroš Bizjak 2012-06-12 19:40:21 UTC
Perhaps simply:

--cut here--
Index: tree-ssa-phiopt.c
===================================================================
--- tree-ssa-phiopt.c   (revision 188475)
+++ tree-ssa-phiopt.c   (working copy)
@@ -1830,6 +1830,11 @@
   unsigned param_align_bits = (unsigned) (param_align * BITS_PER_UNIT);
   gimple_stmt_iterator gsi;
 
+  /* We assume that transformation is not profitable
+     on targets without cache.  */
+  if (param_align_bits == 0)
+    return;
+
   /* Walk the phis in bb3 looking for an opportunity.  We are looking
      for phis of two SSA names, one each of which is defined in bb1 and
      bb2.  */
--cut here--
Comment 8 Uroš Bizjak 2012-06-12 19:45:56 UTC
Alternative patch with the same functionality:

--cut here--
Index: tree-ssa-phiopt.c
===================================================================
--- tree-ssa-phiopt.c   (revision 188475)
+++ tree-ssa-phiopt.c   (working copy)
@@ -1976,12 +1976,14 @@
 /* Determine whether we should attempt to hoist adjacent loads out of
    diamond patterns in pass_phiopt.  Always hoist loads if
    -fhoist-adjacent-loads is specified and the target machine has
-   a conditional move instruction.  */
+   defined cache line size and a conditional move instruction.  */
 
 static bool
 gate_hoist_loads (void)
 {
-  return (flag_hoist_adjacent_loads == 1 && HAVE_conditional_move);
+  return (flag_hoist_adjacent_loads == 1
+         && PARAM_VALUE (PARAM_L1_CACHE_LINE_SIZE)
+         && HAVE_conditional_move);
 }
 
 /* Always do these optimizations if we have SSA
--cut here--
Comment 9 Bill Schmidt 2012-06-12 19:46:30 UTC
Yes, that can be done, but the question is whether it is correct, or just hiding the issue.  Do the processors really not have cache?  Or was this just an error not filling in the values?  I don't want to hide a real problem if that's the situation.  Waiting for someone to clarify the intent behind these processor descriptions.

There's nothing in the documentation of the parameter to suggest zero is a magic number for no cache, but nothing to suggest otherwise, either.
Comment 10 Bill Schmidt 2012-06-12 20:30:46 UTC
OK, after looking at these machine descriptions I think we will have to tolerate a line size of 0.  They appear to be for the ancient 386 chips that didn't have onboard cache, and sometimes no offboard cache.  So I'll go ahead and make the fix.
Comment 11 Bill Schmidt 2012-06-13 12:34:02 UTC
Author: wschmidt
Date: Wed Jun 13 12:33:55 2012
New Revision: 188509

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=188509
Log:
2012-06-13  Bill Schmidt  <wschmidt@linux.ibm.com>

	PR tree-optimization/53647
	* tree-ssa-phiopt.c (gate_hoist_loads): Skip transformation for
	targets with no defined cache line size.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/tree-ssa-phiopt.c
Comment 12 Bill Schmidt 2012-06-13 12:40:20 UTC
Fixed.
Comment 13 hjl@gcc.gnu.org 2012-06-13 17:47:11 UTC
Author: hjl
Date: Wed Jun 13 17:46:59 2012
New Revision: 188523

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=188523
Log:
Set cache values from -mtune

	PR target/53647
	* config/i386/i386.c (ix86_tune_cost): New variable.
	(ix86_option_override_internal): Set ix86_tune_cost.  Use
	ix86_tune_cost for simultaneous_prefetches, prefetch_block,
	l1_cache_size and l2_cache_size.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/i386/i386.c