This problem reports an assertion error when certain rtl expressions
which are not eligible as producers or consumers of a store bypass
optimization are passed as arguments to the store_data_bypass_p
function. Since the problem surfaced with tests targeting the rs6000
architecture, the proposed patch is integrated within the rs6000 back
end.
A new rs6000_store_data_bypass_p function has been introduced and all
calls to store_data_bypass_p from within the rs6000 back end have been
replaced with calls to rs6000_store_data_bypass_p. This new function
scans its arguments for patterns that are known to cause assertion
errors in store_data_bypass_p and returns false if any of those
patterns are encountered. Otherwise, rs6000_store_data_bypass_p simply
returns the result produced when passing its arguments to a call of
store_data_bypass_p.
Thank you for feedback and guidance from Eric Botcazou, Segher
Boessenkool, Richard Sandiford, and Pat Haugen which was offered in
response to my first two patch submissions and an RFC post on this
topic. With all of your help, I now have a much better understanding
of the intended role of store_data_bypass_p.
The patch has been boostrapped without regressions on
powerpc64le-unknown-linux-gnu. Is this ok for the trunk?
gcc/testsuite/ChangeLog:
2017-04-20 Kelvin Nilsen <kel...@gcc.gnu.org>
PR target/80101
* gcc.target/powerpc/pr80101-1.c: New test.
gcc/ChangeLog:
2017-04-20 Kelvin Nilsen <kel...@gcc.gnu.org>
PR target/80101
* config/rs6000/power6.md: Replace store_data_bypass_p calls with
rs6000_store_data_bypass_p in seven define_bypass directives and
in several comments.
* config/rs6000/rs6000-protos.h: Add prototype for
rs6000_store_data_bypass_p function.
* config/rs6000/rs6000.c (rs6000_store_data_bypass_p): New
function implements slightly different (rs6000-specific) semantics
than store_data_bypass_p, returning false rather than aborting
with assertion error when arguments do not satisfy the
requirements of store data bypass.
(rs6000_adjust_cost): Replace six calls of store_data_bypass_p with
rs6000_store_data_bypass_p.
Index: gcc/config/rs6000/power6.md
===================================================================
--- gcc/config/rs6000/power6.md (revision 246469)
+++ gcc/config/rs6000/power6.md (working copy)
@@ -108,7 +108,7 @@
power6-store-update-indexed,\
power6-fpstore,\
power6-fpstore-update"
- "store_data_bypass_p")
+ "rs6000_store_data_bypass_p")
(define_insn_reservation "power6-load-ext" 4 ; fx
(and (eq_attr "type" "load")
@@ -128,7 +128,7 @@
power6-store-update-indexed,\
power6-fpstore,\
power6-fpstore-update"
- "store_data_bypass_p")
+ "rs6000_store_data_bypass_p")
(define_insn_reservation "power6-load-update" 2 ; fx
(and (eq_attr "type" "load")
@@ -276,7 +276,7 @@
power6-store-update-indexed,\
power6-fpstore,\
power6-fpstore-update"
- "store_data_bypass_p")
+ "rs6000_store_data_bypass_p")
(define_insn_reservation "power6-cntlz" 2
(and (eq_attr "type" "cntlz")
@@ -289,7 +289,7 @@
power6-store-update-indexed,\
power6-fpstore,\
power6-fpstore-update"
- "store_data_bypass_p")
+ "rs6000_store_data_bypass_p")
(define_insn_reservation "power6-var-rotate" 4
(and (eq_attr "type" "shift")
@@ -355,7 +355,7 @@
power6-store-update-indexed,\
power6-fpstore,\
power6-fpstore-update"
- "store_data_bypass_p")
+ "rs6000_store_data_bypass_p")
(define_insn_reservation "power6-delayed-compare" 2 ; N/A
(and (eq_attr "type" "shift")
@@ -420,7 +420,7 @@
power6-store-update-indexed,\
power6-fpstore,\
power6-fpstore-update"
- "store_data_bypass_p")
+ "rs6000_store_data_bypass_p")
(define_insn_reservation "power6-idiv" 44
(and (eq_attr "type" "div")
@@ -436,7 +436,7 @@
; power6-store-update-indexed,\
; power6-fpstore,\
; power6-fpstore-update"
-; "store_data_bypass_p")
+; "rs6000_store_data_bypass_p")
(define_insn_reservation "power6-ldiv" 56
(and (eq_attr "type" "div")
@@ -452,7 +452,7 @@
; power6-store-update-indexed,\
; power6-fpstore,\
; power6-fpstore-update"
-; "store_data_bypass_p")
+; "rs6000_store_data_bypass_p")
(define_insn_reservation "power6-mtjmpr" 2
(and (eq_attr "type" "mtjmpr,mfjmpr")
@@ -510,7 +510,7 @@
(define_bypass 1 "power6-fp"
"power6-fpstore,power6-fpstore-update"
- "store_data_bypass_p")
+ "rs6000_store_data_bypass_p")
(define_insn_reservation "power6-fpcompare" 8
(and (eq_attr "type" "fpcompare")
Index: gcc/config/rs6000/rs6000-protos.h
===================================================================
--- gcc/config/rs6000/rs6000-protos.h (revision 246469)
+++ gcc/config/rs6000/rs6000-protos.h (working copy)
@@ -226,6 +226,7 @@ extern void rs6000_aix_asm_output_dwarf_table_ref
extern void get_ppc476_thunk_name (char name[32]);
extern bool rs6000_overloaded_builtin_p (enum rs6000_builtins);
extern const char *rs6000_overloaded_builtin_name (enum rs6000_builtins);
+extern int rs6000_store_data_bypass_p (rtx_insn *, rtx_insn *);
extern HOST_WIDE_INT rs6000_builtin_mask_calculate (void);
extern void rs6000_asm_output_dwarf_pcrel (FILE *file, int size,
const char *label);
Index: gcc/config/rs6000/rs6000.c
===================================================================
--- gcc/config/rs6000/rs6000.c (revision 246469)
+++ gcc/config/rs6000/rs6000.c (working copy)
@@ -508,6 +508,91 @@ mode_supports_pre_modify_p (machine_mode mode)
!= 0);
}
+/* Given that there exists at least one variable that is set (produced)
+ by OUT_INSN and read (consumed) by IN_INSN, return true iff
+ IN_INSN represents one or more memory store operations and none of
+ the variables set by OUT_INSN is used by IN_INSN as the address of a
+ store operation. If either IN_INSN or OUT_INSN does not represent
+ a "single" RTL SET expression (as loosely defined by the
+ implementation of the single_set function) or a PARALLEL with only
+ SETs, CLOBBERs, and USEs inside, this function returns false.
+
+ This rs6000-specific version of store_data_bypass_p checks for
+ certain conditions that result in assertion failures (and internal
+ compiler errors) in the generic store_data_bypass_p function and
+ returns false rather than calling store_data_bypass_p if one of the
+ problematic conditions is detected. */
+
+int
+rs6000_store_data_bypass_p (rtx_insn *out_insn, rtx_insn *in_insn)
+{
+ rtx out_set, in_set;
+ rtx out_pat, in_pat;
+ rtx out_exp, in_exp;
+ int i, j;
+
+ in_set = single_set (in_insn);
+ if (in_set)
+ {
+ if (MEM_P (SET_DEST (in_set)))
+ {
+ out_set = single_set (out_insn);
+ if (!out_set)
+ {
+ out_pat = PATTERN (out_insn);
+ if (GET_CODE (out_pat) == PARALLEL)
+ {
+ for (i = 0; i < XVECLEN (out_pat, 0); i++)
+ {
+ out_exp = XVECEXP (out_pat, 0, i);
+ if ((GET_CODE (out_exp) == CLOBBER)
+ || (GET_CODE (out_exp) == USE))
+ continue;
+ else if (GET_CODE (out_exp) != SET)
+ return false;
+ }
+ }
+ }
+ }
+ }
+ else
+ {
+ in_pat = PATTERN (in_insn);
+ if (GET_CODE (in_pat) != PARALLEL)
+ return false;
+
+ for (i = 0; i < XVECLEN (in_pat, 0); i++)
+ {
+ in_exp = XVECEXP (in_pat, 0, i);
+ if ((GET_CODE (in_exp) == CLOBBER) || (GET_CODE (in_exp) == USE))
+ continue;
+ else if (GET_CODE (in_exp) != SET)
+ return false;
+
+ if (MEM_P (SET_DEST (in_exp)))
+ {
+ out_set = single_set (out_insn);
+ if (!out_set)
+ {
+ out_pat = PATTERN (out_insn);
+ if (GET_CODE (out_pat) != PARALLEL)
+ return false;
+ for (j = 0; j < XVECLEN (out_pat, 0); j++)
+ {
+ out_exp = XVECEXP (out_pat, 0, j);
+ if ((GET_CODE (out_exp) == CLOBBER)
+ || (GET_CODE (out_exp) == USE))
+ continue;
+ else if (GET_CODE (out_exp) != SET)
+ return false;
+ }
+ }
+ }
+ }
+ }
+ return store_data_bypass_p (out_insn, in_insn);
+}
+
/* Return true if we have D-form addressing in altivec registers. */
static inline bool
mode_supports_vmx_dform (machine_mode mode)
@@ -32999,7 +33084,7 @@ rs6000_adjust_cost (rtx_insn *insn, int dep_type,
case TYPE_LOAD:
case TYPE_CNTLZ:
{
- if (! store_data_bypass_p (dep_insn, insn))
+ if (! rs6000_store_data_bypass_p (dep_insn, insn))
return get_attr_sign_extend (dep_insn)
== SIGN_EXTEND_YES ? 6 : 4;
break;
@@ -33006,7 +33091,7 @@ rs6000_adjust_cost (rtx_insn *insn, int dep_type,
}
case TYPE_SHIFT:
{
- if (! store_data_bypass_p (dep_insn, insn))
+ if (! rs6000_store_data_bypass_p (dep_insn, insn))
return get_attr_var_shift (dep_insn) == VAR_SHIFT_YES ?
6 : 3;
break;
@@ -33017,7 +33102,7 @@ rs6000_adjust_cost (rtx_insn *insn, int dep_type,
case TYPE_EXTS:
case TYPE_INSERT:
{
- if (! store_data_bypass_p (dep_insn, insn))
+ if (! rs6000_store_data_bypass_p (dep_insn, insn))
return 3;
break;
}
@@ -33026,19 +33111,19 @@ rs6000_adjust_cost (rtx_insn *insn, int dep_type,
case TYPE_FPSTORE:
{
if (get_attr_update (dep_insn) == UPDATE_YES
- && ! store_data_bypass_p (dep_insn, insn))
+ && ! rs6000_store_data_bypass_p (dep_insn, insn))
return 3;
break;
}
case TYPE_MUL:
{
- if (! store_data_bypass_p (dep_insn, insn))
+ if (! rs6000_store_data_bypass_p (dep_insn, insn))
return 17;
break;
}
case TYPE_DIV:
{
- if (! store_data_bypass_p (dep_insn, insn))
+ if (! rs6000_store_data_bypass_p (dep_insn, insn))
return get_attr_size (dep_insn) == SIZE_32 ? 45 : 57;
break;
}
Index: gcc/testsuite/gcc.target/powerpc/pr80101-1.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr80101-1.c (revision 0)
+++ gcc/testsuite/gcc.target/powerpc/pr80101-1.c (working copy)
@@ -0,0 +1,22 @@
+/* { dg-do compile { target { powerpc*-*-* } } } */
+/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } {
"-mcpu=power6" } } */
+/* { dg-require-effective-target dfp_hw } */
+/* { dg-options "-mcpu=power6 -mno-sched-epilog -Ofast" } */
+
+/* Prior to resolving PR 80101, this test case resulted in an internal
+ compiler error. The role of this test program is to assure that
+ dejagnu's "test for excess errors" does not find any. */
+
+int b;
+
+void e ();
+
+int c ()
+{
+ struct
+ {
+ int a[b];
+ } d;
+ if (d.a[0])
+ e ();
+}
