Re: [C++ PATCH] [PR87770] test partial specializations for type dependence
On Jan 24, 2019, Jason Merrill wrote:
> The latter; you can't have a partial specialization in a function.
*nod* (though not entirely reflected in the patch below, I see)
>> Any suggestion of a good name for the inline function (or would you
>> prefer it to be a macro?) that tests whether a decl satisfies this
>> predicate? primary_or_partial_spec_p?
> Sounds good.
I was not entirely clear on what the predicate was supposed to be when I
wrote the above. I hadn't fully realized we were testing properties of
a template instantiation by inspecting mostly properties of the
template, rather than of the instantiation proper. Once I realized
that, I hesitated between introducing a function to test properties of
the base template directly, or a function to test the instantiation for
those properties. It wasn't clear to me that having e.g. only
DECL_TI_TEMPLATE as an argument would be enough to test everything we
needed: we wouldn't have the context (should be the same) or the
template args (certainly not the same, but sharing the same depth?) of
the instantiation we were supposed to assess to begin with.
So I went with a different name that reflected more closely the test I
implemented: instantiates_primary_template_p.
Now, maybe we're better off with something that tests the template
rather than the instantiation, to use at other places where
PRIMARY_TEMPLATE_P is found insufficient. If that's the case, I'll have
to figure out whether taking just the template is enough, or whether we
need the tinfo object or are better off taking additional arguments.
But since that will take additional investigation and you had nodded to
the logic that involved the args of the instantiation, I'm leaving it at
this for now. Please let me know whether the alternate form would be
preferred.
This patch bootstrapped on x86_64- and i686-linux-gnu, and is undergoing
regression testing ATM. Ok to install if it passes?
for gcc/cp/ChangeLog
PR c++/87770
* pt.c (instantiates_primary_template_p): New.
(type_dependent_expression_p): Use it.
for gcc/testsuite/ChangeLog
PR c++/87770
* g++.dg/pr87770.C: New.
---
gcc/cp/pt.c| 55 +++-
gcc/testsuite/g++.dg/pr87770.C | 11
2 files changed, 65 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/pr87770.C
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 48c180cc13b3..d413fa81c59e 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -400,6 +400,59 @@ template_class_depth (tree type)
return depth;
}
+/* Return TRUE if NODE instantiates a template that has arguments of
+ its own, be it directly a primary template or indirectly through a
+ partial specializations. */
+static inline bool
+instantiates_primary_template_p (tree node)
+{
+ tree tinfo;
+ if (!DECL_P (node))
+tinfo = CLASSTYPE_TEMPLATE_INFO (node);
+ else if (DECL_LANG_SPECIFIC (node))
+tinfo = DECL_TEMPLATE_INFO (node);
+ else
+tinfo = NULL_TREE;
+
+ if (!tinfo)
+return false;
+
+ tree tmpl = TI_TEMPLATE (tinfo);
+ if (PRIMARY_TEMPLATE_P (tmpl))
+return true;
+
+ if (!DECL_TEMPLATE_SPECIALIZATION (tmpl))
+return false;
+
+ /* So now we know we have a specialization, but it could be a full
+ or a partial specialization. To tell which, compare the depth of
+ its template arguments with those of its context. ??? How do we
+ tell apart a partial from a full explicit specialization in a
+ non-template context? */
+
+ tree ctxt;
+ if (!DECL_P (node))
+ctxt = TYPE_CONTEXT (node);
+ else
+ctxt = DECL_CONTEXT (node);
+
+ tree ctinfo;
+ if (!DECL_P (ctxt))
+ctinfo = CLASSTYPE_TEMPLATE_INFO (ctxt);
+ else if (DECL_LANG_SPECIFIC (ctxt))
+ctinfo = DECL_TEMPLATE_INFO (ctxt);
+ else
+ctinfo = NULL_TREE;
+
+ int cdepth;
+ if (!ctinfo)
+cdepth = 0;
+ else
+cdepth = TMPL_ARGS_DEPTH (TI_ARGS (ctinfo));
+
+ return (TMPL_ARGS_DEPTH (TI_ARGS (tinfo)) > cdepth);
+}
+
/* Subroutine of maybe_begin_member_template_processing.
Returns true if processing DECL needs us to push template parms. */
@@ -25622,7 +25675,7 @@ type_dependent_expression_p (tree expression)
that come from the template-id; the template arguments for the
enclosing class do not make it type-dependent unless they are used in
the type of the decl. */
- if (PRIMARY_TEMPLATE_P (DECL_TI_TEMPLATE (expression))
+ if (instantiates_primary_template_p (expression)
&& (any_dependent_template_arguments_p
(INNERMOST_TEMPLATE_ARGS (DECL_TI_ARGS (expression)
return true;
diff --git a/gcc/testsuite/g++.dg/pr87770.C b/gcc/testsuite/g++.dg/pr87770.C
new file mode 100644
index ..69eff4a786fe
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr87770.C
@@ -0,0 +1,11 @@
+// { dg-do compile }
+
+template struct d {
+ template d(e);
+};
+template <> template d::d(e);
+template <> template
Re: PING [PATCH] tighten up -Wbuiltin-declaration-mismatch (PR 86125, 88886, 86308)
It's also broken the build of the glibc testsuite, e.g.: ../time/time.h:88:15: error: mismatch in argument 1 type of built-in function 'strftime'; expected 'char *' [-Werror=builtin-declaration-mismatch] 88 | extern size_t strftime (char *__restrict __s, size_t __maxsize, (presence or absence of qualifiers on a parameter is not part of the function type and should not be compared here). -- Joseph S. Myers jos...@codesourcery.com
Re: C++ PATCH for c++/89024 - ICE with incomplete enum type
On Fri, Jan 25, 2019 at 12:14:07PM -0500, Jason Merrill wrote:
> On 1/25/19 12:09 PM, Marek Polacek wrote:
> > On Fri, Jan 25, 2019 at 10:55:55AM -0600, Tim Song wrote:
> > > On Thu, Jan 24, 2019 at 4:14 PM Jason Merrill wrote:
> > > >
> > > > On 1/24/19 2:16 PM, Marek Polacek wrote:
> > > > > This test ICEs since r159006 which added
> > > > >
> > > > > type = ENUM_UNDERLYING_TYPE (type);
> > > > >
> > > > > to type_promotes_to. In this test ENUM_UNDERLYING_TYPE is null
> > > > > because we
> > > > > haven't yet parsed '}' of the enum and the underlying type isn't
> > > > > fixed, and
> > > > > so checking TYPE_UNSIGNED crashed.
> > > > >
> > > > > I've added some checks to the test to see if the types seem to be OK;
> > > > > clang++
> > > > > agrees.
> > > > >
> > > > > Bootstrapped/regtested on x86_64-linux, ok for trunk/8/7?
> > > > >
> > > > > 2019-01-24 Marek Polacek
> > > > >
> > > > >PR c++/89024 - ICE with incomplete enum type.
> > > > >* cvt.c (type_promotes_to): Check if prom is non-null.
> > > >
> > > > 9.6/6: An enumeration whose underlying type is not fixed is an
> > > > incomplete type from its point of declaration to immediately after the
> > > > closing } of its enum-specifier, at which point it becomes a complete
> > > > type.
> > > >
> > > > So the conversion is ill-formed.
> > > >
> > > > Jason
> > >
> > > But the conversion in the example (in
> > > decltype(__test_aux<_To1>(declval<_From1>(
> > > is in a SFINAE context, so shouldn't it gracefully fall back to the
> > > `(...)` overload?
> >
> > I think so, and clang++ and icc also compile the testcase fine (and we used
> > to
> > too, before r159006).
>
> Absolutely, the conversion being ill-formed means substitution fails, and we
> reject that candidate. I meant that we shouldn't get as far as
> type_promotes_to for an incomplete type.
Makes sense. So here's another attempt:
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2019-01-25 Marek Polacek
PR c++/89024 - ICE with incomplete enum type.
* call.c (standard_conversion): When converting an
ARITHMETIC_TYPE_P to an incomplete type, return NULL.
* g++.dg/cpp0x/enum37.C: New test.
diff --git gcc/cp/call.c gcc/cp/call.c
index 515a9420032..c74d1b4ebdf 100644
--- gcc/cp/call.c
+++ gcc/cp/call.c
@@ -1412,6 +1412,13 @@ standard_conversion (tree to, tree from, tree expr, bool
c_cast_p,
|| (fcode == REAL_TYPE && !(flags & LOOKUP_NO_NON_INTEGRAL)))
|| SCOPED_ENUM_P (from))
return NULL;
+
+ /* If we're parsing an enum with no fixed underlying type, we're
+dealing with an incomplete type, which renders the conversion
+ill-formed. */
+ if (!COMPLETE_TYPE_P (from))
+ return NULL;
+
conv = build_conv (ck_std, to, conv);
/* Give this a better rank if it's a promotion. */
diff --git gcc/testsuite/g++.dg/cpp0x/enum37.C
gcc/testsuite/g++.dg/cpp0x/enum37.C
new file mode 100644
index 000..6aa3d4015d7
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp0x/enum37.C
@@ -0,0 +1,24 @@
+// PR c++/89024
+// { dg-do compile { target c++11 } }
+
+template struct same;
+template struct same {};
+
+template T&& declval();
+
+template
+void __test_aux(_To1);
+
+template(declval<_From1>()))>
+char __test(int);
+
+template
+int __test(...);
+
+enum E {
+x = decltype(__test(0))(0)
+};
+
+same s;
+same s2;
Re: [Patch] PR rtl-optimization/87763 - generate more bfi instructions on aarch64
On Thu, Jan 24, 2019 at 11:17:45PM +, Steve Ellcey wrote:
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -9294,6 +9294,44 @@ aarch64_mask_and_shift_for_ubfiz_p (scalar_int_mode
> mode, rtx mask,
>& ((HOST_WIDE_INT_1U << INTVAL (shft_amnt)) - 1)) == 0;
> }
>
> +/* Return true if the masks and a shift amount from an RTX of the form
> + ((x & MASK1) | ((y << SHIFT_AMNT) & MASK2)) are valid to combine into
> + a BFI instruction of mode MODE. See *arch64_bfi patterns. */
> +
> +bool
> +aarch64_masks_and_shift_for_aarch64_bfi_p (scalar_int_mode mode, rtx mask1,
> +rtx shft_amnt, rtx mask2)
> +{
> + unsigned HOST_WIDE_INT m1, m2, s, t;
> +
> + if (!CONST_INT_P (mask1) || !CONST_INT_P (mask2) || !CONST_INT_P
> (shft_amnt))
> +return false;
> +
> + m1 = UINTVAL (mask1);
> + m2 = UINTVAL (mask2);
> + s = UINTVAL (shft_amnt);
> +
> + /* Verify that there is no overlap in what bits are set in the two masks.
> */
> + if ((m1 + m2 + 1) != 0)
> +return false;
Wouldn't that be clearer to test
if (m1 + m2 != HOST_WIDE_INT_1U)
return false;
?
You IMHO also should test
if ((m1 & m2) != 0)
return false;
> +
> + /* Verify that the shift amount is less than the mode size. */
> + if (s >= GET_MODE_BITSIZE (mode))
> +return false;
> +
> + /* Verify that the mask being shifted is contigious and would be in the
> + least significant bits after shifting by creating a mask 't' based on
> + the number of bits set in mask2 and the shift amount for mask2 and
> + comparing that to the actual mask2. */
> + t = popcount_hwi (m2);
> + t = (1 << t) - 1;
This should be (HOST_WIDE_INT_1U << t), otherwise if popcount of m2 is
>= 32, there will be UB.
> + t = t << s;
> + if (t != m2)
> +return false;
> +
> + return true;
> +}
> +
> +;; Match a bfi instruction where the shift of OP3 means that we are
> +;; actually copying the least significant bits of OP3 into OP0 by way
> +;; of the AND masks and the IOR instruction.
> +
> +(define_insn "*aarch64_bfi4_shift"
> + [(set (match_operand:GPI 0 "register_operand" "=r")
> +(ior:GPI (and:GPI (match_operand:GPI 1 "register_operand" "0")
> + (match_operand:GPI 2 "const_int_operand" "n"))
> + (and:GPI (ashift:GPI
> + (match_operand:GPI 3 "register_operand" "r")
> + (match_operand:GPI 4
> "aarch64_simd_shift_imm_" "n"))
> + (match_operand:GPI 5 "const_int_operand" "n"]
> + "aarch64_masks_and_shift_for_aarch64_bfi_p (mode, operands[2],
> operands[4], operands[5])"
Too long lines.
> +{
> + return "bfi\t%0, %3, %4, %P5";
> +}
> + [(set_attr "type" "bfm")]
> +)
As mentioned in rs6000.md, I believe you also need a similar pattern where
the two ANDs are swapped, because they have the same priority.
> +
> +;; Like the above instruction but with no shifting, we are just copying the
> +;; least significant bits of OP3 to OP0.
> +
> +(define_insn "*aarch64_bfi4_noshift"
> + [(set (match_operand:GPI 0 "register_operand" "=r")
> +(ior:GPI (and:GPI (match_operand:GPI 1 "register_operand" "0")
> + (match_operand:GPI 2 "const_int_operand" "n"))
> + (and:GPI (match_operand:GPI 3 "register_operand" "r")
> + (match_operand:GPI 4 "const_int_operand" "n"]
> + "aarch64_masks_and_shift_for_aarch64_bfi_p (mode, operands[2],
> const0_rtx, operands[4])"
Too long line.
I guess this one has similar issue that you might need two patterns for both
AND orderings (though the "0" needs to be on the right argument in each
case).
Jakub
patch to fix PR88846
The following patch fixes
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88846
The patch was successfully bootstrapped and tested on x86-64 and ppc64.
Committed as rev. 268280
Index: ChangeLog
===
--- ChangeLog (revision 268279)
+++ ChangeLog (working copy)
@@ -1,3 +1,11 @@
+2019-01-25 Vladimir Makarov
+
+ PR rtl-optimization/46
+ * ira.c (process_set_for_memref_referenced_p): New.
+ (memref_referenced_p): Add new param. Use
+ process_set_for_memref_referenced_p. Add new switch cases.
+ (memref_used_between_p): Pass new arg to memref_referenced_p.
+
2019-01-25 Richard Earnshaw
PR target/88469
Index: ira.c
===
--- ira.c (revision 268279)
+++ ira.c (working copy)
@@ -3140,10 +3140,30 @@ equiv_init_movable_p (rtx x, int regno)
return 1;
}
-/* TRUE if X references a memory location that would be affected by a store
- to MEMREF. */
-static int
-memref_referenced_p (rtx memref, rtx x)
+static bool memref_referenced_p (rtx memref, rtx x, bool read_p);
+
+/* Auxiliary function for memref_referenced_p. Process setting X for
+ MEMREF store. */
+static bool
+process_set_for_memref_referenced_p (rtx memref, rtx x)
+{
+ /* If we are setting a MEM, it doesn't count (its address does), but any
+ other SET_DEST that has a MEM in it is referencing the MEM. */
+ if (MEM_P (x))
+{
+ if (memref_referenced_p (memref, XEXP (x, 0), true))
+ return true;
+}
+ else if (memref_referenced_p (memref, x, false))
+return true;
+
+ return false;
+}
+
+/* TRUE if X references a memory location (as a read if READ_P) that
+ would be affected by a store to MEMREF. */
+static bool
+memref_referenced_p (rtx memref, rtx x, bool read_p)
{
int i, j;
const char *fmt;
@@ -3159,30 +3179,51 @@ memref_referenced_p (rtx memref, rtx x)
case CC0:
case HIGH:
case LO_SUM:
- return 0;
+ return false;
case REG:
return (reg_equiv[REGNO (x)].replacement
&& memref_referenced_p (memref,
- reg_equiv[REGNO (x)].replacement));
+ reg_equiv[REGNO (x)].replacement, read_p));
case MEM:
- if (true_dependence (memref, VOIDmode, x))
- return 1;
+ /* Memory X might have another effective type than MEMREF. */
+ if (read_p || true_dependence (memref, VOIDmode, x))
+ return true;
break;
case SET:
- /* If we are setting a MEM, it doesn't count (its address does), but any
- other SET_DEST that has a MEM in it is referencing the MEM. */
- if (MEM_P (SET_DEST (x)))
- {
- if (memref_referenced_p (memref, XEXP (SET_DEST (x), 0)))
- return 1;
- }
- else if (memref_referenced_p (memref, SET_DEST (x)))
- return 1;
+ if (process_set_for_memref_referenced_p (memref, SET_DEST (x)))
+ return true;
+
+ return memref_referenced_p (memref, SET_SRC (x), true);
+
+case CLOBBER:
+case CLOBBER_HIGH:
+ if (process_set_for_memref_referenced_p (memref, XEXP (x, 0)))
+ return true;
+
+ return false;
+
+case PRE_DEC:
+case POST_DEC:
+case PRE_INC:
+case POST_INC:
+ if (process_set_for_memref_referenced_p (memref, XEXP (x, 0)))
+ return true;
+
+ return memref_referenced_p (memref, XEXP (x, 0), true);
+
+case POST_MODIFY:
+case PRE_MODIFY:
+ /* op0 = op0 + op1 */
+ if (process_set_for_memref_referenced_p (memref, XEXP (x, 0)))
+ return true;
+
+ if (memref_referenced_p (memref, XEXP (x, 0), true))
+ return true;
- return memref_referenced_p (memref, SET_SRC (x));
+ return memref_referenced_p (memref, XEXP (x, 1), true);
default:
break;
@@ -3193,17 +3234,17 @@ memref_referenced_p (rtx memref, rtx x)
switch (fmt[i])
{
case 'e':
- if (memref_referenced_p (memref, XEXP (x, i)))
- return 1;
+ if (memref_referenced_p (memref, XEXP (x, i), read_p))
+ return true;
break;
case 'E':
for (j = XVECLEN (x, i) - 1; j >= 0; j--)
- if (memref_referenced_p (memref, XVECEXP (x, i, j)))
- return 1;
+ if (memref_referenced_p (memref, XVECEXP (x, i, j), read_p))
+ return true;
break;
}
- return 0;
+ return false;
}
/* TRUE if some insn in the range (START, END] references a memory location
@@ -3224,7 +3265,7 @@ memref_used_between_p (rtx memref, rtx_i
if (!NONDEBUG_INSN_P (insn))
continue;
- if (memref_referenced_p (memref, PATTERN (insn)))
+ if (memref_referenced_p (memref, PATTERN (insn), false))
return 1;
/* Nonconst functions may access memory. */
Re: [Patch] PR rtl-optimization/87763 - generate more bfi instructions on aarch64
On Fri, 2019-01-25 at 10:32 +, Richard Earnshaw (lists) wrote: > > Do we need another variant pattern to handle the case where the > insertion is into the top of the destination? In that case the > immediate mask on the shifted operand is technically redundant as the > bottom bits are known zero and there are no top bits. I am not sure about this. Do you have an example where this might be needed? I updated my patch to address your other comments and have bootstrapped and tested this on aarch64. Does this version look good for checkin? I had to modify the two tests because with my new instructions we sometimes generate bfi instructions where we used to generate bfxil instructions. The only regression this is fixing is combine_bfi_1.c, combine_bfxil.c was passing before but still needed to be changed in order to keep passing. Steve Ellcey sell...@marvell.com 2018-01-25 Steve Ellcey PR rtl-optimization/87763 * config/aarch64/aarch64-protos.h (aarch64_masks_and_shift_for_bfi_p): New prototype. * config/aarch64/aarch64.c (aarch64_masks_and_shift_for_bfi_p): New function. * config/aarch64/aarch64.md (*aarch64_bfi4_shift): New instruction. (*aarch64_bfi4_noshift): Ditto. 2018-01-25 Steve Ellcey PR rtl-optimization/87763 * gcc.target/aarch64/combine_bfi_1.c: Change some bfxil checks to bfi. * gcc.target/aarch64/combine_bfxil.c: Ditto.
Re: C++ PATCH for c++/78244 - narrowing conversion in template not detected, part 2
On Fri, Jan 25, 2019 at 10:05:00AM -0500, Jason Merrill wrote:
> On 1/24/19 7:17 PM, Marek Polacek wrote:
> > On Wed, Jan 23, 2019 at 03:34:04PM -0500, Jason Merrill wrote:
> > > On Wed, Jan 23, 2019 at 12:57 PM Marek Polacek wrote:
> > > >
> > > > On Wed, Jan 23, 2019 at 09:00:36AM -0500, Jason Merrill wrote:
> > > > > I was talking about digest_init, not reshape_init. digest_init calls
> > > > > convert_for_initialization.
> > > >
> > > > /facepalm
> > > >
> > > > So yes, digest_init calls convert_for_initialization which will end up
> > > > calling perform_implicit_conversion_flags which could call
> > > > convert_like_real
> > > > where the narrowing warnings are given, but it doesn't, we go to this
> > > > case:
> > > >
> > > >else if (processing_template_decl && conv->kind != ck_identity)
> > > > {
> > > >/* In a template, we are only concerned about determining the
> > > > type of non-dependent expressions, so we do not have to
> > > > perform the actual conversion. But for initializers, we
> > > > need to be able to perform it at instantiation
> > > > (or instantiate_non_dependent_expr) time. */
> > > >expr = build1 (IMPLICIT_CONV_EXPR, type, expr);
> > > >
> > > > finish_decltype_type throws away the expression because it's not
> > > > dependent, and
> > > > only uses its type. So narrowing remains undetected. Not sure if I
> > > > should mess
> > > > with perform_implicit_conversion_flags.
> > >
> > > Let's try that; this is a situation where the comment is incorrect.
> > > Perhaps just call check_narrowing here if appropriate, rather than go
> > > through the whole conversion machinery.
> >
> > I have not been successful.
> >
> > First, I modified perform_implicit_conversion_flags to go the convert_like
> > route when dealing with something non-dependent. That breaks e.g. in
> > build_value_init:
> > 346 /* The AGGR_INIT_EXPR tweaking below breaks in templates. */
> > 347 gcc_assert (!processing_template_decl
> > 348 || (SCALAR_TYPE_P (type) || TREE_CODE (type) ==
> > ARRAY_TYPE));
> > Even if I restrict the convert_like way for non-dependent exprs in a
> > template
> > for scalars, it still breaks elsewhere, e.g. constexpr-template3.C where it
> > complains about taking the address of an rvalue.
> >
> > Second, I added check_narrowing to the processing_template_decl case in
> > perform_implicit_conversion_flags. That works except it breaks
> > constexpr-inst1.C -- we no longer get the error. That's because currently
> > check_narrowing in finish_compound_literal calls maybe_constant_init, which
> > calls instantiate_constexpr_fns and we get the desired diagnostic. But if
> > I move check_narrowing to perform_implicit_conversion_flags, we no longer
> > call it in this case -- processing_template_decl is 0 so we call
> > convert_like
> > but that doesn't do the trick.
> >
> > So, back to the patch that leaves check_narrowing in
> > finish_compound_literal?
>
> That patch still needs a test for the aggregate case.
Ok, this is a version with Wnarrowing16.C added.
...but we still don't warn for the TYPE_NON_AGGREGATE_CLASS case in
finish_compound_literal, so the nightmare continues.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2019-01-25 Marek Polacek
PR c++/88815 - narrowing conversion lost in decltype.
PR c++/78244 - narrowing conversion in template not detected.
* cp-tree.h (CONSTRUCTOR_IS_DEPENDENT): New.
* pt.c (instantiation_dependent_r): Consider a CONSTRUCTOR with
CONSTRUCTOR_IS_DEPENDENT instantiation-dependent.
* semantics.c (finish_compound_literal): When the compound literal
isn't instantiation-dependent and the type isn't type-dependent,
fall back to the normal processing. Don't only call check_narrowing
for scalar types. Set CONSTRUCTOR_IS_DEPENDENT.
* g++.dg/cpp0x/Wnarrowing15.C: New test.
* g++.dg/cpp0x/Wnarrowing16.C: New test.
* g++.dg/cpp0x/constexpr-decltype3.C: New test.
* g++.dg/cpp1y/Wnarrowing1.C: New test.
diff --git gcc/cp/cp-tree.h gcc/cp/cp-tree.h
index cd902ce1cf6..77e1425b435 100644
--- gcc/cp/cp-tree.h
+++ gcc/cp/cp-tree.h
@@ -424,6 +424,7 @@ extern GTY(()) tree cp_global_trees[CPTI_MAX];
DECL_FINAL_P (in FUNCTION_DECL)
QUALIFIED_NAME_IS_TEMPLATE (in SCOPE_REF)
DECLTYPE_FOR_INIT_CAPTURE (in DECLTYPE_TYPE)
+ CONSTRUCTOR_IS_DEPENDENT (in CONSTRUCTOR)
TINFO_USED_TEMPLATE_ID (in TEMPLATE_INFO)
PACK_EXPANSION_SIZEOF_P (in *_PACK_EXPANSION)
OVL_USING_P (in OVERLOAD)
@@ -4205,6 +4206,11 @@ more_aggr_init_expr_args_p (const
aggr_init_expr_arg_iterator *iter)
B b{1,2}, not B b({1,2}) or B b = {1,2}. */
#define CONSTRUCTOR_IS_DIRECT_INIT(NODE) (TREE_LANG_FLAG_0 (CONSTRUCTOR_CHECK
(NODE)))
+/* True if this CONSTRUCTOR is instantiation-dependent and needs to be
+ substituted. */
[PATCH v2] C++ concepts: fix ICE with requires on dtors (PR c++/89036)
On Fri, 2019-01-25 at 08:59 -0800, Nathan Sidwell wrote:
> On 1/25/19 8:48 AM, David Malcolm wrote:
> > PR c++/89036 reports an ICE due to this assertion failing
> >
> > 1136 /* A class should never have more than one
> > destructor. */
> > 1137 gcc_assert (!current_fns || via_using ||
> > !DECL_DESTRUCTOR_P (method));
> >
> > on this template with a pair of dtors, with
> > mutually exclusive "requires" clauses:
> >
> > template
> > struct Y {
> > ~Y() requires(true) = default;
> > ~Y() requires(false) {}
> > };
> >
> > (is this valid? my knowledge of this part of C++ is fairly hazy)
>
> Yes. A more sensible example would have 'true' and 'false' replaced
> by
> something determined from the template parms.
>
> >
> > Nathan introduced this assertion as part of:
> >
> >ca9219bf18c68a001d62ecb981bc9176b0feaf12 (aka r251340):
> > 2017-08-24 Nathan Sidwell
> > Conversion operators kept on single overload set
>
> I'd just drop the assert at this point.
>
> nathan
Thanks. Here's a version of the patch which drops the assertion.
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
OK for trunk?
gcc/cp/ChangeLog:
PR c++/89036
* class.c (add_method): Drop destructor assertion.
gcc/testsuite/ChangeLog:
PR c++/89036
* g++.dg/concepts/pr89036.C: New test.
---
gcc/cp/class.c | 3 ---
gcc/testsuite/g++.dg/concepts/pr89036.C | 8
2 files changed, 8 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/concepts/pr89036.C
diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index e8773c2..6e9dac9 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -1133,9 +1133,6 @@ add_method (tree type, tree method, bool via_using)
}
}
- /* A class should never have more than one destructor. */
- gcc_assert (!current_fns || via_using || !DECL_DESTRUCTOR_P (method));
-
current_fns = ovl_insert (method, current_fns, via_using);
if (!COMPLETE_TYPE_P (type) && !DECL_CONV_FN_P (method)
diff --git a/gcc/testsuite/g++.dg/concepts/pr89036.C
b/gcc/testsuite/g++.dg/concepts/pr89036.C
new file mode 100644
index 000..f83ef8b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/pr89036.C
@@ -0,0 +1,8 @@
+// { dg-do compile { target c++11 } }
+// { dg-options "-fconcepts" }
+
+template
+struct Y {
+ ~Y() requires(true) = default;
+ ~Y() requires(false) {}
+};
--
1.8.5.3
Re: [PATCH] Revert a hunk from r261322 (PR lto/88876).
> Hi.
>
> The patch puts back ::get_create for a node that can be seen first time.
> It's due to -O0 optimize attribute. It was unable to write properly
> LTO test-case for it.
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ready to be installed?
> Thanks,
> Martin
>
> gcc/ChangeLog:
>
> 2019-01-18 Martin Liska
>
> PR lto/88876
> * ipa-pure-const.c (propagate_pure_const): Revert hunk as
> we need default values of funct_state for a function that
> is not optimized.
I think you want to test if y enables pure_const prior calling get
becuase get_create will just create empty info that will likely have
IPA_CONST (which is 0) and not IPA_NEITHER. There are multiple copies
of this code in other propagators and I think they all needs to check if
the pass is enabled for particular symbol and if not resort to
propagating nothing.
Honza
> ---
> gcc/ipa-pure-const.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>
> diff --git a/gcc/ipa-pure-const.c b/gcc/ipa-pure-const.c
> index 8227eed29bc..b8fd08c0a7e 100644
> --- a/gcc/ipa-pure-const.c
> +++ b/gcc/ipa-pure-const.c
> @@ -1498,7 +1498,8 @@ propagate_pure_const (void)
> }
> if (avail > AVAIL_INTERPOSABLE)
> {
> - funct_state y_l = funct_state_summaries->get (y);
> + funct_state y_l = funct_state_summaries->get_create (y);
> +
> if (dump_file && (dump_flags & TDF_DETAILS))
> {
> fprintf (dump_file,
>
Re: [PATCH] Revert yet another ::get_create for IPA summary (PR ipa/88958).
> Hi.
>
> This is one very similar patch.
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> Ready to be installed?
> Thanks,
> Martin
> From adf577edd5a1d2b6ed78c1cc18feaff23fbfdd1c Mon Sep 17 00:00:00 2001
> From: marxin
> Date: Thu, 24 Jan 2019 16:07:29 +0100
> Subject: [PATCH] Revert yet another ::get_create for IPA summary (PR
> ipa/88958).
>
> gcc/ChangeLog:
>
> 2019-01-24 Martin Liska
>
> PR ipa/88958
> * ipa-fnsummary.c (estimate_edge_devirt_benefit): Use
> ::get_create as the symbol can be not seen yet.
Using get_create will create empty fnsummary which is invalid and we do
not want to use it anyway, so turning get into get_creates is not right
fix in general. Here you just want to return false if isummary is NULL.
Honza
>
> gcc/testsuite/ChangeLog:
>
> 2019-01-24 Martin Liska
>
> PR ipa/88958
> * gcc.dg/ipa/pr88985.c: New test.
> ---
> gcc/ipa-fnsummary.c| 2 +-
> gcc/testsuite/gcc.dg/ipa/pr88985.c | 13 +
> 2 files changed, 14 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/gcc.dg/ipa/pr88985.c
>
> diff --git a/gcc/ipa-fnsummary.c b/gcc/ipa-fnsummary.c
> index 535b3f22d49..af741f04eb2 100644
> --- a/gcc/ipa-fnsummary.c
> +++ b/gcc/ipa-fnsummary.c
> @@ -2581,7 +2581,7 @@ estimate_edge_devirt_benefit (struct cgraph_edge *ie,
>callee = callee->function_symbol ();
>if (avail < AVAIL_AVAILABLE)
> return false;
> - isummary = ipa_fn_summaries->get (callee);
> + isummary = ipa_fn_summaries->get_create (callee);
>return isummary->inlinable;
> }
>
> diff --git a/gcc/testsuite/gcc.dg/ipa/pr88985.c
> b/gcc/testsuite/gcc.dg/ipa/pr88985.c
> new file mode 100644
> index 000..8253a893cf6
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/ipa/pr88985.c
> @@ -0,0 +1,13 @@
> +void f (void)
> +{
> +}
> +__attribute__((__optimize__("O2")))
> +void g (void f())
> +{
> + f();
> +}
> +__attribute__((__optimize__("O2")))
> +void h (void)
> +{
> + g(f);
> +}
> --
> 2.20.1
>
Re: [PR ipa/88933] Careful CFG cleanup in IPA-CP function transformation
Dne 2019-01-25 19:10, Martin Jambor napsal:
Hi,
the following patch fixes a verification ICE because of mismatching BB
and cgraph_edge counts arising as a consequence of cleaning-up CFG
after
IPA-CP transformation, which is currently done as if it was a normal
tree pass, and which IPA passes should not attempt exaclty because of
this reason.
Fixed (hopefully) exactly as I was told by Honza in Bugzilla, by
resorting to calling delete_unreachable_blocks_update_callgraph
instead. The aforementioned function had to be made public and in the
process was moved to a more suitable file.
Bootstrapped and tested on x86_64-linux. OK for trunk?
OK,
Honza
Thanks,
Martin
2019-01-25 Martin Jambor
* tree-inline.c: Include tree-cfgcleanup.h.
(delete_unreachable_blocks_update_callgraph): Move...
* tree-cfgcleanup.c (delete_unreachable_blocks_update_callgraph):
...here, make externally visible, make second argument bool, adjust
all callers.
* tree-cfgcleanup.c: Include cgraph.h.
* tree-cfgcleanup.h (delete_unreachable_blocks_update_callgraph):
Declare.
* ipa-prop.c: Include tree-cfgcleanup.h.
(ipcp_transform_function): Call
delete_unreachable_blocks_update_callgraph instead of cleaning uo CFG.
testsuite/
* gfortran.dg/gomp/pr88933.f90: New test.
---
gcc/ipa-prop.c | 10 +--
gcc/testsuite/gfortran.dg/gomp/pr88933.f90 | 39 +++
gcc/tree-cfgcleanup.c | 75 +++-
gcc/tree-cfgcleanup.h | 2 +
gcc/tree-inline.c | 80 ++
5 files changed, 125 insertions(+), 81 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/gomp/pr88933.f90
diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index 40ab130b750..d86c2f3db55 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -52,6 +52,7 @@ along with GCC; see the file COPYING3. If not see
#include "dbgcnt.h"
#include "domwalk.h"
#include "builtins.h"
+#include "tree-cfgcleanup.h"
/* Function summary where the parameter infos are actually stored. */
ipa_node_params_t *ipa_node_params_sum = NULL;
@@ -5173,10 +5174,11 @@ ipcp_transform_function (struct cgraph_node
*node)
if (!something_changed)
return 0;
- else if (cfg_changed)
-return TODO_update_ssa_only_virtuals | TODO_cleanup_cfg;
- else
-return TODO_update_ssa_only_virtuals;
+
+ if (cfg_changed)
+delete_unreachable_blocks_update_callgraph (node, false);
+
+ return TODO_update_ssa_only_virtuals;
}
#include "gt-ipa-prop.h"
diff --git a/gcc/testsuite/gfortran.dg/gomp/pr88933.f90
b/gcc/testsuite/gfortran.dg/gomp/pr88933.f90
new file mode 100644
index 000..e4f30ae9f3e
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/gomp/pr88933.f90
@@ -0,0 +1,39 @@
+! PR ipa/88933
+! { dg-do compile }
+! { dg-options "-O1 -fexceptions -fipa-cp -fnon-call-exceptions
-fopenmp -fno-inline-functions-called-once" }
+
+!$omp parallel
+!$omp single
+ call a
+!$omp end single
+!$omp end parallel
+contains
+ subroutine b (c, d, e, f, g, h, i, j, k, m)
+character (*) c
+character d
+integer, dimension (m) :: e
+integer, dimension (m) :: f
+character g
+character h
+real, dimension (:, :, :) :: i
+double precision, dimension (:, :, :) :: j
+integer, dimension (:, :, :) :: k
+
+integer, dimension (m) :: l
+!$omp task firstprivate (k) firstprivate (l)
+!$omp end task
+ c = ''
+ end
+ subroutine a
+character c
+character d
+integer, dimension (7) :: e
+integer, dimension (7) :: f
+character g
+character h
+real, dimension (5, 6, 7) :: i
+double precision, dimension (6, 6, 7) :: j
+integer, dimension (5, 7, 6) :: k
+call b (c, d, e, f, g, h, i, j, k, 7)
+ end
+end
diff --git a/gcc/tree-cfgcleanup.c b/gcc/tree-cfgcleanup.c
index 2adb3953d6b..f2e8b96ee8e 100644
--- a/gcc/tree-cfgcleanup.c
+++ b/gcc/tree-cfgcleanup.c
@@ -43,7 +43,7 @@ along with GCC; see the file COPYING3. If not see
#include "gimple-match.h"
#include "gimple-fold.h"
#include "tree-ssa-loop-niter.h"
-
+#include "cgraph.h"
/* The set of blocks in that at least one of the following changes
happened:
-- the statement at the end of the block was changed
@@ -1380,3 +1380,76 @@ make_pass_cleanup_cfg_post_optimizing
(gcc::context *ctxt)
}
+/* Delete all unreachable basic blocks and update callgraph.
+ Doing so is somewhat nontrivial because we need to update all
clones and
+ remove inline function that become unreachable. */
+
+bool
+delete_unreachable_blocks_update_callgraph (cgraph_node *dst_node,
+ bool update_clones)
+{
+ bool changed = false;
+ basic_block b, next_bb;
+
+ find_unreachable_blocks ();
+
+ /* Delete all unreachable basic blocks. */
+
+ for (b = ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb; b
+ != EXIT_BLOCK_PTR_FOR_FN (cfun); b = next_bb)
[PR ipa/88933] Careful CFG cleanup in IPA-CP function transformation
Hi,
the following patch fixes a verification ICE because of mismatching BB
and cgraph_edge counts arising as a consequence of cleaning-up CFG after
IPA-CP transformation, which is currently done as if it was a normal
tree pass, and which IPA passes should not attempt exaclty because of
this reason.
Fixed (hopefully) exactly as I was told by Honza in Bugzilla, by
resorting to calling delete_unreachable_blocks_update_callgraph
instead. The aforementioned function had to be made public and in the
process was moved to a more suitable file.
Bootstrapped and tested on x86_64-linux. OK for trunk?
Thanks,
Martin
2019-01-25 Martin Jambor
* tree-inline.c: Include tree-cfgcleanup.h.
(delete_unreachable_blocks_update_callgraph): Move...
* tree-cfgcleanup.c (delete_unreachable_blocks_update_callgraph):
...here, make externally visible, make second argument bool, adjust
all callers.
* tree-cfgcleanup.c: Include cgraph.h.
* tree-cfgcleanup.h (delete_unreachable_blocks_update_callgraph):
Declare.
* ipa-prop.c: Include tree-cfgcleanup.h.
(ipcp_transform_function): Call
delete_unreachable_blocks_update_callgraph instead of cleaning uo CFG.
testsuite/
* gfortran.dg/gomp/pr88933.f90: New test.
---
gcc/ipa-prop.c | 10 +--
gcc/testsuite/gfortran.dg/gomp/pr88933.f90 | 39 +++
gcc/tree-cfgcleanup.c | 75 +++-
gcc/tree-cfgcleanup.h | 2 +
gcc/tree-inline.c | 80 ++
5 files changed, 125 insertions(+), 81 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/gomp/pr88933.f90
diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index 40ab130b750..d86c2f3db55 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -52,6 +52,7 @@ along with GCC; see the file COPYING3. If not see
#include "dbgcnt.h"
#include "domwalk.h"
#include "builtins.h"
+#include "tree-cfgcleanup.h"
/* Function summary where the parameter infos are actually stored. */
ipa_node_params_t *ipa_node_params_sum = NULL;
@@ -5173,10 +5174,11 @@ ipcp_transform_function (struct cgraph_node *node)
if (!something_changed)
return 0;
- else if (cfg_changed)
-return TODO_update_ssa_only_virtuals | TODO_cleanup_cfg;
- else
-return TODO_update_ssa_only_virtuals;
+
+ if (cfg_changed)
+delete_unreachable_blocks_update_callgraph (node, false);
+
+ return TODO_update_ssa_only_virtuals;
}
#include "gt-ipa-prop.h"
diff --git a/gcc/testsuite/gfortran.dg/gomp/pr88933.f90
b/gcc/testsuite/gfortran.dg/gomp/pr88933.f90
new file mode 100644
index 000..e4f30ae9f3e
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/gomp/pr88933.f90
@@ -0,0 +1,39 @@
+! PR ipa/88933
+! { dg-do compile }
+! { dg-options "-O1 -fexceptions -fipa-cp -fnon-call-exceptions -fopenmp
-fno-inline-functions-called-once" }
+
+!$omp parallel
+!$omp single
+ call a
+!$omp end single
+!$omp end parallel
+contains
+ subroutine b (c, d, e, f, g, h, i, j, k, m)
+character (*) c
+character d
+integer, dimension (m) :: e
+integer, dimension (m) :: f
+character g
+character h
+real, dimension (:, :, :) :: i
+double precision, dimension (:, :, :) :: j
+integer, dimension (:, :, :) :: k
+
+integer, dimension (m) :: l
+!$omp task firstprivate (k) firstprivate (l)
+!$omp end task
+ c = ''
+ end
+ subroutine a
+character c
+character d
+integer, dimension (7) :: e
+integer, dimension (7) :: f
+character g
+character h
+real, dimension (5, 6, 7) :: i
+double precision, dimension (6, 6, 7) :: j
+integer, dimension (5, 7, 6) :: k
+call b (c, d, e, f, g, h, i, j, k, 7)
+ end
+end
diff --git a/gcc/tree-cfgcleanup.c b/gcc/tree-cfgcleanup.c
index 2adb3953d6b..f2e8b96ee8e 100644
--- a/gcc/tree-cfgcleanup.c
+++ b/gcc/tree-cfgcleanup.c
@@ -43,7 +43,7 @@ along with GCC; see the file COPYING3. If not see
#include "gimple-match.h"
#include "gimple-fold.h"
#include "tree-ssa-loop-niter.h"
-
+#include "cgraph.h"
/* The set of blocks in that at least one of the following changes happened:
-- the statement at the end of the block was changed
@@ -1380,3 +1380,76 @@ make_pass_cleanup_cfg_post_optimizing (gcc::context
*ctxt)
}
+/* Delete all unreachable basic blocks and update callgraph.
+ Doing so is somewhat nontrivial because we need to update all clones and
+ remove inline function that become unreachable. */
+
+bool
+delete_unreachable_blocks_update_callgraph (cgraph_node *dst_node,
+ bool update_clones)
+{
+ bool changed = false;
+ basic_block b, next_bb;
+
+ find_unreachable_blocks ();
+
+ /* Delete all unreachable basic blocks. */
+
+ for (b = ENTRY_BLOCK_PTR_FOR_FN (cfun)->next_bb; b
+ != EXIT_BLOCK_PTR_FOR_FN (cfun); b = next_bb)
+{
+ next_bb = b->next_bb;
+
+ if
Re: [PATCH] rs6000: Add support for the vec_sbox_be, vec_cipher_be etc. builtins.
Hi!
On Wed, Jan 23, 2019 at 03:57:28AM -0600, luo...@linux.vnet.ibm.com wrote:
> The 5 new builtins vec_sbox_be, vec_cipher_be, vec_cipherlast_be,
> vec_ncipher_be
> and vec_ncipherlast_be only support vector unsigned char type parameters.
> Add new instruction crypto_vsbox_ and crypto__ to handle
> them accordingly, where the new mode CR_vqdi can be expanded to vector
> unsigned
> long long for none _be postfix builtins or vector unsigned char for _be
> postfix
> builtins.
Hrm, can't you use the existing CR_mode iterator here?
> 2019-01-23 Xiong Hu Luo
>
> * gcc/testsuite/gcc.target/powerpc/crypto-builtin-1.c
> (crpyto1_be, crpyto2_be, crpyto3_be, crpyto4_be, crpyto5_be):
> New testcases.
Typoes ("crypto"). And that last line is indented incorrectly.
With those things fixed, okay for trunk, with the new iterator if CR_mode
isn't usable here. Thanks!
Segher
[PATCH] Add myself to MAINTAINERS
This adds me to the Write After Approval list in MAINTAINERS. Committed to trunk in r268276. Index: MAINTAINERS === --- MAINTAINERS (revision 268275) +++ MAINTAINERS (working copy) @@ -634,6 +634,7 @@ Canqun Yang Fei Yang Jeffrey Yasskin Joey Ye +Kwok Cheung Yeung Greta Yorsh David Yuste Adhemerval Zanella
Re: [backtrace] Avoid segfault
On Fri, Jan 25, 2019 at 12:15:28PM -0500, Nathan Sidwell wrote: > On 1/25/19 5:28 AM, Tom de Vries wrote: > > > > This patch fixes it by passing "" instead of NULL, in the call to > > elf_add at line 3083 (for .gnu_debugaltlink), not the call to elf_add at > > line 3044 (for .gnu_debuglink) mentioned above. > > > > Nathan, does this fix the problem for you? If not, can you provide a > > reproducer, or give a hint on how one could be constructed? > > I still hit the problem, and am installing this as sufficiently obvious. > I'm on a fedora system debugging pr88995. The debuglink_name is > "../../.dwz/isl-0.16.1-7.fc29.x86_64" > > I'm not sure why this is triggering now -- maybe my debuginfo packages are > out of date? I'm seeing this too. I've resolved it by uninstalling libmpc-debuginfo. If I put it back, the crash in libbacktrace reappears. Again the problem was a null filename and so strchr crashed. Thanks for looking into this Nathan. Marek
Re: [backtrace] Avoid segfault
On 1/25/19 5:28 AM, Tom de Vries wrote: This patch fixes it by passing "" instead of NULL, in the call to elf_add at line 3083 (for .gnu_debugaltlink), not the call to elf_add at line 3044 (for .gnu_debuglink) mentioned above. Nathan, does this fix the problem for you? If not, can you provide a reproducer, or give a hint on how one could be constructed? I still hit the problem, and am installing this as sufficiently obvious. I'm on a fedora system debugging pr88995. The debuglink_name is "../../.dwz/isl-0.16.1-7.fc29.x86_64" I'm not sure why this is triggering now -- maybe my debuginfo packages are out of date? nathan -- Nathan Sidwell 2019-01-25 Nathan Sidwell * elf.c (elf_add): Pass "" filename to recursive call with separated debug. Index: elf.c === --- elf.c (revision 268272) +++ elf.c (working copy) @@ -3041,7 +3041,7 @@ elf_add (struct backtrace_state *state, if (debugaltlink_view_valid) backtrace_release_view (state, _view, error_callback, data); - ret = elf_add (state, NULL, d, base_address, error_callback, data, + ret = elf_add (state, "", d, base_address, error_callback, data, fileline_fn, found_sym, found_dwarf, NULL, 0, 1, NULL, 0); if (ret < 0)
Re: C++ PATCH for c++/89024 - ICE with incomplete enum type
On 1/25/19 12:09 PM, Marek Polacek wrote: On Fri, Jan 25, 2019 at 10:55:55AM -0600, Tim Song wrote: On Thu, Jan 24, 2019 at 4:14 PM Jason Merrill wrote: On 1/24/19 2:16 PM, Marek Polacek wrote: This test ICEs since r159006 which added type = ENUM_UNDERLYING_TYPE (type); to type_promotes_to. In this test ENUM_UNDERLYING_TYPE is null because we haven't yet parsed '}' of the enum and the underlying type isn't fixed, and so checking TYPE_UNSIGNED crashed. I've added some checks to the test to see if the types seem to be OK; clang++ agrees. Bootstrapped/regtested on x86_64-linux, ok for trunk/8/7? 2019-01-24 Marek Polacek PR c++/89024 - ICE with incomplete enum type. * cvt.c (type_promotes_to): Check if prom is non-null. 9.6/6: An enumeration whose underlying type is not fixed is an incomplete type from its point of declaration to immediately after the closing } of its enum-specifier, at which point it becomes a complete type. So the conversion is ill-formed. Jason But the conversion in the example (in decltype(__test_aux<_To1>(declval<_From1>( is in a SFINAE context, so shouldn't it gracefully fall back to the `(...)` overload? I think so, and clang++ and icc also compile the testcase fine (and we used to too, before r159006). Absolutely, the conversion being ill-formed means substitution fails, and we reject that candidate. I meant that we shouldn't get as far as type_promotes_to for an incomplete type. Jason
[aarch64] Fix ABI breakage with 128-bit bitfield types.
This is pretty unlikely in real code, but similar to Arm, the AArch64
ABI has a bug with the handling of 128-bit bit-fields, where if the
bit-field dominates the overall alignment the back-end code may end up
passing the argument correctly. This is a regression that started in
gcc-6 when the ABI support code was updated to support overaligned
types. The fix is very similar in concept to the Arm fix. 128-bit
bit-fields are fortunately extremely rare, so I'd be very surprised if
anyone has been bitten by this.
PR target/88469
gcc/
* config/aarch64/aarch64.c (aarch64_function_arg_alignment): Add new
argument ABI_BREAK. Set to true if the calculated alignment has
changed in gcc-9. Check bit-fields for their base type alignment.
(aarch64_layout_arg): Warn if argument passing has changed in gcc-9.
(aarch64_function_arg_boundary): Likewise.
(aarch64_gimplify_va_arg_expr): Likewise.
gcc/testsuite/
* gcc.target/aarch64/aapcs64/test_align-10.c: New test.
* gcc.target/aarch64/aapcs64/test_align-11.c: New test.
* gcc.target/aarch64/aapcs64/test_align-12.c: New test.
Committed to trunk.
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 5df5a8b7843..d6a9955804f 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -3765,12 +3765,16 @@ aarch64_vfp_is_call_candidate (cumulative_args_t pcum_v, machine_mode mode,
/* Given MODE and TYPE of a function argument, return the alignment in
bits. The idea is to suppress any stronger alignment requested by
- the user and opt for the natural alignment (specified in AAPCS64 \S 4.1).
- This is a helper function for local use only. */
+ the user and opt for the natural alignment (specified in AAPCS64 \S
+ 4.1). ABI_BREAK is set to true if the alignment was incorrectly
+ calculated in versions of GCC prior to GCC-9. This is a helper
+ function for local use only. */
static unsigned int
-aarch64_function_arg_alignment (machine_mode mode, const_tree type)
+aarch64_function_arg_alignment (machine_mode mode, const_tree type,
+bool *abi_break)
{
+ *abi_break = false;
if (!type)
return GET_MODE_ALIGNMENT (mode);
@@ -3786,9 +3790,22 @@ aarch64_function_arg_alignment (machine_mode mode, const_tree type)
return TYPE_ALIGN (TREE_TYPE (type));
unsigned int alignment = 0;
+ unsigned int bitfield_alignment = 0;
for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field))
if (TREE_CODE (field) == FIELD_DECL)
- alignment = std::max (alignment, DECL_ALIGN (field));
+ {
+ alignment = std::max (alignment, DECL_ALIGN (field));
+ if (DECL_BIT_FIELD_TYPE (field))
+ bitfield_alignment
+ = std::max (bitfield_alignment,
+ TYPE_ALIGN (DECL_BIT_FIELD_TYPE (field)));
+ }
+
+ if (bitfield_alignment > alignment)
+{
+ *abi_break = true;
+ return bitfield_alignment;
+}
return alignment;
}
@@ -3805,6 +3822,7 @@ aarch64_layout_arg (cumulative_args_t pcum_v, machine_mode mode,
int ncrn, nvrn, nregs;
bool allocate_ncrn, allocate_nvrn;
HOST_WIDE_INT size;
+ bool abi_break;
/* We need to do this once per argument. */
if (pcum->aapcs_arg_processed)
@@ -3881,25 +3899,28 @@ aarch64_layout_arg (cumulative_args_t pcum_v, machine_mode mode,
entirely general registers. */
if (allocate_ncrn && (ncrn + nregs <= NUM_ARG_REGS))
{
-
gcc_assert (nregs == 0 || nregs == 1 || nregs == 2);
/* C.8 if the argument has an alignment of 16 then the NGRN is
- rounded up to the next even number. */
+ rounded up to the next even number. */
if (nregs == 2
&& ncrn % 2
/* The == 16 * BITS_PER_UNIT instead of >= 16 * BITS_PER_UNIT
comparison is there because for > 16 * BITS_PER_UNIT
alignment nregs should be > 2 and therefore it should be
passed by reference rather than value. */
- && aarch64_function_arg_alignment (mode, type) == 16 * BITS_PER_UNIT)
+ && (aarch64_function_arg_alignment (mode, type, _break)
+ == 16 * BITS_PER_UNIT))
{
+ if (abi_break && warn_psabi && currently_expanding_gimple_stmt)
+ inform (input_location, "parameter passing for argument of type "
+ "%qT changed in GCC 9.1", type);
++ncrn;
gcc_assert (ncrn + nregs <= NUM_ARG_REGS);
}
/* NREGS can be 0 when e.g. an empty structure is to be passed.
- A reg is still generated for it, but the caller should be smart
+ A reg is still generated for it, but the caller should be smart
enough not to use it. */
if (nregs == 0 || nregs == 1 || GET_MODE_CLASS (mode) == MODE_INT)
pcum->aapcs_reg = gen_rtx_REG (mode, R0_REGNUM + ncrn);
@@ -3931,9 +3952,18 @@ aarch64_layout_arg (cumulative_args_t pcum_v, machine_mode mode,
on_stack:
pcum->aapcs_stack_words = size / UNITS_PER_WORD;
- if (aarch64_function_arg_alignment (mode, type) == 16 * BITS_PER_UNIT)
-pcum->aapcs_stack_size
Re: C++ PATCH for c++/89024 - ICE with incomplete enum type
On Fri, Jan 25, 2019 at 10:55:55AM -0600, Tim Song wrote: > On Thu, Jan 24, 2019 at 4:14 PM Jason Merrill wrote: > > > > On 1/24/19 2:16 PM, Marek Polacek wrote: > > > This test ICEs since r159006 which added > > > > > > type = ENUM_UNDERLYING_TYPE (type); > > > > > > to type_promotes_to. In this test ENUM_UNDERLYING_TYPE is null because we > > > haven't yet parsed '}' of the enum and the underlying type isn't fixed, > > > and > > > so checking TYPE_UNSIGNED crashed. > > > > > > I've added some checks to the test to see if the types seem to be OK; > > > clang++ > > > agrees. > > > > > > Bootstrapped/regtested on x86_64-linux, ok for trunk/8/7? > > > > > > 2019-01-24 Marek Polacek > > > > > > PR c++/89024 - ICE with incomplete enum type. > > > * cvt.c (type_promotes_to): Check if prom is non-null. > > > > 9.6/6: An enumeration whose underlying type is not fixed is an > > incomplete type from its point of declaration to immediately after the > > closing } of its enum-specifier, at which point it becomes a complete type. > > > > So the conversion is ill-formed. > > > > Jason > > But the conversion in the example (in > decltype(__test_aux<_To1>(declval<_From1>( > is in a SFINAE context, so shouldn't it gracefully fall back to the > `(...)` overload? I think so, and clang++ and icc also compile the testcase fine (and we used to too, before r159006). Marek
Re: [PATCH] C++ concepts: fix ICE with requires on dtors (PR c++/89036)
On 1/25/19 8:48 AM, David Malcolm wrote:
PR c++/89036 reports an ICE due to this assertion failing
1136 /* A class should never have more than one destructor. */
1137 gcc_assert (!current_fns || via_using || !DECL_DESTRUCTOR_P (method));
on this template with a pair of dtors, with
mutually exclusive "requires" clauses:
template
struct Y {
~Y() requires(true) = default;
~Y() requires(false) {}
};
(is this valid? my knowledge of this part of C++ is fairly hazy)
Yes. A more sensible example would have 'true' and 'false' replaced by
something determined from the template parms.
Nathan introduced this assertion as part of:
ca9219bf18c68a001d62ecb981bc9176b0feaf12 (aka r251340):
2017-08-24 Nathan Sidwell
Conversion operators kept on single overload set
I'd just drop the assert at this point.
nathan
--
Nathan Sidwell
Re: C++ PATCH for c++/89024 - ICE with incomplete enum type
On Thu, Jan 24, 2019 at 4:14 PM Jason Merrill wrote: > > On 1/24/19 2:16 PM, Marek Polacek wrote: > > This test ICEs since r159006 which added > > > > type = ENUM_UNDERLYING_TYPE (type); > > > > to type_promotes_to. In this test ENUM_UNDERLYING_TYPE is null because we > > haven't yet parsed '}' of the enum and the underlying type isn't fixed, and > > so checking TYPE_UNSIGNED crashed. > > > > I've added some checks to the test to see if the types seem to be OK; > > clang++ > > agrees. > > > > Bootstrapped/regtested on x86_64-linux, ok for trunk/8/7? > > > > 2019-01-24 Marek Polacek > > > > PR c++/89024 - ICE with incomplete enum type. > > * cvt.c (type_promotes_to): Check if prom is non-null. > > 9.6/6: An enumeration whose underlying type is not fixed is an > incomplete type from its point of declaration to immediately after the > closing } of its enum-specifier, at which point it becomes a complete type. > > So the conversion is ill-formed. > > Jason But the conversion in the example (in decltype(__test_aux<_To1>(declval<_From1>( is in a SFINAE context, so shouldn't it gracefully fall back to the `(...)` overload?
Re: [PATCH] libgfortran: Use proper gthr.h API
On 25 January 2019 12:44:30 CET, Sebastian Huber
wrote:
>libgfortran/
>
> * io/async.c (init_adv_cond): Use
> __GTHREAD_COND_INIT_FUNCTION().
LGTM.
Please CC the FORTRAN list for FORTRAN patches.
thanks,
>---
> libgfortran/io/async.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/libgfortran/io/async.c b/libgfortran/io/async.c
>index 3394e595a8b..e3d1d01122e 100644
>--- a/libgfortran/io/async.c
>+++ b/libgfortran/io/async.c
>@@ -224,7 +224,7 @@ init_adv_cond (struct adv_cond *ac)
> {
> ac->pending = 0;
> __GTHREAD_MUTEX_INIT_FUNCTION (>lock);
>- __gthread_cond_init_function (>signal);
>+ __GTHREAD_COND_INIT_FUNCTION (>signal);
> }
>
> /* Initialize an asyncronous unit, returning zero on success,
Re: Fix output_constructor_bitfield handling of wide bitfields (PR89037)
On January 25, 2019 1:12:07 PM GMT+01:00, Richard Sandiford
wrote:
>The testcase was failing because we were trying to access
>TREE_INT_CST_ELT (x, 1) of a 128-bit integer that was small enough
>to need only a single element.
>
>Tested on aarch64-linux-gnu, aarch64_be-elf and x86_64-linux-gnu.
>OK to install?
OK..
Richard.
>Richard
>
>
>2019-01-25 Richard Sandiford
>
>gcc/
> PR middle-end/89037
> * varasm.c (output_constructor_bitfield): Use wi::extract_uhwi
> instead of accessing TREE_INT_CST_ELT directly.
>
>gcc/testsuite/
> PR middle-end/89037
> * gcc.dg/pr89037.c: New test.
>
>Index: gcc/varasm.c
>===
>--- gcc/varasm.c 2019-01-04 11:39:27.182246717 +
>+++ gcc/varasm.c 2019-01-25 12:10:23.969006336 +
>@@ -5349,7 +5349,7 @@ output_constructor_bitfield (oc_local_st
> {
> int this_time;
> int shift;
>- HOST_WIDE_INT value;
>+ unsigned HOST_WIDE_INT value;
> HOST_WIDE_INT next_byte = next_offset / BITS_PER_UNIT;
> HOST_WIDE_INT next_bit = next_offset % BITS_PER_UNIT;
>
>@@ -5381,15 +5381,13 @@ output_constructor_bitfield (oc_local_st
> this_time = end - shift + 1;
> }
>
>-/* Now get the bits from the appropriate constant word. */
>-value = TREE_INT_CST_ELT (local->val, shift /
>HOST_BITS_PER_WIDE_INT);
>-shift = shift & (HOST_BITS_PER_WIDE_INT - 1);
>+/* Now get the bits we want to insert. */
>+value = wi::extract_uhwi (wi::to_widest (local->val),
>+ shift, this_time);
>
> /* Get the result. This works only when:
>1 <= this_time <= HOST_BITS_PER_WIDE_INT. */
>-local->byte |= (((value >> shift)
>- & (((HOST_WIDE_INT) 2 << (this_time - 1)) - 1))
>-<< (BITS_PER_UNIT - this_time - next_bit));
>+local->byte |= value << (BITS_PER_UNIT - this_time - next_bit);
> }
> else
> {
>@@ -5406,15 +5404,13 @@ output_constructor_bitfield (oc_local_st
> this_time
> = HOST_BITS_PER_WIDE_INT - (shift & (HOST_BITS_PER_WIDE_INT -
>1));
>
>-/* Now get the bits from the appropriate constant word. */
>-value = TREE_INT_CST_ELT (local->val, shift /
>HOST_BITS_PER_WIDE_INT);
>-shift = shift & (HOST_BITS_PER_WIDE_INT - 1);
>+/* Now get the bits we want to insert. */
>+value = wi::extract_uhwi (wi::to_widest (local->val),
>+ shift, this_time);
>
> /* Get the result. This works only when:
>1 <= this_time <= HOST_BITS_PER_WIDE_INT. */
>-local->byte |= (((value >> shift)
>- & (((HOST_WIDE_INT) 2 << (this_time - 1)) - 1))
>-<< next_bit);
>+local->byte |= value << next_bit;
> }
>
> next_offset += this_time;
>Index: gcc/testsuite/gcc.dg/pr89037.c
>===
>--- /dev/null 2019-01-24 08:42:49.147091464 +
>+++ gcc/testsuite/gcc.dg/pr89037.c 2019-01-25 12:10:23.965006370 +
>@@ -0,0 +1,24 @@
>+/* { dg-do run { target int128 } } */
>+/* { dg-options "" } */
>+
>+struct s
>+{
>+ __int128 y : 66;
>+};
>+typedef struct s T;
>+T a[] = { 1, 1, 0x12345, 0xff01, 1ULL << 63, (__int128) 1 <<
>64,
>+((__int128) 1 << 64) | 1 };
>+
>+int
>+main (void)
>+{
>+ if (a[0].y != 1
>+ || a[1].y != 1
>+ || a[2].y != 0x12345
>+ || a[3].y != 0xff01
>+ || a[4].y != (1ULL << 63)
>+ || a[5].y != ((__int128) 1 << 64)
>+ || a[6].y != (((__int128) 1 << 64) | 1))
>+__builtin_abort ();
>+ return 0;
>+}
[PATCH] C++ concepts: fix ICE with requires on dtors (PR c++/89036)
PR c++/89036 reports an ICE due to this assertion failing
1136 /* A class should never have more than one destructor. */
1137 gcc_assert (!current_fns || via_using || !DECL_DESTRUCTOR_P (method));
on this template with a pair of dtors, with
mutually exclusive "requires" clauses:
template
struct Y {
~Y() requires(true) = default;
~Y() requires(false) {}
};
(is this valid? my knowledge of this part of C++ is fairly hazy)
Nathan introduced this assertion as part of:
ca9219bf18c68a001d62ecb981bc9176b0feaf12 (aka r251340):
2017-08-24 Nathan Sidwell
Conversion operators kept on single overload set
which, amongst other changes to add_method had this:
/* A class should never have more than one destructor. */
- if (current_fns && DECL_MAYBE_IN_CHARGE_DESTRUCTOR_P (method))
-return false;
+ gcc_assert (!current_fns || !DECL_DESTRUCTOR_P (method));
The following patch generalizes the assertion to allow for multiple
dtors if they have "requires" clauses. (I already generalized
the assertion in another way in r268041 to fix PR c++/88699;
alternatively, is this too much like whack-a-mole, and would
dropping the assertion altogether be better?)
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
OK for trunk?
gcc/cp/ChangeLog:
PR c++/89036
* class.c (add_method): Generalize assertion to allow for
multiple dtors if they have "requires" clauses.
gcc/testsuite/ChangeLog:
PR c++/89036
* g++.dg/concepts/pr89036.C: New test.
---
gcc/cp/class.c | 11 +--
gcc/testsuite/g++.dg/concepts/pr89036.C | 8
2 files changed, 17 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/concepts/pr89036.C
diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index e8773c2..fb46f92 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -1133,8 +1133,15 @@ add_method (tree type, tree method, bool via_using)
}
}
- /* A class should never have more than one destructor. */
- gcc_assert (!current_fns || via_using || !DECL_DESTRUCTOR_P (method));
+ /* A class should never have more than one destructor... */
+ gcc_assert (!current_fns
+ || via_using
+ || !DECL_DESTRUCTOR_P (method)
+ /* ...unless the destructors are constrained by "requires"
+clauses. */
+ || (flag_concepts
+ && get_constraints (method)
+ && CI_DECLARATOR_REQS (get_constraints (method;
current_fns = ovl_insert (method, current_fns, via_using);
diff --git a/gcc/testsuite/g++.dg/concepts/pr89036.C
b/gcc/testsuite/g++.dg/concepts/pr89036.C
new file mode 100644
index 000..f83ef8b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/pr89036.C
@@ -0,0 +1,8 @@
+// { dg-do compile { target c++11 } }
+// { dg-options "-fconcepts" }
+
+template
+struct Y {
+ ~Y() requires(true) = default;
+ ~Y() requires(false) {}
+};
--
1.8.5.3
RFA: PATCH to gimple-fold.c for c++/80916, bogus "static but not defined" warning
Here we warn because i::dispatch has internal linkage (because l
does) and is never instantiated (because the vtable is never emitted).
The regression happened because devirtualization started adding it to
cgraph as a possible target. I think the way to fix this is to avoid
adding an undefined internal function to cgraph as a possible target,
since it is not, in fact, possible for it to be the actual target.
I think that the best place to fix this would be in
can_refer_decl_in_current_unit_p, since the same reasoning applies to
other possible references. But we could fix it only in
gimple_get_virt_method_for_vtable.
First patch tested x86_64-pc-linux-gnu.
commit 3a02b58301c2c11620c2adc1aee4db1b7e8e36f2
Author: Jason Merrill
Date: Fri Jan 25 09:09:17 2019 -0500
PR c++/80916 - spurious "static but not defined" warning.
* gimple-fold.c (can_refer_decl_in_current_unit_p): Return false
for an internal function with no definition.
diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
index 92d3fb4a9e0..20564e26de1 100644
--- a/gcc/gimple-fold.c
+++ b/gcc/gimple-fold.c
@@ -135,6 +135,12 @@ can_refer_decl_in_current_unit_p (tree decl, tree from_decl)
return !node || !node->global.inlined_to;
}
+ /* This function is internal and not defined, so nothing can refer to it. */
+ if (!TREE_PUBLIC (decl) && DECL_EXTERNAL (decl)
+ && TREE_CODE (decl) == FUNCTION_DECL
+ && DECL_INITIAL (decl) == NULL_TREE)
+return false;
+
/* We will later output the initializer, so we can refer to it.
So we are concerned only when DECL comes from initializer of
external var or var that has been optimized out. */
diff --git a/gcc/testsuite/g++.dg/warn/unused-fn1.C b/gcc/testsuite/g++.dg/warn/unused-fn1.C
new file mode 100644
index 000..aabc01b3f44
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/unused-fn1.C
@@ -0,0 +1,16 @@
+// PR c++/80916
+// { dg-options "-Os -Wunused" }
+
+struct j {
+ virtual void dispatch(void *) {}
+};
+template
+struct i : j {
+ void dispatch(void *) {} // warning: 'void i< >::dispatch(void*) [with = {anonymous}::l]' declared 'static' but never defined [-Wunused-function]
+};
+namespace {
+ struct l : i {};
+}
+void f(j *k) {
+ k->dispatch(0);
+}
diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
index 92d3fb4a9e0..8d63d815a5e 100644
--- a/gcc/gimple-fold.c
+++ b/gcc/gimple-fold.c
@@ -7146,7 +7146,10 @@ gimple_get_virt_method_for_vtable (HOST_WIDE_INT token,
devirtualize. This can happen in WHOPR when the actual method
ends up in other partition, because we found devirtualization
possibility too late. */
- if (!can_refer_decl_in_current_unit_p (fn, vtable))
+ if (!can_refer_decl_in_current_unit_p (fn, vtable)
+ || (!TREE_PUBLIC (decl) && DECL_EXTERNAL (decl)
+ && TREE_CODE (decl) == FUNCTION_DECL
+ && DECL_INITIAL (decl) == NULL_TREE))
{
if (can_refer)
{
Re: [Driver] Add support for -fuse-ld=lld
On 20/10/18 12:18 +0200, Romain Geissler wrote: Hi, I would like to raise again the question of supporting -fuse-ld=ldd. A patch implementing it was already submitted in https://gcc.gnu.org/ml/gcc-patches/2016-06/msg01722.html by Davide Italiano. This patch still applies correctly to current trunk. I am CC-ing the original author and re-posting it here unchanged for reference. I think we can consider this patch as relevant despite the goals and licence difference of LLVM vs GNU, based on what was written by Mike Stump in https://gcc.gnu.org/ml/gcc-patches/2016-07/msg00157.html Back then, the technical problem raised by lld was reported as https://bugs.llvm.org/show_bug.cgi?id=28414 now closed. In this bug, every reported problems have been fixed except the last one. H.J. Lu mentions this last problem (lld does not produces symbol versions predecessor relationship while ld.bfd and ld.gold do, which seems to be a decision taken on purpose and advertised as a harmless change) as being one reason against supporting in -fuse-ld=ldd in gcc. Is it still the case today, and if yes, why ? Is there any other reason why -fuse-ld=ldd shall not be supported by gcc ? This patch was committed to trunk (r265940), but HJ's review comments were never addressed (and look correct to me): https://gcc.gnu.org/ml/gcc-patches/2016-07/msg00146.html The multi-line condition should be split before the || operator not after it, and the negation of -fuse-ld=lld is not -fuse-ld-lld.
Re: C++ PATCH for c++/78244 - narrowing conversion in template not detected, part 2
On 1/24/19 7:17 PM, Marek Polacek wrote:
On Wed, Jan 23, 2019 at 03:34:04PM -0500, Jason Merrill wrote:
On Wed, Jan 23, 2019 at 12:57 PM Marek Polacek wrote:
On Wed, Jan 23, 2019 at 09:00:36AM -0500, Jason Merrill wrote:
I was talking about digest_init, not reshape_init. digest_init calls
convert_for_initialization.
/facepalm
So yes, digest_init calls convert_for_initialization which will end up
calling perform_implicit_conversion_flags which could call convert_like_real
where the narrowing warnings are given, but it doesn't, we go to this case:
else if (processing_template_decl && conv->kind != ck_identity)
{
/* In a template, we are only concerned about determining the
type of non-dependent expressions, so we do not have to
perform the actual conversion. But for initializers, we
need to be able to perform it at instantiation
(or instantiate_non_dependent_expr) time. */
expr = build1 (IMPLICIT_CONV_EXPR, type, expr);
finish_decltype_type throws away the expression because it's not dependent, and
only uses its type. So narrowing remains undetected. Not sure if I should mess
with perform_implicit_conversion_flags.
Let's try that; this is a situation where the comment is incorrect.
Perhaps just call check_narrowing here if appropriate, rather than go
through the whole conversion machinery.
I have not been successful.
First, I modified perform_implicit_conversion_flags to go the convert_like
route when dealing with something non-dependent. That breaks e.g. in
build_value_init:
346 /* The AGGR_INIT_EXPR tweaking below breaks in templates. */
347 gcc_assert (!processing_template_decl
348 || (SCALAR_TYPE_P (type) || TREE_CODE (type) ==
ARRAY_TYPE));
Even if I restrict the convert_like way for non-dependent exprs in a template
for scalars, it still breaks elsewhere, e.g. constexpr-template3.C where it
complains about taking the address of an rvalue.
Second, I added check_narrowing to the processing_template_decl case in
perform_implicit_conversion_flags. That works except it breaks
constexpr-inst1.C -- we no longer get the error. That's because currently
check_narrowing in finish_compound_literal calls maybe_constant_init, which
calls instantiate_constexpr_fns and we get the desired diagnostic. But if
I move check_narrowing to perform_implicit_conversion_flags, we no longer
call it in this case -- processing_template_decl is 0 so we call convert_like
but that doesn't do the trick.
So, back to the patch that leaves check_narrowing in finish_compound_literal?
That patch still needs a test for the aggregate case.
Jason
Re: [C++ Patch] PR 88969 ("[9 Regression] ICE in build_op_delete_call, at cp/call.c:6509")
On 1/25/19 6:20 AM, Paolo Carlini wrote: Hi, On 24/01/19 23:21, Jason Merrill wrote: On 1/24/19 2:53 PM, Paolo Carlini wrote: Hi, as far as I can see this ICE on invalid points to a substantive, if minor, weakness of our implementation of the destroying operator delete facility: we aren't implementing the bits, per 7.6.2.5/(10.1), about destroying operator delete having precedence over any other operator delete. Thus the below, which is the most straightforward implementation I have been able to figure out given the current infrastructure. Tested x86_64-linux. OK, thanks. Thanks you. Yesterday I didn't notice that the bug report includes another testcase, for an unrelated buglet: if the destroying operator delete is wrongly specified as not-taking a pointer to the class type as first argument, TYPE_POINTER_TO may not be set yet in coerce_delete_type and we crash later on. Thus the below, which changes it to build_pointer_type. Also OK. :) Jason
Re: [PATCH] Fix __has_include error recovery in libcpp (PR preprocessor/88974)
On 1/25/19 12:15 AM, Jakub Jelinek wrote: On Thu, Jan 24, 2019 at 05:16:52PM -0500, Jason Merrill wrote: --- libcpp/expr.c.jj2019-01-01 12:38:16.132007335 +0100 +++ libcpp/expr.c 2019-01-24 14:07:10.080774120 +0100 @@ -2238,7 +2238,9 @@ parse_has_include (cpp_reader *pfile, en XDELETEVEC (fname); } - if (paren && cpp_get_token (pfile)->type != CPP_CLOSE_PAREN) + if (paren + && pfile->cur_token[-1].type != CPP_EOF Is there a reason not to use the SEEN_EOL macro here, too (first moving it into a header)? I can move it if you want to internal.h. OK with that change. Jason
Re: [backtrace] Avoid segfault
On Fri, Jan 25, 2019 at 5:27 AM Tom de Vries wrote: > > On 25-01-19 14:17, Tom de Vries wrote: > > On 25-01-19 01:51, Ian Lance Taylor wrote: > >> On Thu, Jan 24, 2019 at 4:11 PM Nathan Sidwell wrote: > >>> > >>> I just tripped over a segfault in libbacktrace. We apply strrchr to a > >>> possibly NULL filename, with predictable results when it is. > >>> > >>> elf.c:3044 passes NULL as the filename parm: > >>> ret = elf_add (state, NULL, d, base_address, error_callback, > >>> data, > >>> fileline_fn, found_sym, found_dwarf, NULL, 0, 1, > >>> NULL, > >>> 0); > >>> > >>> This gets to elf_open_debugfile_by_debuglink which passes it on through: > >>>ddescriptor = elf_find_debugfile_by_debuglink (state, filename, > >>> debuglink_name, > >>> error_callback, data); > >>> > >>> this patch avoids the strrchr when filename is null. I reordered the > >>> way prefix & prefix len got set, finding it prefereable to: > >>>slash = filename ? NULL : strrchr (filename, '/'); > >>> but if you prefer to avoid the assignment in the conditional I'm fine > >>> with that too. > >> > >> Yeah, please don't do an assignment in a conditional. > >> > >> Why don't we just pass "" instead of NULL in the call to elf_add? If > >> that works, that is OK. > >> > > > > With this refactoring preamble ... > > ... I can more easily add a test-case btest_dwz_gnudebuglink, which > triggers a segfault due to strrchr being called with a NULL string. > > This patch fixes it by passing "" instead of NULL, in the call to > elf_add at line 3083 (for .gnu_debugaltlink), not the call to elf_add at > line 3044 (for .gnu_debuglink) mentioned above. > > Nathan, does this fix the problem for you? If not, can you provide a > reproducer, or give a hint on how one could be constructed? This is OK. s/Call/call/ in the ChangeLog entry. Thanks. Ian
Re: [backtrace] Avoid segfault
On Fri, Jan 25, 2019 at 5:17 AM Tom de Vries wrote: > > On 25-01-19 01:51, Ian Lance Taylor wrote: > > On Thu, Jan 24, 2019 at 4:11 PM Nathan Sidwell wrote: > >> > >> I just tripped over a segfault in libbacktrace. We apply strrchr to a > >> possibly NULL filename, with predictable results when it is. > >> > >> elf.c:3044 passes NULL as the filename parm: > >> ret = elf_add (state, NULL, d, base_address, error_callback, > >> data, > >> fileline_fn, found_sym, found_dwarf, NULL, 0, 1, > >> NULL, > >> 0); > >> > >> This gets to elf_open_debugfile_by_debuglink which passes it on through: > >>ddescriptor = elf_find_debugfile_by_debuglink (state, filename, > >> debuglink_name, > >> error_callback, data); > >> > >> this patch avoids the strrchr when filename is null. I reordered the > >> way prefix & prefix len got set, finding it prefereable to: > >>slash = filename ? NULL : strrchr (filename, '/'); > >> but if you prefer to avoid the assignment in the conditional I'm fine > >> with that too. > > > > Yeah, please don't do an assignment in a conditional. > > > > Why don't we just pass "" instead of NULL in the call to elf_add? If > > that works, that is OK. > > > > With this refactoring preamble ... This is OK. Thanks. Ian
[PATCH, OpenACC] Remove spurious OpenACC error on combining "auto" with gang/worker/vector
On OpenACC loop constructs, it is OK according to the OpenACC spec to
have both the "auto" clause and one or more of the "gang", "worker", or
"vector" clauses. GCC emits errors for this combination; this patch
eliminates that error.
OK for openacc-gcc-8-branch?
Thanks,
Gergö
gcc/
* omp-low.c (check_oacc_kernel_gwv): Remove spurious error message.
* omp-offload.c (oacc_loop_fixed_partitions): Likewise.
gcc/testsuite/
* c-c++-common/goacc/combined-directives-3.c: Adjust test.
* c-c++-common/goacc/loop-2-kernels.c: Likewise.
* c-c++-common/goacc/loop-2-parallel.c: Likewise.
>From d8e7f1826d423de05e11afcb6e422ccaced0f6ea Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Gerg=C3=B6=20Barany?=
Date: Wed, 23 Jan 2019 03:10:07 -0800
Subject: [PATCH] Remove spurious OpenACC error on combining "auto" with
gang/worker/vector
gcc/
* omp-low.c (check_oacc_kernel_gwv): Remove spurious error message.
* omp-offload.c (oacc_loop_fixed_partitions): Likewise.
gcc/testsuite/
* c-c++-common/goacc/combined-directives-3.c: Adjust test.
* c-c++-common/goacc/loop-2-kernels.c: Likewise.
* c-c++-common/goacc/loop-2-parallel.c: Likewise.
---
gcc/ChangeLog.openacc| 5 +
gcc/omp-low.c| 3 ---
gcc/omp-offload.c| 7 ++-
gcc/testsuite/ChangeLog.openacc | 6 ++
gcc/testsuite/c-c++-common/goacc/combined-directives-3.c | 4 ++--
gcc/testsuite/c-c++-common/goacc/loop-2-kernels.c| 12 ++--
gcc/testsuite/c-c++-common/goacc/loop-2-parallel.c | 12 ++--
7 files changed, 27 insertions(+), 22 deletions(-)
diff --git a/gcc/ChangeLog.openacc b/gcc/ChangeLog.openacc
index 932fb37..f3c741c 100644
--- a/gcc/ChangeLog.openacc
+++ b/gcc/ChangeLog.openacc
@@ -1,5 +1,10 @@
2019-01-24 Gergö Barany
+ * omp-low.c (check_oacc_kernel_gwv): Remove spurious error message.
+ * omp-offload.c (oacc_loop_fixed_partitions): Likewise.
+
+2019-01-24 Gergö Barany
+
* gimplify.c (oacc_default_clause): Refactor and unify computation of
default mapping clauses.
diff --git a/gcc/omp-low.c b/gcc/omp-low.c
index 72b6548..f48002e 100644
--- a/gcc/omp-low.c
+++ b/gcc/omp-low.c
@@ -2397,9 +2397,6 @@ check_oacc_kernel_gwv (gomp_for *stmt, omp_context *ctx)
if (has_seq && (this_mask || has_auto))
error_at (gimple_location (stmt), "% overrides other"
" OpenACC loop specifiers");
- else if (has_auto && this_mask)
- error_at (gimple_location (stmt), "% conflicts with other"
- " OpenACC loop specifiers");
if (this_mask & outer_mask)
error_at (gimple_location (stmt), "inner loop uses same"
diff --git a/gcc/omp-offload.c b/gcc/omp-offload.c
index d428c6f..57a7a06 100644
--- a/gcc/omp-offload.c
+++ b/gcc/omp-offload.c
@@ -1211,14 +1211,11 @@ oacc_loop_fixed_partitions (oacc_loop *loop, unsigned outer_mask)
bool maybe_auto
= !seq_par && this_mask == (tiling ? this_mask & -this_mask : 0);
- if ((this_mask != 0) + auto_par + seq_par > 1)
+ if (seq_par && (this_mask != 0 || auto_par))
{
if (noisy)
error_at (loop->loc,
- seq_par
- ? G_("% overrides other OpenACC loop specifiers")
- : G_("% conflicts with other OpenACC loop "
- "specifiers"));
+ G_("% overrides other OpenACC loop specifiers"));
maybe_auto = false;
loop->flags &= ~OLF_AUTO;
if (seq_par)
diff --git a/gcc/testsuite/ChangeLog.openacc b/gcc/testsuite/ChangeLog.openacc
index 3bdce2e..3850d97 100644
--- a/gcc/testsuite/ChangeLog.openacc
+++ b/gcc/testsuite/ChangeLog.openacc
@@ -1,3 +1,9 @@
+2019-01-24 Gergö Barany
+
+ * c-c++-common/goacc/combined-directives-3.c: Adjust test.
+ * c-c++-common/goacc/loop-2-kernels.c: Likewise.
+ * c-c++-common/goacc/loop-2-parallel.c: Likewise.
+
2019-01-09 Julian Brown
* c-c++-common/cpp/openacc-define-3.c: Update expected value for
diff --git a/gcc/testsuite/c-c++-common/goacc/combined-directives-3.c b/gcc/testsuite/c-c++-common/goacc/combined-directives-3.c
index 77d4182..5aa84dc 100644
--- a/gcc/testsuite/c-c++-common/goacc/combined-directives-3.c
+++ b/gcc/testsuite/c-c++-common/goacc/combined-directives-3.c
@@ -12,9 +12,9 @@ main ()
for (y = 0; y < 10; y++)
;
-#pragma acc parallel loop gang auto /* { dg-error "'auto' conflicts with other OpenACC loop specifiers" } */
+#pragma acc parallel loop gang seq /* { dg-error "'seq' overrides other OpenACC loop specifiers" } */
for (x = 0; x < 10; x++)
-#pragma acc loop worker auto /* { dg-error "'auto' conflicts with other OpenACC loop specifiers" } */
+#pragma acc loop worker seq /* { dg-error "'seq' overrides other OpenACC loop specifiers" } */
for (y = 0; y < 10; y++)
#pragma acc loop vector
for (z = 0; z < 10; z++)
diff --git a/gcc/testsuite/c-c++-common/goacc/loop-2-kernels.c b/gcc/testsuite/c-c++-common/goacc/loop-2-kernels.c
index
[PATCH, OpenACC] Rework OpenACC Fortran DO loop initialization
This patch moves OpenACC Fortan DO loop setup code from the head of a
region to just before each loop. This is in preparation for upcoming
patches reworking the handling of OpenACC kernels regions.
OK for openacc-gcc-8-branch?
Thanks,
Gergö
gcc/fortran/
* trans-openmp.c (gfc_privatize_nodesc_array_clauses): Renamed from
gfc_privatize_nodesc_arrays, initialization part factored out to...
(gfc_reinitialize_privatized_arrays): ... this new function, called...
(gfc_trans_omp_do): ... from here for OpenACC loops.
libgomp/
* testsuite/libgomp.oacc-fortran/initialize_kernels_loops.f90: New
test.
>From f4768a88a4e2ab5dc80feb7bfb06cd273c849f72 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Gerg=C3=B6=20Barany?=
Date: Mon, 21 Jan 2019 03:08:57 -0800
Subject: [PATCH] Rework OpenACC Fortran DO loop initialization
Fortran DO loops on arrays with non-constant bounds (like a(lo:hi)) need
special setup code to compute the bounds and offsets for the iteration. In
an OpenACC region containing multiple loops, this used to be done in a block
of code at the start of the region for all of the loops. But the upcoming
kernels conversion expects this kind of setup code to immediately precede
the corresponding loop, and variables are not mapped correctly otherwise.
This patch separates out the initialization part for each loop and places it
immediately before the loop.
gcc/fortran/
* trans-openmp.c (gfc_privatize_nodesc_array_clauses): Renamed from
gfc_privatize_nodesc_arrays, initialization part factored out to...
(gfc_reinitialize_privatized_arrays): ... this new function, called...
(gfc_trans_omp_do): ... from here for OpenACC loops.
libgomp/
* testsuite/libgomp.oacc-fortran/initialize_kernels_loops.f90: New test.
---
gcc/fortran/ChangeLog.openacc | 7 ++
gcc/fortran/trans-openmp.c | 86 +-
libgomp/ChangeLog.openacc | 4 +
.../initialize_kernels_loops.f90 | 31
4 files changed, 92 insertions(+), 36 deletions(-)
create mode 100644 libgomp/testsuite/libgomp.oacc-fortran/initialize_kernels_loops.f90
diff --git a/gcc/fortran/ChangeLog.openacc b/gcc/fortran/ChangeLog.openacc
index 0f31f3e..450056d 100644
--- a/gcc/fortran/ChangeLog.openacc
+++ b/gcc/fortran/ChangeLog.openacc
@@ -1,3 +1,10 @@
+2019-01-24 Gergö Barany
+
+ * trans-openmp.c (gfc_privatize_nodesc_array_clauses): Renamed from
+ gfc_privatize_nodesc_arrays, initialization part factored out to...
+ (gfc_reinitialize_privatized_arrays): ... this new function, called...
+ (gfc_trans_omp_do): ... from here for OpenACC loops.
+
2019-01-09 Julian Brown
* cpp.c (cpp_define_builtins): Update _OPENACC define to 201711.
diff --git a/gcc/fortran/trans-openmp.c b/gcc/fortran/trans-openmp.c
index d5dbf18..5a444c3 100644
--- a/gcc/fortran/trans-openmp.c
+++ b/gcc/fortran/trans-openmp.c
@@ -3198,6 +3198,44 @@ gfc_scan_nodesc_arrays (gfc_expr **e, int *walk_subtrees ATTRIBUTE_UNUSED,
return 0;
}
+/* Reinitialize any arrays used inside CODE. Place the initialization
+ sequences in CODE. */
+
+static void
+gfc_reinitialize_privatized_arrays (gfc_code *code, stmtblock_t *block)
+{
+ hash_set *array_set = new hash_set ();
+ gfc_code_walker (, gfc_dummy_code_callback, gfc_scan_nodesc_arrays,
+ array_set);
+
+ hash_set::iterator its = array_set->begin ();
+
+ for (; its != array_set->end (); ++its)
+{
+ gfc_symbol *sym = *its;
+ tree parm = gfc_get_symbol_decl (sym);
+ tree type = TREE_TYPE (parm);
+ tree offset, tmp;
+
+ /* Evaluate the bounds of the array. */
+ gfc_trans_array_bounds (type, sym, , block, false);
+
+ /* Set the offset. */
+ if (TREE_CODE (GFC_TYPE_ARRAY_OFFSET (type)) == VAR_DECL)
+gfc_add_modify (block, GFC_TYPE_ARRAY_OFFSET (type), offset);
+
+ /* Set the pointer itself if we aren't using the parameter
+ directly. */
+ if (TREE_CODE (parm) != PARM_DECL && DECL_LANG_SPECIFIC (parm)
+ && GFC_DECL_SAVED_DESCRIPTOR (parm))
+{
+ tmp = convert (TREE_TYPE (parm),
+ GFC_DECL_SAVED_DESCRIPTOR (parm));
+ gfc_add_modify (block, parm, tmp);
+}
+}
+}
+
/* Build a set of internal array variables (lbound, ubound, stride, etc.)
that need privatization. */
@@ -3219,41 +3257,12 @@ gfc_privatize_nodesc_arrays_1 (tree *tp, int *walk_subtrees, void *data)
return NULL;
}
-/* Reinitialize all of the arrays inside ARRAY_SET in BLOCK. Append private
- clauses for those arrays in CLAUSES. */
+/* Append private clauses for the arrays in BLOCK to CLAUSES. */
static tree
-gfc_privatize_nodesc_arrays (hash_set *array_set,
- stmtblock_t *block, tree clauses)
+gfc_privatize_nodesc_array_clauses (stmtblock_t *block, tree clauses)
{
- hash_set::iterator its = array_set->begin ();
hash_set *private_decls = new hash_set;
-
[PATCH, OpenACC] Rework computation of default OpenACC mapping clauses
This patch unifies and simplifies the handling of OpenACC default
mapping clauses for parallel, serial, and kernels regions.
OK for openacc-gcc-8-branch?
Thanks,
Gergö
gcc/
* gimplify.c (oacc_default_clause): Refactor and unify computation of
default mapping clauses.
>From 32a38daf2084bb266aa3a0c61c9176098d2d4bdb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Gerg=C3=B6=20Barany?=
Date: Mon, 21 Jan 2019 03:01:02 -0800
Subject: [PATCH] Rework computation of default OpenACC mapping clauses
gcc/
* gimplify.c (oacc_default_clause): Refactor and unify computation of
default mapping clauses.
---
gcc/ChangeLog.openacc | 5
gcc/gimplify.c| 75 +--
2 files changed, 41 insertions(+), 39 deletions(-)
diff --git a/gcc/ChangeLog.openacc b/gcc/ChangeLog.openacc
index 22cdb5b..932fb37 100644
--- a/gcc/ChangeLog.openacc
+++ b/gcc/ChangeLog.openacc
@@ -1,3 +1,8 @@
+2019-01-24 Gergö Barany
+
+ * gimplify.c (oacc_default_clause): Refactor and unify computation of
+ default mapping clauses.
+
2019-01-09 Julian Brown
* doc/invoke.texi: Update mention of OpenACC version to 2.6.
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index a60e395..a6a4d2a 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -7191,58 +7191,55 @@ oacc_default_clause (struct gimplify_omp_ctx *ctx, tree decl, unsigned flags)
flags |= GOVD_MAP_TO_ONLY;
}
+ unsigned private_mapping_flag = 0;
+ unsigned default_scalar_flags = 0;
+ /* Aggregates default to 'present_or_copy', or 'present'. */
+ unsigned aggregate_flags
+= (ctx->default_kind != OMP_CLAUSE_DEFAULT_PRESENT
+? GOVD_MAP
+: GOVD_MAP | GOVD_MAP_FORCE_PRESENT);
+
switch (ctx->region_type)
{
case ORT_ACC_KERNELS:
rkind = "kernels";
-
- if (is_private)
- flags |= GOVD_MAP;
- else if (AGGREGATE_TYPE_P (type))
- {
- /* Aggregates default to 'present_or_copy', or 'present'. */
- if (ctx->default_kind != OMP_CLAUSE_DEFAULT_PRESENT)
- flags |= GOVD_MAP;
- else
- flags |= GOVD_MAP | GOVD_MAP_FORCE_PRESENT;
- }
- else
- /* Scalars default to 'copy'. */
- flags |= GOVD_MAP | GOVD_MAP_FORCE;
-
+ /* Scalars default to 'copy'. */
+ default_scalar_flags = GOVD_MAP | GOVD_MAP_FORCE;
+ /* There are no private mappings on kernels regions. */
+ gcc_assert (!is_private);
break;
-
case ORT_ACC_PARALLEL:
+ rkind = "parallel";
+ /* Scalars default to 'firstprivate'. */
+ default_scalar_flags = GOVD_FIRSTPRIVATE;
+ private_mapping_flag = GOVD_FIRSTPRIVATE;
+ break;
case ORT_ACC_SERIAL:
- rkind = ctx->region_type == ORT_ACC_PARALLEL ? "parallel" : "serial";
-
- if (TREE_CODE (type) == REFERENCE_TYPE
- && TREE_CODE (TREE_TYPE (type)) == POINTER_TYPE)
- flags |= GOVD_MAP | GOVD_MAP_0LEN_ARRAY;
- else if (!lang_GNU_Fortran () && TREE_CODE (type) == POINTER_TYPE)
- flags |= GOVD_MAP | GOVD_MAP_0LEN_ARRAY;
- else if (is_private)
- flags |= GOVD_FIRSTPRIVATE;
- else if (on_device || declared)
- flags |= GOVD_MAP;
- else if (AGGREGATE_TYPE_P (type))
- {
- /* Aggregates default to 'present_or_copy', or 'present'. */
- if (ctx->default_kind != OMP_CLAUSE_DEFAULT_PRESENT)
- flags |= GOVD_MAP;
- else
- flags |= GOVD_MAP | GOVD_MAP_FORCE_PRESENT;
- }
- else
- /* Scalars default to 'firstprivate'. */
- flags |= GOVD_FIRSTPRIVATE;
-
+ rkind = "serial";
+ /* Scalars default to 'firstprivate'. */
+ default_scalar_flags = GOVD_FIRSTPRIVATE;
+ private_mapping_flag = GOVD_FIRSTPRIVATE;
break;
default:
gcc_unreachable ();
}
+ if (TREE_CODE (type) == REFERENCE_TYPE
+ && TREE_CODE (TREE_TYPE (type)) == POINTER_TYPE)
+flags |= GOVD_MAP | GOVD_MAP_0LEN_ARRAY;
+ else if (!lang_GNU_Fortran () && TREE_CODE (type) == POINTER_TYPE)
+flags |= GOVD_MAP | GOVD_MAP_0LEN_ARRAY;
+ else if (is_private)
+flags |= private_mapping_flag;
+ else if (on_device || declared)
+flags |= GOVD_MAP;
+ else if (AGGREGATE_TYPE_P (type))
+flags |= aggregate_flags;
+ else
+/* This is a scalar getting the default mapping. */
+flags |= default_scalar_flags;
+
if (DECL_ARTIFICIAL (decl))
; /* We can get compiler-generated decls, and should not complain
about them. */
--
2.8.1
Re: [backtrace] Avoid segfault
On 25-01-19 14:17, Tom de Vries wrote: > On 25-01-19 01:51, Ian Lance Taylor wrote: >> On Thu, Jan 24, 2019 at 4:11 PM Nathan Sidwell wrote: >>> >>> I just tripped over a segfault in libbacktrace. We apply strrchr to a >>> possibly NULL filename, with predictable results when it is. >>> >>> elf.c:3044 passes NULL as the filename parm: >>> ret = elf_add (state, NULL, d, base_address, error_callback, data, >>> fileline_fn, found_sym, found_dwarf, NULL, 0, 1, >>> NULL, >>> 0); >>> >>> This gets to elf_open_debugfile_by_debuglink which passes it on through: >>>ddescriptor = elf_find_debugfile_by_debuglink (state, filename, >>> debuglink_name, >>> error_callback, data); >>> >>> this patch avoids the strrchr when filename is null. I reordered the >>> way prefix & prefix len got set, finding it prefereable to: >>>slash = filename ? NULL : strrchr (filename, '/'); >>> but if you prefer to avoid the assignment in the conditional I'm fine >>> with that too. >> >> Yeah, please don't do an assignment in a conditional. >> >> Why don't we just pass "" instead of NULL in the call to elf_add? If >> that works, that is OK. >> > > With this refactoring preamble ... ... I can more easily add a test-case btest_dwz_gnudebuglink, which triggers a segfault due to strrchr being called with a NULL string. This patch fixes it by passing "" instead of NULL, in the call to elf_add at line 3083 (for .gnu_debugaltlink), not the call to elf_add at line 3044 (for .gnu_debuglink) mentioned above. Nathan, does this fix the problem for you? If not, can you provide a reproducer, or give a hint on how one could be constructed? Thanks, - Tom [libbacktrace] Fix strrchr segfault Currently, when handling a libbacktrace testcase t with .gnu_debuglink to t.debug, and t.debug having a .gnu_debugaltlink to t.alt.debug, an segfault is triggered in when calling strrchr with a NULL string from elf_find_debugfile_by_debuglink. The NULL string originates from the elf_add called for the .gnu_debugaltlink, which uses NULL as filename argument. Fix this by using "" as filename argument instead. 2019-01-25 Tom de Vries * elf.c (elf_add): When handling .gnu_debugaltlink, Call elf_add with filename == "". * Makefile.am (TESTS): Add btest_dwz_gnudebuglink. * Makefile.in: Regenerate. --- libbacktrace/Makefile.am | 6 ++ libbacktrace/Makefile.in | 22 +++--- libbacktrace/elf.c | 2 +- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/libbacktrace/Makefile.am b/libbacktrace/Makefile.am index 2f1d9af89c3..997a535dff4 100644 --- a/libbacktrace/Makefile.am +++ b/libbacktrace/Makefile.am @@ -190,6 +190,12 @@ if HAVE_DWZ TESTS += btest_dwz +if HAVE_OBJCOPY_DEBUGLINK + +TESTS += btest_dwz_gnudebuglink + +endif HAVE_OBJCOPY_DEBUGLINK + endif HAVE_DWZ stest_SOURCES = stest.c diff --git a/libbacktrace/Makefile.in b/libbacktrace/Makefile.in index 0b73e3d6981..f04577066f8 100644 --- a/libbacktrace/Makefile.in +++ b/libbacktrace/Makefile.in @@ -127,11 +127,12 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) @NATIVE_TRUE@ ztest ztest_alloc edtest edtest_alloc @NATIVE_TRUE@am__append_2 = allocfail.sh @HAVE_DWZ_TRUE@@NATIVE_TRUE@am__append_3 = btest_dwz -@HAVE_ZLIB_TRUE@@NATIVE_TRUE@am__append_4 = -lz +@HAVE_DWZ_TRUE@@HAVE_OBJCOPY_DEBUGLINK_TRUE@@NATIVE_TRUE@am__append_4 = btest_dwz_gnudebuglink @HAVE_ZLIB_TRUE@@NATIVE_TRUE@am__append_5 = -lz -@HAVE_PTHREAD_TRUE@@NATIVE_TRUE@am__append_6 = ttest ttest_alloc -@HAVE_OBJCOPY_DEBUGLINK_TRUE@@NATIVE_TRUE@am__append_7 = btest_gnudebuglink -@HAVE_COMPRESSED_DEBUG_TRUE@@NATIVE_TRUE@am__append_8 = ctestg ctesta \ +@HAVE_ZLIB_TRUE@@NATIVE_TRUE@am__append_6 = -lz +@HAVE_PTHREAD_TRUE@@NATIVE_TRUE@am__append_7 = ttest ttest_alloc +@HAVE_OBJCOPY_DEBUGLINK_TRUE@@NATIVE_TRUE@am__append_8 = btest_gnudebuglink +@HAVE_COMPRESSED_DEBUG_TRUE@@NATIVE_TRUE@am__append_9 = ctestg ctesta \ @HAVE_COMPRESSED_DEBUG_TRUE@@NATIVE_TRUE@ ctestg_alloc \ @HAVE_COMPRESSED_DEBUG_TRUE@@NATIVE_TRUE@ ctesta_alloc subdir = . @@ -789,7 +790,7 @@ libbacktrace_la_LIBADD = \ libbacktrace_la_DEPENDENCIES = $(libbacktrace_la_LIBADD) TESTS = $(check_PROGRAMS) $(am__append_2) $(am__append_3) \ - $(am__append_7) + $(am__append_4) $(am__append_8) @NATIVE_TRUE@check_LTLIBRARIES = libbacktrace_alloc.la \ @NATIVE_TRUE@ libbacktrace_noformat.la \ @NATIVE_TRUE@ libbacktrace_instrumented_alloc.la @@ -834,9 +835,9 @@ TESTS = $(check_PROGRAMS) $(am__append_2) $(am__append_3) \ @NATIVE_TRUE@stest_alloc_LDADD = libbacktrace_alloc.la @NATIVE_TRUE@ztest_SOURCES = ztest.c testlib.c @NATIVE_TRUE@ztest_CFLAGS = -DSRCDIR=\"$(srcdir)\" -@NATIVE_TRUE@ztest_LDADD = libbacktrace.la $(am__append_4) \ +@NATIVE_TRUE@ztest_LDADD = libbacktrace.la $(am__append_5) \ @NATIVE_TRUE@ $(CLOCK_GETTIME_LINK)
Re: PING [PATCH] tighten up -Wbuiltin-declaration-mismatch (PR 86125, 88886, 86308)
On Fri, 25 Jan 2019 at 06:40, Jakub Jelinek wrote: > > On Wed, Jan 23, 2019 at 11:41:55PM +, Joseph Myers wrote: > > On Wed, 23 Jan 2019, Martin Sebor wrote: > > > > > Ping: https://gcc.gnu.org/ml/gcc-patches/2019-01/msg00969.html > > > > This patch is OK. > > Note, the testcase FAILs on i686-linux: > +FAIL: gcc.dg/Wbuiltin-declaration-mismatch-7.c (test for warnings, line 20) > size_t is unsigned int there I believe. > On aarch64: /gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch-7.c:16:5: warning: conflicting types for built-in function 'vfprintf'; expected 'int(void *, const char *, __va_list)' [-Wbuiltin-declaration-mismatch] /gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch-7.c:24:5: warning: conflicting types for built-in function 'vfscanf'; expected 'int(void *, const char *, __va_list)' [-Wbuiltin-declaration-mismatch] Both errors are also present on arm, as well as: FAIL: gcc.dg/Wbuiltin-declaration-mismatch-7.c (test for warnings, line 20) (for lack of warning apparently) > Jakub
Re: [backtrace] Avoid segfault
On 25-01-19 01:51, Ian Lance Taylor wrote: > On Thu, Jan 24, 2019 at 4:11 PM Nathan Sidwell wrote: >> >> I just tripped over a segfault in libbacktrace. We apply strrchr to a >> possibly NULL filename, with predictable results when it is. >> >> elf.c:3044 passes NULL as the filename parm: >> ret = elf_add (state, NULL, d, base_address, error_callback, data, >> fileline_fn, found_sym, found_dwarf, NULL, 0, 1, >> NULL, >> 0); >> >> This gets to elf_open_debugfile_by_debuglink which passes it on through: >>ddescriptor = elf_find_debugfile_by_debuglink (state, filename, >> debuglink_name, >> error_callback, data); >> >> this patch avoids the strrchr when filename is null. I reordered the >> way prefix & prefix len got set, finding it prefereable to: >>slash = filename ? NULL : strrchr (filename, '/'); >> but if you prefer to avoid the assignment in the conditional I'm fine >> with that too. > > Yeah, please don't do an assignment in a conditional. > > Why don't we just pass "" instead of NULL in the call to elf_add? If > that works, that is OK. > With this refactoring preamble ... [libbacktrace] Rename dtest to btest_gnudebuglink Create a pattern rule for copying an existing test-case, separating out the debug information into a .debug file, and referencing the .debug file from the copied test-case using a .gnu_debuglink. 2019-01-25 Tom de Vries * Makefile.am: Rewrite dtest rule into "%_gnudebuglink" pattern rule. (TESTS): Rename dtest to btest_gnudebuglink. * Makefile.in: Regenerate. --- libbacktrace/Makefile.am | 8 libbacktrace/Makefile.in | 14 +++--- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/libbacktrace/Makefile.am b/libbacktrace/Makefile.am index bf90ebdb2d5..2f1d9af89c3 100644 --- a/libbacktrace/Makefile.am +++ b/libbacktrace/Makefile.am @@ -255,11 +255,11 @@ endif HAVE_PTHREAD if HAVE_OBJCOPY_DEBUGLINK -TESTS += dtest +TESTS += btest_gnudebuglink -dtest: btest - $(OBJCOPY) --only-keep-debug btest btest.debug - $(OBJCOPY) --strip-debug --add-gnu-debuglink=btest.debug btest dtest +%_gnudebuglink: % + $(OBJCOPY) --only-keep-debug $< $@.debug + $(OBJCOPY) --strip-debug --add-gnu-debuglink=$@.debug $< $@ endif HAVE_OBJCOPY_DEBUGLINK diff --git a/libbacktrace/Makefile.in b/libbacktrace/Makefile.in index d55e0501171..0b73e3d6981 100644 --- a/libbacktrace/Makefile.in +++ b/libbacktrace/Makefile.in @@ -130,7 +130,7 @@ check_PROGRAMS = $(am__EXEEXT_1) $(am__EXEEXT_2) $(am__EXEEXT_3) @HAVE_ZLIB_TRUE@@NATIVE_TRUE@am__append_4 = -lz @HAVE_ZLIB_TRUE@@NATIVE_TRUE@am__append_5 = -lz @HAVE_PTHREAD_TRUE@@NATIVE_TRUE@am__append_6 = ttest ttest_alloc -@HAVE_OBJCOPY_DEBUGLINK_TRUE@@NATIVE_TRUE@am__append_7 = dtest +@HAVE_OBJCOPY_DEBUGLINK_TRUE@@NATIVE_TRUE@am__append_7 = btest_gnudebuglink @HAVE_COMPRESSED_DEBUG_TRUE@@NATIVE_TRUE@am__append_8 = ctestg ctesta \ @HAVE_COMPRESSED_DEBUG_TRUE@@NATIVE_TRUE@ ctestg_alloc \ @HAVE_COMPRESSED_DEBUG_TRUE@@NATIVE_TRUE@ ctesta_alloc @@ -1585,9 +1585,9 @@ btest_dwz.log: btest_dwz --log-file $$b.log --trs-file $$b.trs \ $(am__common_driver_flags) $(AM_LOG_DRIVER_FLAGS) $(LOG_DRIVER_FLAGS) -- $(LOG_COMPILE) \ "$$tst" $(AM_TESTS_FD_REDIRECT) -dtest.log: dtest - @p='dtest'; \ - b='dtest'; \ +btest_gnudebuglink.log: btest_gnudebuglink + @p='btest_gnudebuglink'; \ + b='btest_gnudebuglink'; \ $(am__check_pre) $(LOG_DRIVER) --test-name "$$f" \ --log-file $$b.log --trs-file $$b.trs \ $(am__common_driver_flags) $(AM_LOG_DRIVER_FLAGS) $(LOG_DRIVER_FLAGS) -- $(LOG_COMPILE) \ @@ -1767,9 +1767,9 @@ uninstall-am: @NATIVE_TRUE@ $(SHELL) $(srcdir)/../move-if-change tmp-edtest2_build.c edtest2_build.c @NATIVE_TRUE@ echo timestamp > $@ -@HAVE_OBJCOPY_DEBUGLINK_TRUE@@NATIVE_TRUE@dtest: btest -@HAVE_OBJCOPY_DEBUGLINK_TRUE@@NATIVE_TRUE@ $(OBJCOPY) --only-keep-debug btest btest.debug -@HAVE_OBJCOPY_DEBUGLINK_TRUE@@NATIVE_TRUE@ $(OBJCOPY) --strip-debug --add-gnu-debuglink=btest.debug btest dtest +@HAVE_OBJCOPY_DEBUGLINK_TRUE@@NATIVE_TRUE@%_gnudebuglink: % +@HAVE_OBJCOPY_DEBUGLINK_TRUE@@NATIVE_TRUE@ $(OBJCOPY) --only-keep-debug $< $@.debug +@HAVE_OBJCOPY_DEBUGLINK_TRUE@@NATIVE_TRUE@ $(OBJCOPY) --strip-debug --add-gnu-debuglink=$@.debug $< $@ alloc.lo: config.h backtrace.h internal.h backtrace.lo: config.h backtrace.h internal.h btest.lo: (INCDIR)/filenames.h backtrace.h backtrace-supported.h
Re: Fortran vector math header
On 1/24/19 10:24 PM, Steve Ellcey wrote:
> On Thu, 2019-01-24 at 16:25 +0100, Martin Liška wrote:
>>
>>>
+ix86_get_multilib_abi_name (void)
+{
+ if (!(TARGET_64BIT_P (ix86_isa_flags)))
+return "i386";
+ else if (TARGET_X32_P (ix86_isa_flags))
+return "x32";
+ else
+return "x86_64";
+}
>
>
> I'd like to get an aarch64 version of this target function into GCC 9.*
> too in order to support ILP32/LP64 on aarch64. It doesn't necessarily
> have to be part of this patch though, I could submit one later if you
> do not want to add it here.
Sure, feel free to provide a patch candidate. Once the patch is installed,
I'm planning to document format of the pre-include header file.
Martin
>
> Aarch64 ILP32 support is in GCC and binutils but not GLIBC (except on a
> branch), I am thinking the Aarch64 version of this function would
> return "aarch64" or "aarch64_ilp32". Perhaps we should also have
> "aarch64_be" and "aarch64_be_ilp32" for big endian ABI's?
>
> Steve Ellcey
> sell...@marvell.com
>
[PATCH] Fix PR87295
The following fixes an ICE with -flto -ffat-lto-objects
-fdebug-types-section -g where optimize_external_refs does not
expect to see DW_AT_signature as made "local" by build_abbrev_table(sic!).
This is because we run optimize_external_refs twice in this setup.
The fix is to make the second run not pick up "local" DW_AT_signature
as external. Alternatively build_abbrev_table could be adjusted
to not keep those as DW_AT_signature. It's not even clear to me
whether a DW_AT_signature as we output it is valid DWARF ...
to quote non-LTO -g:
<1><1d>: Abbrev Number: 13 (DW_TAG_structure_type)
<1e> DW_AT_name: (indirect string, offset: 0x8b):
integral_constant
<22> DW_AT_signature : <0x5d>
<26> DW_AT_declaration : 1
<26> DW_AT_sibling : <0x48>
...
<1><5d>: Abbrev Number: 16 (DW_TAG_structure_type)
<5e> DW_AT_name: (indirect string, offset: 0x8b):
integral_constant
<62> DW_AT_signature : signature: 0x1f6a4ae7cc5a362e
<6a> DW_AT_declaration : 1
<6a> DW_AT_sibling : <0x7b>
13 DW_TAG_structure_type[has children]
DW_AT_name DW_FORM_strp
DW_AT_signatureDW_FORM_ref4
16 DW_TAG_structure_type[has children]
DW_AT_name DW_FORM_strp
DW_AT_signatureDW_FORM_ref_sig8
Using DW_AT_specification (as other code in dwarf seems to try
doing) seems fishy as well given the refering DIE is
DW_AT_declaration...
The DWARF spec doesn't seem to explicitely list an appropriate
refering attribute type that is applicable here. My favorite
kitchen-sink DW_AT_abstract_origin might count ;)
Of ocurse the main "issue" is that copy_unworthy_types
duplicated this parent into 1d and 5d in the first place
(but that's a pure optimization issue?). Not doing that
would have avoided the situation in PR87295. But then
why should optimize_external_refs_1 have this special
code handling DW_AT_signature in the first place if this
was not intended... (so even better, kill that and the
build_abbrev_table optimization and fix copy_unworthy_types?)
Oh, and insert rants of -fdebug-types-section not being
used by anybody.
Bootstrap & regtest running on x86_64-unknown-linux-gnu, OK?
Thanks,
Richard.
2019-01-25 Richard Biener
PR debug/87295
* dwarf2out.c (optimize_external_refs_1): Do not pick up
already optimized DW_AT_signature refs.
* g++.dg/lto/pr87295_0.C: New testcase.
Index: gcc/dwarf2out.c
===
--- gcc/dwarf2out.c (revision 268260)
+++ gcc/dwarf2out.c (working copy)
@@ -8911,7 +8911,11 @@ optimize_external_refs_1 (dw_die_ref die
struct external_ref *ref_p;
if (is_type_die (die)
- && (c = get_AT_ref (die, DW_AT_signature)))
+ && (c = get_AT_ref (die, DW_AT_signature))
+ /* Make sure to not pick up optimized local refs from the
+ early LTO run of build_abbrev_table which performs the
+actual redirection. */
+ && c->comdat_type_p)
{
/* This is a local skeleton; use it for local references. */
ref_p = lookup_external_ref (map, c);
Index: gcc/testsuite/g++.dg/lto/pr87295_0.C
===
--- gcc/testsuite/g++.dg/lto/pr87295_0.C(nonexistent)
+++ gcc/testsuite/g++.dg/lto/pr87295_0.C(working copy)
@@ -0,0 +1,20 @@
+// { dg-lto-do assemble }
+// { dg-lto-options { { -flto -ffat-lto-objects -fdebug-types-section -g
-std=gnu++17 } } }
+
+template
+struct integral_constant
+{
+ static constexpr _Tp value = __v;
+ typedef _Tp value_type;
+ constexpr operator value_type() const noexcept { return value; }
+};
+
+typedef integral_constant false_type;
+
+template
+struct __or_;
+
+template<>
+struct __or_<>
+ : public false_type
+{ };
[PATCH] Fix vector cost issue noted in PR89049
We were blindly costing all scalar stmts that appear in patterns even if those are not relevant for vectorization. Fixed as follows. Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk. Richard. 2019-01-25 Richard Biener PR tree-optimization/89049 * tree-vect-loop.c (vect_compute_single_scalar_iteration_cost): Look at the pattern stmt to determine if the stmt is vectorized. Index: gcc/tree-vect-loop.c === --- gcc/tree-vect-loop.c(revision 268257) +++ gcc/tree-vect-loop.c(working copy) @@ -1100,11 +1100,11 @@ vect_compute_single_scalar_iteration_cos continue; /* Skip stmts that are not vectorized inside the loop. */ - if (stmt_info - && !STMT_VINFO_RELEVANT_P (stmt_info) - && (!STMT_VINFO_LIVE_P (stmt_info) - || !VECTORIZABLE_CYCLE_DEF (STMT_VINFO_DEF_TYPE (stmt_info))) - && !STMT_VINFO_IN_PATTERN_P (stmt_info)) + stmt_vec_info vstmt_info = vect_stmt_to_vectorize (stmt_info); + if (!STMT_VINFO_RELEVANT_P (vstmt_info) + && (!STMT_VINFO_LIVE_P (vstmt_info) + || !VECTORIZABLE_CYCLE_DEF + (STMT_VINFO_DEF_TYPE (vstmt_info continue; vect_cost_for_stmt kind;
[SVE ACLE] Fix a typo and dot_prod_optab interface
I've applied the patches below to aarch64/sve-acle-branch. The first
fixes the modes and oerand order of the dot_prod patterns, and the second
fixes a typo in one of the vector duplicate patterns.
Thanks,
Richard
[SVE ACLE] Fix sdot/udot modes
This patch makes the dot_prod patterns have the mode and operand
order expected by the optabs. I'll leave proper use of the patterns
in autovec for later, since it's not really ACLE work.
diff --git a/gcc/config/aarch64/aarch64-sve-builtins.c b/gcc/config/aarch64/aarch64-sve-builtins.c
index 0e3db669422..ed06db9b7c6 100644
--- a/gcc/config/aarch64/aarch64-sve-builtins.c
+++ b/gcc/config/aarch64/aarch64-sve-builtins.c
@@ -501,7 +501,7 @@ private:
rtx expand_signed_pred_op (rtx_code, rtx_code, int,
unsigned int = DEFAULT_MERGE_ARGNO);
rtx expand_signed_pred_op (int, int, int);
- rtx expand_via_unpred_direct_optab (optab);
+ rtx expand_via_unpred_direct_optab (optab, machine_mode = VOIDmode);
rtx expand_via_unpred_insn (insn_code);
rtx expand_via_pred_direct_optab (optab, unsigned int = DEFAULT_MERGE_ARGNO);
rtx expand_via_sel_insn (insn_code);
@@ -512,9 +512,11 @@ private:
void require_immediate_range (unsigned int, HOST_WIDE_INT, HOST_WIDE_INT);
+ void rotate_inputs_left (unsigned int, unsigned int);
bool try_negating_argument (unsigned int, machine_mode);
machine_mode get_mode (unsigned int);
+ machine_mode get_quarter_mode (unsigned int);
machine_mode get_pred_mode (unsigned int);
rtx get_fallback_value (machine_mode, unsigned int,
unsigned int, unsigned int &);
@@ -2127,10 +2129,14 @@ function_expander::expand_div (bool reversed_p)
rtx
function_expander::expand_dot ()
{
+ /* In the optab, the multiplication operands come before the
+ accumulator operand. */
+ rotate_inputs_left (0, 3);
+ machine_mode mode = get_quarter_mode (0);
if (type_suffixes[m_fi.types[0]].unsigned_p)
-return expand_via_unpred_direct_optab (udot_prod_optab);
+return expand_via_unpred_direct_optab (udot_prod_optab, mode);
else
-return expand_via_unpred_direct_optab (sdot_prod_optab);
+return expand_via_unpred_direct_optab (sdot_prod_optab, mode);
}
/* Expand a call to svdup. */
@@ -2264,10 +2270,7 @@ function_expander::expand_msb (unsigned int merge_argno)
rtx
function_expander::expand_mla ()
{
- rtx t = m_args[1];
- m_args[1] = m_args[2];
- m_args[2] = m_args[3];
- m_args[3] = t;
+ rotate_inputs_left (1, 4);
return expand_mad (3);
}
@@ -2282,10 +2285,7 @@ function_expander::expand_mla ()
rtx
function_expander::expand_mls ()
{
- rtx t = m_args[1];
- m_args[1] = m_args[2];
- m_args[2] = m_args[3];
- m_args[3] = t;
+ rotate_inputs_left (1, 4);
return expand_msb (3);
}
@@ -2401,11 +2401,14 @@ function_expander::expand_sub (bool reversed_p)
}
/* Implement the call using optab OP, which is an unpredicated direct
- (i.e. single-mode) optab. */
+ (i.e. single-mode) optab. MODE is the mode of the operation,
+ or VOIDmode if the mode associated with type suffix 0 is correct. */
rtx
-function_expander::expand_via_unpred_direct_optab (optab op)
+function_expander::expand_via_unpred_direct_optab (optab op,
+ machine_mode mode)
{
- machine_mode mode = get_mode (0);
+ if (mode == VOIDmode)
+mode = get_mode (0);
insn_code icode = direct_optab_handler (op, mode);
return expand_via_unpred_insn (icode);
}
@@ -2664,6 +2667,17 @@ function_expander::require_immediate_range (unsigned int argno,
m_args[argno] = GEN_INT (min);
}
+/* Rotate inputs m_args[START:END] one position to the left, so that
+ m_args[START] becomes m_args[END - 1]. */
+void
+function_expander::rotate_inputs_left (unsigned int start, unsigned int end)
+{
+ rtx new_last = m_args[start];
+ for (unsigned int i = start; i < end - 1; ++i)
+m_args[i] = m_args[i + 1];
+ m_args[end - 1] = new_last;
+}
+
/* Return true if argument I is a constant argument that can be negated
at compile time, replacing it with the negated value if so. MODE is the
associated vector mode, but the argument could be a single element. */
@@ -2689,6 +2703,14 @@ function_expander::get_mode (unsigned int i)
return TYPE_MODE (m_fi.vector_type (i));
}
+/* Return the vector mode for elements that are a quarter the size of integer
+ type suffix I. */
+machine_mode
+function_expander::get_quarter_mode (unsigned int i)
+{
+ return TYPE_MODE (m_fi.quarter_vector_type (i));
+}
+
/* Return the predicate mode associated with type suffix I. */
machine_mode
function_expander::get_pred_mode (unsigned int i)
diff --git a/gcc/config/aarch64/aarch64-sve.md b/gcc/config/aarch64/aarch64-sve.md
index 47a94882ea2..b08029085b5 100644
--- a/gcc/config/aarch64/aarch64-sve.md
+++ b/gcc/config/aarch64/aarch64-sve.md
@@ -3904,15 +3904,15 @@
)
;; Unpredicated DOT product.
-(define_insn "dot_prod"
+(define_insn "dot_prod"
[(set (match_operand:SVE_SDI 0 "register_operand" "=w, ?")
- (plus:SVE_SDI
[AArch64][SVE] Handle register-register pred_movs
pred_mov is defined for predicated loads and stores, where exactly one of the operands is a register. However, the instruction condition only checked for "one" rather than "exactly one", and Prathamesh found a case in which combine could fold a predicated pattern to an all-register pred_mov. The constraints would then force one of the registers to memory. This patch splits all-register forms into a normal move as soon as possible, but also adds an all-register alternative in case the instruction doesn't get split before RA (or in case the RA can use inheritance to avoid a reload). The testcase for this will be added to aarch64/sve-acle-branch. Tested on aarch64-linux-gnu (with and without SVE) and aarch64_be-elf. Applied to trunk and aarch64/sve-acle-branch. Richard 2018-01-25 Richard Sandiford gcc/ * config/aarch64/aarch64-sve.md (*pred_mov) (pred_mov): Handle all-register forms using both a new alternative and a split. Index: gcc/config/aarch64/aarch64-sve.md === --- gcc/config/aarch64/aarch64-sve.md 2019-01-11 10:55:28.536599178 + +++ gcc/config/aarch64/aarch64-sve.md 2019-01-25 12:16:19.489967684 + @@ -170,18 +170,22 @@ (define_expand "aarch64_sve_reload_be" ;; all-true. Note that this pattern is generated directly by ;; aarch64_emit_sve_pred_move, so changes to this pattern will ;; need changes there as well. -(define_insn "*pred_mov" - [(set (match_operand:SVE_ALL 0 "nonimmediate_operand" "=w, m") +(define_insn_and_split "*pred_mov" + [(set (match_operand:SVE_ALL 0 "nonimmediate_operand" "=w, w, m") (unspec:SVE_ALL - [(match_operand: 1 "register_operand" "Upl, Upl") - (match_operand:SVE_ALL 2 "nonimmediate_operand" "m, w")] + [(match_operand: 1 "register_operand" "Upl, Upl, Upl") + (match_operand:SVE_ALL 2 "nonimmediate_operand" "w, m, w")] UNSPEC_MERGE_PTRUE))] "TARGET_SVE && (register_operand (operands[0], mode) || register_operand (operands[2], mode))" "@ + # ld1\t%0., %1/z, %2 st1\t%2., %1, %0" + "&& register_operand (operands[0], mode) + && register_operand (operands[2], mode)" + [(set (match_dup 0) (match_dup 2))] ) (define_expand "movmisalign" @@ -401,10 +405,10 @@ (define_split ;; Predicated structure moves. This works for both endiannesses but in ;; practice is only useful for big-endian. (define_insn_and_split "pred_mov" - [(set (match_operand:SVE_STRUCT 0 "aarch64_sve_struct_nonimmediate_operand" "=w, Utx") + [(set (match_operand:SVE_STRUCT 0 "aarch64_sve_struct_nonimmediate_operand" "=w, w, Utx") (unspec:SVE_STRUCT - [(match_operand: 1 "register_operand" "Upl, Upl") - (match_operand:SVE_STRUCT 2 "aarch64_sve_struct_nonimmediate_operand" "Utx, w")] + [(match_operand: 1 "register_operand" "Upl, Upl, Upl") + (match_operand:SVE_STRUCT 2 "aarch64_sve_struct_nonimmediate_operand" "w, Utx, w")] UNSPEC_MERGE_PTRUE))] "TARGET_SVE && (register_operand (operands[0], mode)
Re: undefined references to mkdir, etc in libstdc++ testing
On 24/01/19 17:14 -0700, Sandra Loosemore wrote: On 1/24/19 1:20 PM, Jonathan Wakely wrote: On 24/01/19 11:53 -0700, Sandra Loosemore wrote: BTW, I'm more worried about the link errors introduced by the patch for PR 86756. Those are regressions and apparently a problem that could affect user code, not just broken test cases for half-baked new features. If I understand correctly(*) it can only affect user code that uses the std::filesystem library, which was new in GCC 8.1, and if those tests are failing for nios2-elf then it was never usable anyway. The tests run by default now, but previously they were only run if gcc was configured with --enable-libstdcxx-filesystem-ts, so this isn't really a regression in what users can do with the library. I agree they're more important than the experimental/net/ tests though. (*) only a few of the 27_io/filesystem/path tests are showing these linker errors, right? Or are tests outside 27_io/filesystem also affected? Other tests are affected. This appears to be the full list: FAIL: 19_diagnostics/error_code/cons/39882.cc (test for excess errors) FAIL: 19_diagnostics/error_code/modifiers/39882.cc (test for excess errors) FAIL: 19_diagnostics/error_condition/cons/39881.cc (test for excess errors) FAIL: 19_diagnostics/error_condition/modifiers/39881.cc (test for excess errors) FAIL: 19_diagnostics/system_error/cons_virtual_derivation.cc (test for excess errors) FAIL: 20_util/hash/operators/size_t.cc (test for excess errors) FAIL: 27_io/filesystem/operations/all.cc (test for excess errors) FAIL: 27_io/filesystem/operations/resize_file.cc (test for excess errors) FAIL: 27_io/filesystem/path/generation/normal2.cc (test for excess errors) FAIL: experimental/net/buffer/arithmetic.cc (test for excess errors) FAIL: experimental/net/buffer/const.cc (test for excess errors) FAIL: experimental/net/buffer/creation.cc (test for excess errors) FAIL: experimental/net/buffer/mutable.cc (test for excess errors) FAIL: experimental/net/buffer/size.cc (test for excess errors) In GCC 8 the 19_diagnostics and 20_util tests in that list PASSed. The other tests are new in GCC 9, but in GCC 8 all the other 27_io/filesystem tests showed up as UNSUPPORTED on this target. BTW, here's a snippet from the link map for the first FAIL on that list. AFAICT there is nothing that references the libstdc++ functions that call mkdir, etc; those functions are only getting sucked into the link because of a reference to some template function in that compilation unit. /scratch/sandra/nios2-elf-fsf-gcc/install/opt/codesourcery/bin/../lib/gcc/nios2-elf/9.0.0/../../../../nios2-elf/lib/libstdc++.a(fs_ops.o) /scratch/sandra/nios2-elf-fsf-gcc/install/opt/codesourcery/bin/../lib/gcc/nios2-elf/9.0.0/../../../../nios2-elf/lib/libstdc++.a(system_error.o) (void std::__cxx11::basic_string, std::allocator >::_M_construct(char const*, char const*, std::forward_iterator_tag)) Thanks. I wonder if this is caused by using -fimplicit-templates in src/c++17/Makefile.
Fix output_constructor_bitfield handling of wide bitfields (PR89037)
The testcase was failing because we were trying to access
TREE_INT_CST_ELT (x, 1) of a 128-bit integer that was small enough
to need only a single element.
Tested on aarch64-linux-gnu, aarch64_be-elf and x86_64-linux-gnu.
OK to install?
Richard
2019-01-25 Richard Sandiford
gcc/
PR middle-end/89037
* varasm.c (output_constructor_bitfield): Use wi::extract_uhwi
instead of accessing TREE_INT_CST_ELT directly.
gcc/testsuite/
PR middle-end/89037
* gcc.dg/pr89037.c: New test.
Index: gcc/varasm.c
===
--- gcc/varasm.c2019-01-04 11:39:27.182246717 +
+++ gcc/varasm.c2019-01-25 12:10:23.969006336 +
@@ -5349,7 +5349,7 @@ output_constructor_bitfield (oc_local_st
{
int this_time;
int shift;
- HOST_WIDE_INT value;
+ unsigned HOST_WIDE_INT value;
HOST_WIDE_INT next_byte = next_offset / BITS_PER_UNIT;
HOST_WIDE_INT next_bit = next_offset % BITS_PER_UNIT;
@@ -5381,15 +5381,13 @@ output_constructor_bitfield (oc_local_st
this_time = end - shift + 1;
}
- /* Now get the bits from the appropriate constant word. */
- value = TREE_INT_CST_ELT (local->val, shift / HOST_BITS_PER_WIDE_INT);
- shift = shift & (HOST_BITS_PER_WIDE_INT - 1);
+ /* Now get the bits we want to insert. */
+ value = wi::extract_uhwi (wi::to_widest (local->val),
+ shift, this_time);
/* Get the result. This works only when:
1 <= this_time <= HOST_BITS_PER_WIDE_INT. */
- local->byte |= (((value >> shift)
- & (((HOST_WIDE_INT) 2 << (this_time - 1)) - 1))
- << (BITS_PER_UNIT - this_time - next_bit));
+ local->byte |= value << (BITS_PER_UNIT - this_time - next_bit);
}
else
{
@@ -5406,15 +5404,13 @@ output_constructor_bitfield (oc_local_st
this_time
= HOST_BITS_PER_WIDE_INT - (shift & (HOST_BITS_PER_WIDE_INT - 1));
- /* Now get the bits from the appropriate constant word. */
- value = TREE_INT_CST_ELT (local->val, shift / HOST_BITS_PER_WIDE_INT);
- shift = shift & (HOST_BITS_PER_WIDE_INT - 1);
+ /* Now get the bits we want to insert. */
+ value = wi::extract_uhwi (wi::to_widest (local->val),
+ shift, this_time);
/* Get the result. This works only when:
1 <= this_time <= HOST_BITS_PER_WIDE_INT. */
- local->byte |= (((value >> shift)
- & (((HOST_WIDE_INT) 2 << (this_time - 1)) - 1))
- << next_bit);
+ local->byte |= value << next_bit;
}
next_offset += this_time;
Index: gcc/testsuite/gcc.dg/pr89037.c
===
--- /dev/null 2019-01-24 08:42:49.147091464 +
+++ gcc/testsuite/gcc.dg/pr89037.c 2019-01-25 12:10:23.965006370 +
@@ -0,0 +1,24 @@
+/* { dg-do run { target int128 } } */
+/* { dg-options "" } */
+
+struct s
+{
+ __int128 y : 66;
+};
+typedef struct s T;
+T a[] = { 1, 1, 0x12345, 0xff01, 1ULL << 63, (__int128) 1 << 64,
+ ((__int128) 1 << 64) | 1 };
+
+int
+main (void)
+{
+ if (a[0].y != 1
+ || a[1].y != 1
+ || a[2].y != 0x12345
+ || a[3].y != 0xff01
+ || a[4].y != (1ULL << 63)
+ || a[5].y != ((__int128) 1 << 64)
+ || a[6].y != (((__int128) 1 << 64) | 1))
+__builtin_abort ();
+ return 0;
+}
[PATCH] libgfortran: Use proper gthr.h API
libgfortran/
* io/async.c (init_adv_cond): Use
__GTHREAD_COND_INIT_FUNCTION().
---
libgfortran/io/async.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libgfortran/io/async.c b/libgfortran/io/async.c
index 3394e595a8b..e3d1d01122e 100644
--- a/libgfortran/io/async.c
+++ b/libgfortran/io/async.c
@@ -224,7 +224,7 @@ init_adv_cond (struct adv_cond *ac)
{
ac->pending = 0;
__GTHREAD_MUTEX_INIT_FUNCTION (>lock);
- __gthread_cond_init_function (>signal);
+ __GTHREAD_COND_INIT_FUNCTION (>signal);
}
/* Initialize an asyncronous unit, returning zero on success,
--
2.16.4
Re: [C++ Patch] PR 88969 ("[9 Regression] ICE in build_op_delete_call, at cp/call.c:6509")
Hi,
On 24/01/19 23:21, Jason Merrill wrote:
On 1/24/19 2:53 PM, Paolo Carlini wrote:
Hi,
as far as I can see this ICE on invalid points to a substantive, if
minor, weakness of our implementation of the destroying operator
delete facility: we aren't implementing the bits, per 7.6.2.5/(10.1),
about destroying operator delete having precedence over any other
operator delete. Thus the below, which is the most straightforward
implementation I have been able to figure out given the current
infrastructure. Tested x86_64-linux.
OK, thanks.
Thanks you.
Yesterday I didn't notice that the bug report includes another testcase,
for an unrelated buglet: if the destroying operator delete is wrongly
specified as not-taking a pointer to the class type as first argument,
TYPE_POINTER_TO may not be set yet in coerce_delete_type and we crash
later on. Thus the below, which changes it to build_pointer_type.
Thanks, Paolo.
//
/cp
2019-01-25 Paolo Carlini
PR c++/88969
* call.c (build_op_delete_call): Implement 7.6.2.5/(10.1).
* decl2.c (coerce_delete_type): Use build_pointer_type instead
of TYPE_POINTER_TO.
/testsuite
2019-01-25 Paolo Carlini
PR c++/88969
* g++.dg/cpp2a/destroying-delete2.C: New.
* g++.dg/cpp2a/destroying-delete3.C: Likewise.
Index: cp/call.c
===
--- cp/call.c (revision 268257)
+++ cp/call.c (working copy)
@@ -6461,6 +6461,19 @@ build_op_delete_call (enum tree_code code, tree ad
continue;
}
+ /* -- If any of the deallocation functions is a destroying
+ operator delete, all deallocation functions that are not
+ destroying operator deletes are eliminated from further
+ consideration. */
+ bool fn_destroying = destroying_delete_p (fn);
+ bool elt_destroying = destroying_delete_p (elt);
+ if (elt_destroying != fn_destroying)
+ {
+ if (elt_destroying)
+ fn = elt;
+ continue;
+ }
+
/* -- If the type has new-extended alignment, a function with a
parameter of type std::align_val_t is preferred; otherwise a
function without such a parameter is preferred. If exactly one
Index: cp/decl2.c
===
--- cp/decl2.c (revision 268257)
+++ cp/decl2.c (working copy)
@@ -1757,9 +1757,9 @@ coerce_delete_type (tree decl, location_t loc)
if (destroying_delete_p (decl))
{
if (DECL_CLASS_SCOPE_P (decl))
- /* If the function is a destroying operator delete declared in class
type
- C, the type of its first parameter shall be C*. */
- ptrtype = TYPE_POINTER_TO (DECL_CONTEXT (decl));
+ /* If the function is a destroying operator delete declared in class
+ type C, the type of its first parameter shall be C*. */
+ ptrtype = build_pointer_type (DECL_CONTEXT (decl));
else
/* A destroying operator delete shall be a class member function named
operator delete. */
Index: testsuite/g++.dg/cpp2a/destroying-delete2.C
===
--- testsuite/g++.dg/cpp2a/destroying-delete2.C (nonexistent)
+++ testsuite/g++.dg/cpp2a/destroying-delete2.C (working copy)
@@ -0,0 +1,20 @@
+// PR c++/88969
+// { dg-do compile { target c++2a } }
+
+#include
+
+namespace delete_selection_d {
+ struct B {
+void operator delete(void*) = delete;
+void operator delete(B *, std::destroying_delete_t) = delete; // {
dg-message "declared here" }
+ };
+ void delete_B(B *b) { delete b; } // { dg-error "use of deleted function" }
+}
+
+namespace delete_selection_r {
+ struct B {
+void operator delete(B *, std::destroying_delete_t) = delete; // {
dg-message "declared here" }
+void operator delete(void*) = delete;
+ };
+ void delete_B(B *b) { delete b; } // { dg-error "use of deleted function" }
+}
Index: testsuite/g++.dg/cpp2a/destroying-delete3.C
===
--- testsuite/g++.dg/cpp2a/destroying-delete3.C (nonexistent)
+++ testsuite/g++.dg/cpp2a/destroying-delete3.C (working copy)
@@ -0,0 +1,8 @@
+// PR c++/88969
+// { dg-do compile { target c++2a } }
+
+#include
+
+struct B {
+ void operator delete(void*, std::destroying_delete_t); // { dg-error
".operator delete. takes type .B*." }
+};
[PATCH] Revert yet another ::get_create for IPA summary (PR ipa/88958).
Hi.
This is one very similar patch.
Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
Ready to be installed?
Thanks,
Martin
>From adf577edd5a1d2b6ed78c1cc18feaff23fbfdd1c Mon Sep 17 00:00:00 2001
From: marxin
Date: Thu, 24 Jan 2019 16:07:29 +0100
Subject: [PATCH] Revert yet another ::get_create for IPA summary (PR
ipa/88958).
gcc/ChangeLog:
2019-01-24 Martin Liska
PR ipa/88958
* ipa-fnsummary.c (estimate_edge_devirt_benefit): Use
::get_create as the symbol can be not seen yet.
gcc/testsuite/ChangeLog:
2019-01-24 Martin Liska
PR ipa/88958
* gcc.dg/ipa/pr88985.c: New test.
---
gcc/ipa-fnsummary.c| 2 +-
gcc/testsuite/gcc.dg/ipa/pr88985.c | 13 +
2 files changed, 14 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.dg/ipa/pr88985.c
diff --git a/gcc/ipa-fnsummary.c b/gcc/ipa-fnsummary.c
index 535b3f22d49..af741f04eb2 100644
--- a/gcc/ipa-fnsummary.c
+++ b/gcc/ipa-fnsummary.c
@@ -2581,7 +2581,7 @@ estimate_edge_devirt_benefit (struct cgraph_edge *ie,
callee = callee->function_symbol ();
if (avail < AVAIL_AVAILABLE)
return false;
- isummary = ipa_fn_summaries->get (callee);
+ isummary = ipa_fn_summaries->get_create (callee);
return isummary->inlinable;
}
diff --git a/gcc/testsuite/gcc.dg/ipa/pr88985.c b/gcc/testsuite/gcc.dg/ipa/pr88985.c
new file mode 100644
index 000..8253a893cf6
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/pr88985.c
@@ -0,0 +1,13 @@
+void f (void)
+{
+}
+__attribute__((__optimize__("O2")))
+void g (void f())
+{
+ f();
+}
+__attribute__((__optimize__("O2")))
+void h (void)
+{
+ g(f);
+}
--
2.20.1
Re: [PATCH][AArch64] Initial -mcpu=ares tuning
On 15/01/2019 15:28, Kyrill Tkachov wrote:
> Hi all,
>
> This patch adds a tuning struct for the Arm Ares CPU and uses it for
> -m{cpu,tune}=ares.
> The tunings are an initial attempt and may be improved upon in the
> future, but they serve
> as a decent starting point for GCC 9.
>
> With this I see a 1.3% improvement on SPEC2006 int and 0.3% on SPEC2006
> fp with -mcpu=ares.
> On SPEC2017 I see a 0.6% improvement in intrate and changes in the noise
> for fprate.
>
> Bootstrapped and tested on aarch64-none-linux-gnu.
>
> Ok for trunk?
OK.
R.
> Thanks,
> Kyrill
>
> 2019-01-15 Kyrylo Tkachov
>
> * config/aarch64/aarch64.c (ares_tunings): Define.
> * config/aarch64/aarch64-cores.def (ares): Use the above.
>
> ares-tuning.patch
>
> diff --git a/gcc/config/aarch64/aarch64-cores.def
> b/gcc/config/aarch64/aarch64-cores.def
> index
> 70b076694d9901ccf15bfd26c950b6466d3d1cc2..7c4bd52049e5ae33241acce37414da91abaa989c
> 100644
> --- a/gcc/config/aarch64/aarch64-cores.def
> +++ b/gcc/config/aarch64/aarch64-cores.def
> @@ -100,7 +100,7 @@ AARCH64_CORE("thunderx2t99", thunderx2t99,
> thunderx2t99, 8_1A, AARCH64_FL_FOR
> AARCH64_CORE("cortex-a55", cortexa55, cortexa53, 8_2A,
> AARCH64_FL_FOR_ARCH8_2 | AARCH64_FL_F16 | AARCH64_FL_RCPC |
> AARCH64_FL_DOTPROD, cortexa53, 0x41, 0xd05, -1)
> AARCH64_CORE("cortex-a75", cortexa75, cortexa57, 8_2A,
> AARCH64_FL_FOR_ARCH8_2 | AARCH64_FL_F16 | AARCH64_FL_RCPC |
> AARCH64_FL_DOTPROD, cortexa73, 0x41, 0xd0a, -1)
> AARCH64_CORE("cortex-a76", cortexa76, cortexa57, 8_2A,
> AARCH64_FL_FOR_ARCH8_2 | AARCH64_FL_F16 | AARCH64_FL_RCPC |
> AARCH64_FL_DOTPROD, cortexa72, 0x41, 0xd0b, -1)
> -AARCH64_CORE("ares", ares, cortexa57, 8_2A, AARCH64_FL_FOR_ARCH8_2 |
> AARCH64_FL_F16 | AARCH64_FL_RCPC | AARCH64_FL_DOTPROD | AARCH64_FL_PROFILE,
> cortexa72, 0x41, 0xd0c, -1)
> +AARCH64_CORE("ares", ares, cortexa57, 8_2A, AARCH64_FL_FOR_ARCH8_2 |
> AARCH64_FL_F16 | AARCH64_FL_RCPC | AARCH64_FL_DOTPROD | AARCH64_FL_PROFILE,
> ares, 0x41, 0xd0c, -1)
>
> /* HiSilicon ('H') cores. */
> AARCH64_CORE("tsv110", tsv110, cortexa57, 8_2A, AARCH64_FL_FOR_ARCH8_2 |
> AARCH64_FL_CRYPTO | AARCH64_FL_F16 | AARCH64_FL_AES | AARCH64_FL_SHA2,
> tsv110, 0x48, 0xd01, -1)
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index
> 2fd6bb9821a256eaa2acaee305926b4efebf9c8c..b1e5eacb69728741517e313b110ebbc203d415a4
> 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -1083,6 +1083,32 @@ static const struct tune_params thunderx2t99_tunings =
>_prefetch_tune
> };
>
> +static const struct tune_params ares_tunings =
> +{
> + _extra_costs,
> + _addrcost_table,
> + _regmove_cost,
> + _vector_cost,
> + _branch_cost,
> + _approx_modes,
> + SVE_NOT_IMPLEMENTED, /* sve_width */
> + 4, /* memmov_cost */
> + 3, /* issue_rate */
> + AARCH64_FUSE_AES_AESMC, /* fusible_ops */
> + "32:16", /* function_align. */
> + "32:16", /* jump_align. */
> + "32:16", /* loop_align. */
> + 2, /* int_reassoc_width. */
> + 4, /* fp_reassoc_width. */
> + 2, /* vec_reassoc_width. */
> + 2, /* min_div_recip_mul_sf. */
> + 2, /* min_div_recip_mul_df. */
> + 0, /* max_case_values. */
> + tune_params::AUTOPREFETCHER_WEAK, /* autoprefetcher_model. */
> + (AARCH64_EXTRA_TUNE_NONE), /* tune_flags. */
> + _prefetch_tune
> +};
> +
> /* Support for fine-grained override of the tuning structures. */
> struct aarch64_tuning_override_function
> {
>
Re: [Patch] PR rtl-optimization/87763 - generate more bfi instructions on aarch64
On 24/01/2019 23:17, Steve Ellcey wrote:
> Here is my attempt at creating a couple of new instructions to
> generate more bfi instructions on aarch64. I haven't finished
> testing this but it helps with gcc.target/aarch64/combine_bfi_1.c.
>
> Before I went any further with it I wanted to see if anyone
> else was working on something like this and if this seems like
> a reasonable approach.
>
> Steve Ellcey
> sell...@marvell.com
>
>
> 2018-01-24 Steve Ellcey
>
> PR rtl-optimization/87763
> * config/aarch64/aarch64-protos.h
> (aarch64_masks_and_shift_for_aarch64_bfi_p): New prototype.
> * config/aarch64/aarch64.c (aarch64_masks_and_shift_for_aarch64_bfi_p):
> New function.
> * config/aarch64/aarch64.md (*aarch64_bfi4_shift):
> New instruction.
> (*aarch64_bfi4_noshift): Ditto.
>
>
>
> combine.patch
>
> diff --git a/gcc/config/aarch64/aarch64-protos.h
> b/gcc/config/aarch64/aarch64-protos.h
> index b035e35..ec90053 100644
> --- a/gcc/config/aarch64/aarch64-protos.h
> +++ b/gcc/config/aarch64/aarch64-protos.h
> @@ -429,6 +429,7 @@ bool aarch64_label_mentioned_p (rtx);
> void aarch64_declare_function_name (FILE *, const char*, tree);
> bool aarch64_legitimate_pic_operand_p (rtx);
> bool aarch64_mask_and_shift_for_ubfiz_p (scalar_int_mode, rtx, rtx);
> +bool aarch64_masks_and_shift_for_aarch64_bfi_p (scalar_int_mode, rtx, rtx,
> rtx);
> bool aarch64_zero_extend_const_eq (machine_mode, rtx, machine_mode, rtx);
> bool aarch64_move_imm (HOST_WIDE_INT, machine_mode);
> opt_machine_mode aarch64_sve_pred_mode (unsigned int);
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index 5df5a8b..69cc69f 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -9294,6 +9294,44 @@ aarch64_mask_and_shift_for_ubfiz_p (scalar_int_mode
> mode, rtx mask,
>& ((HOST_WIDE_INT_1U << INTVAL (shft_amnt)) - 1)) == 0;
> }
>
> +/* Return true if the masks and a shift amount from an RTX of the form
> + ((x & MASK1) | ((y << SHIFT_AMNT) & MASK2)) are valid to combine into
> + a BFI instruction of mode MODE. See *arch64_bfi patterns. */
> +
> +bool
> +aarch64_masks_and_shift_for_aarch64_bfi_p (scalar_int_mode mode, rtx mask1,
> +rtx shft_amnt, rtx mask2)
> +{
> + unsigned HOST_WIDE_INT m1, m2, s, t;
> +
> + if (!CONST_INT_P (mask1) || !CONST_INT_P (mask2) || !CONST_INT_P
> (shft_amnt))
> +return false;
Why not change the callers to pass HWI values directly? They all have
either immediate-only restrictions in the patters or pass a constructed
immediate value anyway.
> +
> + m1 = UINTVAL (mask1);
> + m2 = UINTVAL (mask2);
> + s = UINTVAL (shft_amnt);
> +
> + /* Verify that there is no overlap in what bits are set in the two masks.
> */
> + if ((m1 + m2 + 1) != 0)
> +return false;
> +
> + /* Verify that the shift amount is less than the mode size. */
> + if (s >= GET_MODE_BITSIZE (mode))
> +return false;
> +
> + /* Verify that the mask being shifted is contigious and would be in the
contiguous
> + least significant bits after shifting by creating a mask 't' based on
> + the number of bits set in mask2 and the shift amount for mask2 and
> + comparing that to the actual mask2. */
> + t = popcount_hwi (m2);
> + t = (1 << t) - 1;
> + t = t << s;
> + if (t != m2)
> +return false;
> +
> + return true;
> +}
> +
> /* Calculate the cost of calculating X, storing it in *COST. Result
> is true if the total cost of the operation has now been calculated. */
> static bool
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index b7f6fe0..e1f526b 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -5476,6 +5476,41 @@
>[(set_attr "type" "bfm")]
> )
>
> +;; Match a bfi instruction where the shift of OP3 means that we are
> +;; actually copying the least significant bits of OP3 into OP0 by way
> +;; of the AND masks and the IOR instruction.
> +
> +(define_insn "*aarch64_bfi4_shift"
> + [(set (match_operand:GPI 0 "register_operand" "=r")
> +(ior:GPI (and:GPI (match_operand:GPI 1 "register_operand" "0")
> + (match_operand:GPI 2 "const_int_operand" "n"))
> + (and:GPI (ashift:GPI
> + (match_operand:GPI 3 "register_operand" "r")
> + (match_operand:GPI 4
> "aarch64_simd_shift_imm_" "n"))
> + (match_operand:GPI 5 "const_int_operand" "n"]
> + "aarch64_masks_and_shift_for_aarch64_bfi_p (mode, operands[2],
> operands[4], operands[5])"
> +{
> + return "bfi\t%0, %3, %4, %P5";
You don't need C code for this instruction printer, just use a textual
pattern.
> +}
> + [(set_attr "type" "bfm")]
> +)
> +
> +;; Like the above instruction but with no shifting, we are just copying the
> +;; least significant bits of OP3 to OP0.
> +
[PATCH] Fix PR86865
The current translation of GIMPLE to ISL only handles do-while loops
correctly so we have to reject others.
Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk,
queued for backporting.
Richard.
2019-01-25 Richard Biener
PR tree-optimization/86865
* graphite-scop-detection.c (scop_detection::can_represent_loop):
Reject non-do-while loops.
* gcc.dg/graphite/pr86865.c: New testcase.
Index: gcc/graphite-scop-detection.c
===
--- gcc/graphite-scop-detection.c (revision 268010)
+++ gcc/graphite-scop-detection.c (working copy)
@@ -555,8 +555,15 @@ scop_detection::can_represent_loop (loop
tree niter;
struct tree_niter_desc niter_desc;
- return single_exit (loop)
-&& !(loop_preheader_edge (loop)->flags & EDGE_IRREDUCIBLE_LOOP)
+ /* We can only handle do {} while () style loops correctly. */
+ edge exit = single_exit (loop);
+ if (!exit
+ || !single_pred_p (loop->latch)
+ || exit->src != single_pred (loop->latch)
+ || !empty_block_p (loop->latch))
+return false;
+
+ return !(loop_preheader_edge (loop)->flags & EDGE_IRREDUCIBLE_LOOP)
&& number_of_iterations_exit (loop, single_exit (loop), _desc, false)
&& niter_desc.control.no_overflow
&& (niter = number_of_latch_executions (loop))
Index: gcc/testsuite/gcc.dg/graphite/pr86865.c
===
--- gcc/testsuite/gcc.dg/graphite/pr86865.c (nonexistent)
+++ gcc/testsuite/gcc.dg/graphite/pr86865.c (working copy)
@@ -0,0 +1,35 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -fgraphite-identity -fstack-reuse=none -fwrapv
-fno-tree-ch -fno-tree-dce -fno-tree-dominator-opts -fno-tree-loop-ivcanon" } */
+
+int xy, tb;
+
+void
+bt (void)
+{
+ for (xy = 0; xy >= 0; --xy)
+{
+ int yt[8] = { 0 };
+ int pz[2] = { 0 };
+ int sa[32] = { 0 };
+ int us;
+
+ for (us = 0; us < 8; ++us)
+ yt[us] = 0;
+
+ (void) yt;
+ (void) pz;
+ (void) sa;
+}
+
+ tb = 1;
+}
+
+int
+main (void)
+{
+ bt ();
+ if (xy != -1)
+__builtin_abort ();
+
+ return 0;
+}
Index: gcc/testsuite/gcc.dg/graphite/pr69728.c
===
--- gcc/testsuite/gcc.dg/graphite/pr69728.c (revision 268010)
+++ gcc/testsuite/gcc.dg/graphite/pr69728.c (working copy)
@@ -24,4 +24,6 @@ fn1 ()
run into scheduling issues before here, not being able to handle
empty domains. */
-/* { dg-final { scan-tree-dump "loop nest optimized" "graphite" } } */
+/* XFAILed by fix for PR86865. */
+
+/* { dg-final { scan-tree-dump "loop nest optimized" "graphite" { xfail *-*-*
} } } */
Index: gcc/testsuite/gcc.dg/graphite/scop-21.c
===
--- gcc/testsuite/gcc.dg/graphite/scop-21.c (revision 268010)
+++ gcc/testsuite/gcc.dg/graphite/scop-21.c (working copy)
@@ -30,4 +30,5 @@ int test ()
return a[20];
}
-/* { dg-final { scan-tree-dump-times "number of SCoPs: 1" 1 "graphite"} } */
+/* XFAILed by the fix for PR86865. */
+/* { dg-final { scan-tree-dump-times "number of SCoPs: 1" 1 "graphite" { xfail
*-*-* } } } */
