On 2/20/20 11:33 AM, Peter Bergner wrote:
> Ok, I pushed the trunk fix now. I'll kick off the release branch
> backports now.
>
> Jakub, I know you're getting the GCC 8.4 release ready. Is this fix ok
> for FSF 8 now or do you want me to wait until after 8.4 is out?
The backport of the PR93658 fix to GCC 8 & 9 showed a few testsuite regressions.
With Mike's help, we tracked those down to some fixes Mike had applied to
trunk, but did not backport. With the following patch, PR93658 is fixed in
GCC 9 with no regressions.
GCC 8 is also regression free with the small update to the fragile insn
counting in vss-vector-6-le.c. We made major changes to this test case
in trunk, but I think it's easier to just modify the count in the patch
hunk below.
Is this still ok for GCC 8 & 9?
Peter
rs6000: Fix infinite loop building ghostscript and icu [PR93658]
Fix rs6000_legitimate_address_p(), which erroneously marks a valid Altivec
address as being invalid, which causes LRA's process_address() to go into
an infinite loop spilling the same address over and over again.
Include Mike's earlier commits that fix bugs this patch exposes.
Backport from master
2020-02-20 Peter Bergner <berg...@linux.ibm.com>
PR target/93658
* config/rs6000/rs6000.c (rs6000_legitimate_address_p): Handle VSX
vector modes.
* gcc.target/powerpc/pr93658.c: New test.
Fix PR 93568 (thinko)
Backport from master
2020-02-05 Michael Meissner <meiss...@linux.ibm.com>
PR target/93568
* config/rs6000/rs6000.c (get_vector_offset): Fix
Adjust how variable vector extraction is done.
Backport from master
2020-02-03 Michael Meissner <meiss...@linux.ibm.com>
* config/rs6000/rs6000.c (get_vector_offset): New helper function
to calculate the offset in memory from the start of a vector of a
particular element. Add code to keep the element number in
bounds if the element number is variable.
(rs6000_adjust_vec_address): Move calculation of offset of the
vector element to get_vector_offset.
(rs6000_split_vec_extract_var): Do not do the initial AND of
element here, move the code to get_vector_offset.
Fix bad code of vector extract of PC-relative address with variable element #.
Backport from master
2020-01-06 Michael Meissner <meiss...@linux.ibm.com>
* config/rs6000/vsx.md (vsx_extract_<mode>_var, VSX_D iterator):
Use 'Q' for doing vector extract from memory.
(vsx_extract_v4sf_var): Use 'Q' for doing vector extract from
memory.
(vsx_extract_<mode>_var, VSX_EXTRACT_I iterator): Use 'Q' for
doing vector extract from memory.
(vsx_extract_<mode>_<VS_scalar>mode_var): Use 'Q' for doing vector
extract from memory.
Only for GCC 8:
* gcc.target/powerpc/vsx-vector-6-le.c: Update fragile insn count.
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 87d60078bb0..1d93570e7a5 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -7017,10 +7017,53 @@ rs6000_expand_vector_extract (rtx target, rtx vec, rtx
elt)
}
}
+/* Return the offset within a memory object (MEM) of a vector type to a given
+ element within the vector (ELEMENT) with an element size (SCALAR_SIZE). If
+ the element is constant, we return a constant integer.
+
+ Otherwise, we use a base register temporary to calculate the offset after
+ masking it to fit within the bounds of the vector and scaling it. The
+ masking is required by the 64-bit ELF version 2 ABI for the vec_extract
+ built-in function. */
+
+static rtx
+get_vector_offset (rtx mem, rtx element, rtx base_tmp, unsigned scalar_size)
+{
+ if (CONST_INT_P (element))
+ return GEN_INT (INTVAL (element) * scalar_size);
+
+ /* All insns should use the 'Q' constraint (address is a single register) if
+ the element number is not a constant. */
+ gcc_assert (satisfies_constraint_Q (mem));
+
+ /* Mask the element to make sure the element number is between 0 and the
+ maximum number of elements - 1 so that we don't generate an address
+ outside the vector. */
+ rtx num_ele_m1 = GEN_INT (GET_MODE_NUNITS (GET_MODE (mem)) - 1);
+ rtx and_op = gen_rtx_AND (Pmode, element, num_ele_m1);
+ emit_insn (gen_rtx_SET (base_tmp, and_op));
+
+ /* Shift the element to get the byte offset from the element number. */
+ int shift = exact_log2 (scalar_size);
+ gcc_assert (shift >= 0);
+
+ if (shift > 0)
+ {
+ rtx shift_op = gen_rtx_ASHIFT (Pmode, base_tmp, GEN_INT (shift));
+ emit_insn (gen_rtx_SET (base_tmp, shift_op));
+ }
+
+ return base_tmp;
+}
+
/* Adjust a memory address (MEM) of a vector type to point to a scalar field
within the vector (ELEMENT) with a mode (SCALAR_MODE). Use a base register
temporary (BASE_TMP) to fixup the address. Return the new memory address
- that is valid for reads or writes to a given register (SCALAR_REG). */
+ that is valid for reads or writes to a given register (SCALAR_REG).
+
+ This function is expected to be called after reload is completed when we are
+ splitting insns. The temporary BASE_TMP might be set multiple times with
+ this code. */
rtx
rs6000_adjust_vec_address (rtx scalar_reg,
@@ -7031,7 +7074,6 @@ rs6000_adjust_vec_address (rtx scalar_reg,
{
unsigned scalar_size = GET_MODE_SIZE (scalar_mode);
rtx addr = XEXP (mem, 0);
- rtx element_offset;
rtx new_addr;
bool valid_addr_p;
@@ -7040,26 +7082,7 @@ rs6000_adjust_vec_address (rtx scalar_reg,
/* Calculate what we need to add to the address to get the element
address. */
- if (CONST_INT_P (element))
- element_offset = GEN_INT (INTVAL (element) * scalar_size);
- else
- {
- int byte_shift = exact_log2 (scalar_size);
- gcc_assert (byte_shift >= 0);
-
- if (byte_shift == 0)
- element_offset = element;
-
- else
- {
- if (TARGET_POWERPC64)
- emit_insn (gen_ashldi3 (base_tmp, element, GEN_INT (byte_shift)));
- else
- emit_insn (gen_ashlsi3 (base_tmp, element, GEN_INT (byte_shift)));
-
- element_offset = base_tmp;
- }
- }
+ rtx element_offset = get_vector_offset (mem, element, base_tmp, scalar_size);
/* Create the new address pointing to the element within the vector. If we
are adding 0, we don't have to change the address. */
@@ -7194,13 +7217,9 @@ rs6000_split_vec_extract_var (rtx dest, rtx src, rtx
element, rtx tmp_gpr,
systems. */
if (MEM_P (src))
{
- int num_elements = GET_MODE_NUNITS (mode);
- rtx num_ele_m1 = GEN_INT (num_elements - 1);
-
- emit_insn (gen_anddi3 (element, element, num_ele_m1));
- gcc_assert (REG_P (tmp_gpr));
- emit_move_insn (dest, rs6000_adjust_vec_address (dest, src, element,
- tmp_gpr, scalar_mode));
+ emit_move_insn (dest,
+ rs6000_adjust_vec_address (dest, src, element, tmp_gpr,
+ scalar_mode));
return;
}
@@ -9339,7 +9358,7 @@ rs6000_legitimate_address_p (machine_mode mode, rtx x,
bool reg_ok_strict)
bool quad_offset_p = mode_supports_dq_form (mode);
/* If this is an unaligned stvx/ldvx type address, discard the outer AND. */
- if (VECTOR_MEM_ALTIVEC_P (mode)
+ if (VECTOR_MEM_ALTIVEC_OR_VSX_P (mode)
&& GET_CODE (x) == AND
&& CONST_INT_P (XEXP (x, 1))
&& INTVAL (XEXP (x, 1)) == -16)
diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md
index 607c0cd33f2..5973e0a399d 100644
--- a/gcc/config/rs6000/vsx.md
+++ b/gcc/config/rs6000/vsx.md
@@ -3295,7 +3295,7 @@
;; Variable V2DI/V2DF extract
(define_insn_and_split "vsx_extract_<mode>_var"
[(set (match_operand:<VS_scalar> 0 "gpc_reg_operand" "=v,<VSa>,r")
- (unspec:<VS_scalar> [(match_operand:VSX_D 1 "input_operand" "v,m,m")
+ (unspec:<VS_scalar> [(match_operand:VSX_D 1 "input_operand" "v,Q,Q")
(match_operand:DI 2 "gpc_reg_operand" "r,r,r")]
UNSPEC_VSX_EXTRACT))
(clobber (match_scratch:DI 3 "=r,&b,&b"))
@@ -3364,7 +3364,7 @@
;; Variable V4SF extract
(define_insn_and_split "vsx_extract_v4sf_var"
[(set (match_operand:SF 0 "gpc_reg_operand" "=ww,ww,?r")
- (unspec:SF [(match_operand:V4SF 1 "input_operand" "v,m,m")
+ (unspec:SF [(match_operand:V4SF 1 "input_operand" "v,Q,Q")
(match_operand:DI 2 "gpc_reg_operand" "r,r,r")]
UNSPEC_VSX_EXTRACT))
(clobber (match_scratch:DI 3 "=r,&b,&b"))
@@ -3724,7 +3724,7 @@
(define_insn_and_split "vsx_extract_<mode>_var"
[(set (match_operand:<VS_scalar> 0 "gpc_reg_operand" "=r,r,r")
(unspec:<VS_scalar>
- [(match_operand:VSX_EXTRACT_I 1 "input_operand" "wK,v,m")
+ [(match_operand:VSX_EXTRACT_I 1 "input_operand" "wK,v,Q")
(match_operand:DI 2 "gpc_reg_operand" "r,r,r")]
UNSPEC_VSX_EXTRACT))
(clobber (match_scratch:DI 3 "=r,r,&b"))
@@ -3743,7 +3743,7 @@
[(set (match_operand:<VS_scalar> 0 "gpc_reg_operand" "=r,r,r")
(zero_extend:<VS_scalar>
(unspec:<VSX_EXTRACT_I:VS_scalar>
- [(match_operand:VSX_EXTRACT_I 1 "input_operand" "wK,v,m")
+ [(match_operand:VSX_EXTRACT_I 1 "input_operand" "wK,v,Q")
(match_operand:DI 2 "gpc_reg_operand" "r,r,r")]
UNSPEC_VSX_EXTRACT)))
(clobber (match_scratch:DI 3 "=r,r,&b"))
diff --git a/gcc/testsuite/gcc.target/powerpc/pr93658.c
b/gcc/testsuite/gcc.target/powerpc/pr93658.c
new file mode 100644
index 00000000000..0170d34d259
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr93658.c
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-options "-O3 -fstack-protector-strong -mcpu=power8" } */
+/* { dg-require-effective-target powerpc_vsx_ok } */
+
+/* PR93658: Failure compiling this test is an infinite loop in LRA's
+ process_address(), so set a short timeout limit. */
+/* { dg-timeout 5 } */
+
+void bar();
+char b;
+void
+foo (void)
+{
+ char a;
+ int d = b;
+ char *e = &a;
+ while (d)
+ *e++ = --d;
+ bar ();
+}
Only for GCC 8:
diff --git a/gcc/testsuite/gcc.target/powerpc/vsx-vector-6-le.c
b/gcc/testsuite/gcc.target/powerpc/vsx-vector-6-le.c
index fe7eeb12ff9..dc6ccb77d1f 100644
--- a/gcc/testsuite/gcc.target/powerpc/vsx-vector-6-le.c
+++ b/gcc/testsuite/gcc.target/powerpc/vsx-vector-6-le.c
@@ -14,7 +14,7 @@
their usage counts being stable. Therefore, we just ensure at least one
xxlor instruction was generated. */
/* { dg-final { scan-assembler "xxlor" } } */
-/* { dg-final { scan-assembler-times "xvcmpeqdp" 5 } } */
+/* { dg-final { scan-assembler-times "xvcmpeqdp" 6 } } */
/* { dg-final { scan-assembler-times "xvcmpgtdp" 8 } } */
/* { dg-final { scan-assembler-times "xvcmpgedp" 6 } } */
/* { dg-final { scan-assembler-times "xvrdpim" 1 } } */
