Re: [Patch, vrp] Allow VRP type conversion folding only for widenings upto word mode
On Wed, Nov 18, 2015 at 09:36:21AM +0100, Richard Biener wrote:
> On Wed, 18 Nov 2015, Senthil Kumar Selvaraj wrote:
>
> > On Mon, Nov 16, 2015 at 10:02:15AM +0100, Richard Biener wrote:
> > > On Sat, 14 Nov 2015, Senthil Kumar Selvaraj wrote:
> > >
> > > > On Sat, Nov 14, 2015 at 09:57:40AM +0100, Richard Biener wrote:
> > > > > On November 14, 2015 9:49:28 AM GMT+01:00, Senthil Kumar Selvaraj
> > > > > wrote:
> > > > > >On Sat, Nov 14, 2015 at 09:13:41AM +0100, Marc Glisse wrote:
> > > > > >> On Sat, 14 Nov 2015, Senthil Kumar Selvaraj wrote:
> > > > > >>
> > > > > >> >This patch came out of a discussion held in the gcc mailing list
> > > > > >> >(https://gcc.gnu.org/ml/gcc/2015-11/msg00067.html).
> > > > > >> >
> > > > > >> >The patch restricts folding of conditional exprs with lhs
> > > > > >> >previously
> > > > > >> >set by a type conversion to occur only if the source of the type
> > > > > >> >conversion's mode is word mode or smaller.
> > > > > >> >
> > > > > >> >Bootstrapped and reg tested on x86_64 (with
> > > > > >--enable-languages=c,c++).
> > > > > >> >
> > > > > >> >If ok, could you commit please? I don't have commit access.
> > > > > >> >
> > > > > >> >Regards
> > > > > >> >Senthil
> > > > > >> >
> > > > > >> >gcc/ChangeLog
> > > > > >> >
> > > > > >> >2015-11-11 Senthil Kumar Selvaraj
> > > > > >
> > > > > >> >
> > > > > >> > * tree-vrp.c (simplify_cond_using_ranges): Fold only
> > > > > >> > if innerop's mode is word_mode or smaller.
> > > > > >> >
> > > > > >> >
> > > > > >> >diff --git gcc/tree-vrp.c gcc/tree-vrp.c
> > > > > >> >index e2393e4..c139bc6 100644
> > > > > >> >--- gcc/tree-vrp.c
> > > > > >> >+++ gcc/tree-vrp.c
> > > > > >> >@@ -9467,6 +9467,8 @@ simplify_cond_using_ranges (gcond *stmt)
> > > > > >> > innerop = gimple_assign_rhs1 (def_stmt);
> > > > > >> >
> > > > > >> > if (TREE_CODE (innerop) == SSA_NAME
> > > > > >> >+ && (GET_MODE_SIZE(TYPE_MODE(TREE_TYPE(innerop)))
> > > > > >> >+ <= GET_MODE_SIZE(word_mode))
> > > > > >> >&& !POINTER_TYPE_P (TREE_TYPE (innerop)))
> > > > > >> > {
> > > > > >> >value_range *vr = get_value_range (innerop);
> > > > > >>
> > > > > >> I thought the result of the discussion was that the transformation
> > > > > >> is
> > > > > >ok if
> > > > > >> either it is narrowing or it widens but to something no bigger than
> > > > > >> word_mode. So you should have 2 comparisons, or 1 with a max.
> > > > > >
> > > > > >Hmm, I came to the opposite conclusion - I thought Richard only
> > > > > >okayed
> > > > > >"widening upto word-mode", not the narrowing.
> > > > >
> > > > > I didn't mean to suggest narrowing is not OK. In fact narrowing is
> > > > > always OK.
> > > >
> > > > My bad. Here's a revised patch that checks for both conditions, using
> > > > max as Marc suggested to limit to word_mode or narrowing conversions.
> > > >
> > > > Bootstrapped and regtested for x86_64 with c and c++.
> > > >
> > > > Is this ok? If yes, would you commit it
> > > > for me please? I don't have commit access.
> > > >
> > > > gcc/ChangeLog
> > > > 2015-11-14 Senthil Kumar Selvaraj
> > > >
> > > > * tree-vrp.c (simplify_cond_using_ranges): Fold only
> > > > if innerop's mode smaller or equal to word_mode or op0's mode.
> > > >
> > > >
> > > > diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
> > > > index e2393e4..cfd90e7 100644
> > > > --- a/gcc/tree-vrp.c
> > > > +++ b/gcc/tree-vrp.c
> > > > @@ -9467,7 +9467,10 @@ simplify_cond_using_ranges (gcond *stmt)
> > > >innerop = gimple_assign_rhs1 (def_stmt);
> > > >
> > > >if (TREE_CODE (innerop) == SSA_NAME
> > > > - && !POINTER_TYPE_P (TREE_TYPE (innerop)))
> > > > + && !POINTER_TYPE_P (TREE_TYPE (innerop))
> > > > + && (GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (innerop)))
> > > > + <= std::max (GET_MODE_SIZE (word_mode),
> > > > +GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (op0))
> > >
> > > Please use TYPE_PRECISION (...) and GET_MODE_PRECISION (word_mode) and
> > > add a comment as to what we are testing here and why.
> > >
> > > Btw, ideally we'd factor out a
> > >
> > > bool
> > > desired_pro_or_demotion_p (tree to_type, tree from_type) {}
> > >
> > > function somewhere as we have similar tests throughout the compiler
> > > that we might want to unify (and also have a central place to
> > > eventually add a target hook if ever desired).
> > >
> > > In fact in other places we also check that the type we promote/demote
> > > to matches its mode precision or the type we promote/demote from
> > > already does not.
> > >
> > > I'd suggest tree.[ch] for that function.
> > >
> > > Please also add a testcase.
> >
> > How does the below patch look? Bootstrapped, but not regtested yet.
> >
> > The testcase was rather tricky to write - I wasn't sure how to reliably
> > get a type bigger than a word for all targets. I resorted to __int128,
> > not sure it's a good idea though - I
Re: PATCH to shorten_compare -Wtype-limits handling
On Wed, Nov 18, 2015 at 11:26 PM, Jason Merrill wrote: > The rs6000 target was hitting a bootstrap failure due to > -Werror=type-limits. Since warn_tautological_cmp and other warnings avoid > warning if one of the operands comes from a macro, I thought it would make > sense to do that here as well. > > Tested that this allows rs6000 bootstrap to proceed, regression tested on > x86_64-pc-linux-gnu, applying to trunk. David, do you want to revert the > #pragma GCC diagnostic change? Thanks for the patch. Yes, I will revert the #pragma GCC diagnostic patch. It was meant as a temporary hack to fix bootstrap while a long-term solution was developed. Thanks, David
PATCH to shorten_compare -Wtype-limits handling
The rs6000 target was hitting a bootstrap failure due to
-Werror=type-limits. Since warn_tautological_cmp and other warnings
avoid warning if one of the operands comes from a macro, I thought it
would make sense to do that here as well.
Tested that this allows rs6000 bootstrap to proceed, regression tested
on x86_64-pc-linux-gnu, applying to trunk. David, do you want to revert
the #pragma GCC diagnostic change?
commit 3d382500ccb766eb1c6dea69a32348d62c86b950
Author: Jason Merrill
Date: Wed Nov 18 16:30:06 2015 -0500
* c-common.c (shorten_compare): Don't -Wtype-limits if the
non-constant operand comes from a macro.
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index f50ca48..068a0bc 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -4650,7 +4650,9 @@ shorten_compare (location_t loc, tree *op0_ptr, tree *op1_ptr,
type = c_common_unsigned_type (type);
}
- if (TREE_CODE (primop0) != INTEGER_CST)
+ if (TREE_CODE (primop0) != INTEGER_CST
+ /* Don't warn if it's from a macro. */
+ && !from_macro_expansion_at (EXPR_LOCATION (primop0)))
{
if (val == truthvalue_false_node)
warning_at (loc, OPT_Wtype_limits,
diff --git a/gcc/testsuite/g++.dg/warn/Wtype-limits2.C b/gcc/testsuite/g++.dg/warn/Wtype-limits2.C
new file mode 100644
index 000..a46baad
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wtype-limits2.C
@@ -0,0 +1,11 @@
+// { dg-options -Wtype-limits }
+
+unsigned char array[4];
+bool b;
+#define VAL (b ? array[0] : (unsigned char)0)
+
+int main()
+{
+ if (VAL > 1000)
+__builtin_abort();
+}
Re: [C++ PATCH] Issue hard error even with -fpermissive for certain goto violations (PR c++/67409)
On 18/11/15 22:55, Jakub Jelinek wrote:
static bool
-identify_goto (tree decl, const location_t *locus)
+identify_goto (tree decl, const location_t *locus, bool harderr)
{
- bool complained = (decl
-? permerror (input_location, "jump to label %qD", decl)
-: permerror (input_location, "jump to case label"));
+ bool complained;
+ if (!harderr)
+{
+ if (decl)
+ complained = permerror (input_location, "jump to label %qD", decl);
+ else
+ complained = permerror (input_location, "jump to case label");
+}
+ else
+{
+ if (decl)
+ error ("jump to label %qD", decl);
+ else
+ error ("jump to case label");
+ complained = true;
+}
if (complained && locus)
inform (*locus, " from here");
return complained;
The above is a bit repetitive. Why not simply?
static bool
error_jumpto (diagnostic_t kind, location_t loc, tree decl)
{
bool complained = (decl
? emit_diagnostic (kind, input_location, 0,
"jump to label %qD", decl)
: emit_diagnostic (kind, input_location, 0,
"jump to case label"));
if (complained && loc)
inform (loc, " from here");
return complained;
}
That is, call a function that gives errors about X, error_X; no point in
passing a pointer to location_t; most diagnostic functions take loc as the
first argument; no obscure bool parameter. Then call:
@@ -2991,15 +3004,16 @@ check_previous_goto_1 (tree decl, cp_bin
bool exited_omp, const location_t *locus)
{
cp_binding_level *b;
- bool identified = false, complained = false;
+ bool complained = false;
+ int identified = 0;
bool saw_eh = false, saw_omp = false, saw_tm = false;
if (exited_omp)
{
- complained = identify_goto (decl, locus);
+ complained = identify_goto (decl, locus, true);
complained = error_jumpto (DK_ERROR, loc, decl);
+ complained = identify_goto (decl, locus, false);
complained = error_jumpto (DK_PERMERROR, loc, decl);
+ if (ent->in_try_scope || ent->in_catch_scope
+ || ent->in_transaction_scope || ent->in_omp_scope)
+ {
+ error_at (DECL_SOURCE_LOCATION (decl), "jump to label %qD", decl);
+ complained = true;
+ identified = 2;
+ }
+ else
+ {
+ complained = permerror (DECL_SOURCE_LOCATION (decl),
+ "jump to label %qD", decl);
+ identified = 1;
+ }
if (complained)
inform (input_location, " from here");
Note that if the function above takes another location_t argument, you can also
simplify this hunk to:
diagnostic_t kind;
if (ent->in_try_scope || ent->in_catch_scope
|| ent->in_transaction_scope || ent->in_omp_scope)
{
kind = DK_ERROR;
identified = 2;
}
else
{
kind = DK_PERMERROR;
identified = 1;
}
complained = error_jumpto (kind, loc, DECL_SOURCE_LOCATION (decl),
decl);
You can even use kind (maybe 'error_kind') directly instead of identified for
what you are trying to achieve, with error_kind in {DK_UNSPECIFIED, DK_ERROR,
DK_PERMERROR}.
FOR_EACH_VEC_SAFE_ELT (ent->bad_decls, ix, bad)
@@ -3155,6 +3180,14 @@ check_goto (tree decl)
if (u > 1 && DECL_ARTIFICIAL (bad))
{
/* Can't skip init of __exception_info. */
+ if (identified == 1)
+ {
+ error_at (DECL_SOURCE_LOCATION (decl),
+ "jump to label %qD", decl);
+ inform (input_location, " from here");
+ complained = true;
+ identified = 2;
+ }
and here:
kind = DK_ERROR;
complained = error_jumpto (kind, input_location,
DECL_SOURCE_LOCATION (decl), decl);
if (complained)
inform (DECL_SOURCE_LOCATION (bad), " enters catch block");
saw_catch = true;
@@ -3195,13 +3228,13 @@ check_goto (tree decl)
break;
if (b->kind == sk_omp)
{
- if (!identified)
+ if (identified < 2)
{
- complained = permerror (DECL_SOURCE_LOCATION (decl),
- "jump to label %qD", decl);
- if (complained)
- inform (input_location, " from here");
- identified = true;
+ error_at (DECL_SOURCE_LOCATION (decl),
+ "jump to label %qD", decl);
+ inform (input_location, " from here");
+ complained = true;
+ identified = 2;
}
and the same here.
Cheers,
Manuel.
Re: [PATCH AArch64]Handle REG+REG+CONST and REG+NON_REG+CONST in legitimize address
On Tue, Nov 17, 2015 at 6:08 PM, James Greenhalgh
wrote:
> On Tue, Nov 17, 2015 at 05:21:01PM +0800, Bin Cheng wrote:
>> Hi,
>> GIMPLE IVO needs to call backend interface to calculate costs for addr
>> expressions like below:
>>FORM1: "r73 + r74 + 16380"
>>FORM2: "r73 << 2 + r74 + 16380"
>>
>> They are invalid address expression on AArch64, so will be legitimized by
>> aarch64_legitimize_address. Below are what we got from that function:
>>
>> For FORM1, the address expression is legitimized into below insn sequence
>> and rtx:
>>r84:DI=r73:DI+r74:DI
>>r85:DI=r84:DI+0x3000
>>r83:DI=r85:DI
>>"r83 + 4092"
>>
>> For FORM2, the address expression is legitimized into below insn sequence
>> and rtx:
>>r108:DI=r73:DI<<0x2
>>r109:DI=r108:DI+r74:DI
>>r110:DI=r109:DI+0x3000
>>r107:DI=r110:DI
>>"r107 + 4092"
>>
>> So the costs computed are 12/16 respectively. The high cost prevents IVO
>> from choosing right candidates. Besides cost computation, I also think the
>> legitmization is bad in terms of code generation.
>> The root cause in aarch64_legitimize_address can be described by it's
>> comment:
>>/* Try to split X+CONST into Y=X+(CONST & ~mask), Y+(CONST&mask),
>> where mask is selected by alignment and size of the offset.
>> We try to pick as large a range for the offset as possible to
>> maximize the chance of a CSE. However, for aligned addresses
>> we limit the range to 4k so that structures with different sized
>> elements are likely to use the same base. */
>> I think the split of CONST is intended for REG+CONST where the const offset
>> is not in the range of AArch64's addressing modes. Unfortunately, it
>> doesn't explicitly handle/reject "REG+REG+CONST" and "REG+REG<> when the CONST are in the range of addressing modes. As a result, these two
>> cases fallthrough this logic, resulting in sub-optimal results.
>>
>> It's obvious we can do below legitimization:
>> FORM1:
>>r83:DI=r73:DI+r74:DI
>>"r83 + 16380"
>> FORM2:
>>r107:DI=0x3ffc
>>r106:DI=r74:DI+r107:DI
>> REG_EQUAL r74:DI+0x3ffc
>>"r106 + r73 << 2"
>>
>> This patch handles these two cases as described.
>
> Thanks for the description, it made the patch very easy to review. I only
> have a style comment.
>
>> Bootstrap & test on AArch64 along with other patch. Is it OK?
>>
>> 2015-11-04 Bin Cheng
>> Jiong Wang
>>
>> * config/aarch64/aarch64.c (aarch64_legitimize_address): Handle
>> address expressions like REG+REG+CONST and REG+NON_REG+CONST.
>
>> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
>> index 5c8604f..47875ac 100644
>> --- a/gcc/config/aarch64/aarch64.c
>> +++ b/gcc/config/aarch64/aarch64.c
>> @@ -4710,6 +4710,51 @@ aarch64_legitimize_address (rtx x, rtx /* orig_x */,
>> machine_mode mode)
>> {
>>HOST_WIDE_INT offset = INTVAL (XEXP (x, 1));
>>HOST_WIDE_INT base_offset;
>> + rtx op0 = XEXP (x,0);
>> +
>> + if (GET_CODE (op0) == PLUS)
>> + {
>> + rtx op0_ = XEXP (op0, 0);
>> + rtx op1_ = XEXP (op0, 1);
>
> I don't see this trailing _ on a variable name in many places in the source
> tree (mostly in the Go frontend), and certainly not in the aarch64 backend.
> Can we pick a different name for op0_ and op1_?
>
>> +
>> + /* RTX pattern in the form of (PLUS (PLUS REG, REG), CONST) will
>> + reach here, the 'CONST' may be valid in which case we should
>> + not split. */
>> + if (REG_P (op0_) && REG_P (op1_))
>> + {
>> + machine_mode addr_mode = GET_MODE (op0);
>> + rtx addr = gen_reg_rtx (addr_mode);
>> +
>> + rtx ret = plus_constant (addr_mode, addr, offset);
>> + if (aarch64_legitimate_address_hook_p (mode, ret, false))
>> + {
>> + emit_insn (gen_adddi3 (addr, op0_, op1_));
>> + return ret;
>> + }
>> + }
>> + /* RTX pattern in the form of (PLUS (PLUS REG, NON_REG), CONST)
>> + will reach here. If (PLUS REG, NON_REG) is valid addr expr,
>> + we split it into Y=REG+CONST, Y+NON_REG. */
>> + else if (REG_P (op0_) || REG_P (op1_))
>> + {
>> + machine_mode addr_mode = GET_MODE (op0);
>> + rtx addr = gen_reg_rtx (addr_mode);
>> +
>> + /* Switch to make sure that register is in op0_. */
>> + if (REG_P (op1_))
>> + std::swap (op0_, op1_);
>> +
>> + rtx ret = gen_rtx_fmt_ee (PLUS, addr_mode, addr, op1_);
>> + if (aarch64_legitimate_address_hook_p (mode, ret, false))
>> + {
>> + addr = force_operand (plus_constant (addr_mode,
>> +op0_, offset),
>> + NULL_RTX);
>> + ret = gen_rtx_fmt_ee (PLUS, addr_mode, addr, op1_);
>> + return ret;
>> + }
>
> The
Re: RFC/RFA: Fix bug with REE optimization corrupting extended registers
On 11/18/2015 05:52 PM, Bernd Schmidt wrote: (This is on the RL78 target where HImode values occupy two hard registers and QImode values only one. The bug however is generic, not RL78 specific). The REE pass transforms this into: (insn 44 (set (reg:QI r11) (mem:QI (reg:HI r20))) (insn 45 (set (reg:HI r10) (zero_extend:HI (mem:QI (reg:HI r18 [...] (insn 71 (set (reg:HI r14) (zero_extend:HI (reg:QI r11))) [...] (insn 88 deleted) Note how the new set at insn 45 clobbers the value loaded by insn 44 into r11. I had a look around. There's code testing HARD_REGNO_NREGS in ree.c:combine_set_extension. It's inside #if 0, and labelled "temporarily disabled". See if enabling that helps you? (Jeff, that #if 0 was added by you). I was going to dig into this discussion tomorrow since I was the last one in that code. Just had to get that threading regression resolved first :-) jeff
Re: [PATCH] Fix memory leaks in tree-ssa-uninit.c
On 11/19/2015 01:50 AM, Joseph Myers wrote: I don't think all the reformattings here are things we want to do globally for most source files. While I do appreciate the sentiment behind the patch, I agree with all of Joseph's points. Especially the clearly incorrect changes should be reverted if the patch has already been applied. @@ -135,10 +133,9 @@ warn_uninit (enum opt_code wc, tree t, tree expr, tree var, /* TREE_NO_WARNING either means we already warned, or the front end wishes to suppress the warning. */ - if ((context - && (gimple_no_warning_p (context) - || (gimple_assign_single_p (context) - && TREE_NO_WARNING (gimple_assign_rhs1 (context) + if ((context && (gimple_no_warning_p (context) + || (gimple_assign_single_p (context) + && TREE_NO_WARNING (gimple_assign_rhs1 (context) I think in cases such as this, putting the operator && on the start of the next line to make subsequent lines less-indented makes sense. Again, the new formatting isn't incorrect, but that doesn't make it a desirable change. @@ -217,24 +212,20 @@ warn_uninitialized_vars (bool warn_possibly_uninitialized) so must be limited which means we would miss warning opportunities. */ use = gimple_vuse (stmt); - if (use - && gimple_assign_single_p (stmt) - && !gimple_vdef (stmt) + if (use && gimple_assign_single_p (stmt) && !gimple_vdef (stmt) && SSA_NAME_IS_DEFAULT_DEF (use)) I think there may be a preference, where it's necessary to use multiple lines (if the whole condition doesn't fit on one line) for a condition like this, for each "&& condition" to go on its own line, rather than some lines having multiple conditions. These ones (and any others like them) I would also like to see reverted. Both of them are contrary to existing practice. Bernd
Re: [PATCH] Fix libcpp ICE (PR preprocessor/60736)
On Thu, Nov 19, 2015 at 01:41:52AM +0100, Bernd Schmidt wrote: > On 11/19/2015 12:02 AM, Jakub Jelinek wrote: > >2015-11-18 Jakub Jelinek > > > > PR preprocessor/60736 > > * include/cpplib.h (cpp_errno_filename): New prototype. > > * errors.c (cpp_errno): Don't handle msgid "" specially, use > > _(msgid) instead of msgid as argument to cpp_error. > > (cpp_errno_filename): New function. > > * files.c (read_file_guts): Use cpp_errno_filename instead of > > cpp_errno. > > (open_file_failed): Likewise. Use file->name if file->path is NULL > > in diagnostics. > > Is it worth it to put Ian's mini-testcase of "#include " > into the testsuite? Well, it behaves differently when stdc-predef.h is present in /usr/include and when it is not, so it would have to say dg-prune-output the diagnostics or something similar. Jakub
Re: RFC/RFA: Fix bug with REE optimization corrupting extended registers
(This is on the RL78 target where HImode values occupy two hard registers and QImode values only one. The bug however is generic, not RL78 specific). The REE pass transforms this into: (insn 44 (set (reg:QI r11) (mem:QI (reg:HI r20))) (insn 45 (set (reg:HI r10) (zero_extend:HI (mem:QI (reg:HI r18 [...] (insn 71 (set (reg:HI r14) (zero_extend:HI (reg:QI r11))) [...] (insn 88 deleted) Note how the new set at insn 45 clobbers the value loaded by insn 44 into r11. I had a look around. There's code testing HARD_REGNO_NREGS in ree.c:combine_set_extension. It's inside #if 0, and labelled "temporarily disabled". See if enabling that helps you? (Jeff, that #if 0 was added by you). Bernd
Re: [PATCH] Fix memory leaks in tree-ssa-uninit.c
I don't think all the reformattings here are things we want to do globally
for most source files. E.g.
> @@ -75,18 +74,17 @@ get_mask_first_set_bit (unsigned mask)
> static bool
> has_undefined_value_p (tree t)
> {
> - return (ssa_undefined_value_p (t)
> - || (possibly_undefined_names
> - && possibly_undefined_names->contains (t)));
> + return ssa_undefined_value_p (t)
> + || (possibly_undefined_names
> + && possibly_undefined_names->contains (t));
This seems plain wrong. The GNU Coding Standards explicitly say that in
such cases where a binary operator goes on the start of the next line, you
should insert extra parentheses so that Emacs will get the indentation
right (even though otherwise parentheses shouldn't be used with "return").
So the old code was better.
> static void
> -warn_uninit (enum opt_code wc, tree t, tree expr, tree var,
> - const char *gmsgid, void *data, location_t phiarg_loc)
> +warn_uninit (enum opt_code wc, tree t, tree expr, tree var, const char
> *gmsgid,
> + void *data, location_t phiarg_loc)
Just because it's OK to go up to an 80- or 79-column limit if necessary
doesn't mean code should be reformatted to do so. Breaking a bit before
that is perfectly reasonable in general (and in some cases, the choice of
where to break may be based on logical grouping of arguments).
> @@ -135,10 +133,9 @@ warn_uninit (enum opt_code wc, tree t, tree expr, tree
> var,
>
>/* TREE_NO_WARNING either means we already warned, or the front end
> wishes to suppress the warning. */
> - if ((context
> - && (gimple_no_warning_p (context)
> -|| (gimple_assign_single_p (context)
> -&& TREE_NO_WARNING (gimple_assign_rhs1 (context)
> + if ((context && (gimple_no_warning_p (context)
> +|| (gimple_assign_single_p (context)
> +&& TREE_NO_WARNING (gimple_assign_rhs1 (context)
I think in cases such as this, putting the operator && on the start of the
next line to make subsequent lines less-indented makes sense. Again, the
new formatting isn't incorrect, but that doesn't make it a desirable
change.
> @@ -217,24 +212,20 @@ warn_uninitialized_vars (bool
> warn_possibly_uninitialized)
>so must be limited which means we would miss warning
>opportunities. */
> use = gimple_vuse (stmt);
> - if (use
> - && gimple_assign_single_p (stmt)
> - && !gimple_vdef (stmt)
> + if (use && gimple_assign_single_p (stmt) && !gimple_vdef (stmt)
> && SSA_NAME_IS_DEFAULT_DEF (use))
I think there may be a preference, where it's necessary to use multiple
lines (if the whole condition doesn't fit on one line) for a condition
like this, for each "&& condition" to go on its own line, rather than some
lines having multiple conditions.
> /* Do not warn if it can be initialized outside this function. */
> - if (TREE_CODE (base) != VAR_DECL
> - || DECL_HARD_REGISTER (base)
> + if (TREE_CODE (base) != VAR_DECL || DECL_HARD_REGISTER (base)
> || is_global_var (base))
Likewise.
> @@ -2393,15 +2338,28 @@ class pass_late_warn_uninitialized : public
> gimple_opt_pass
> public:
>pass_late_warn_uninitialized (gcc::context *ctxt)
> : gimple_opt_pass (pass_data_late_warn_uninitialized, ctxt)
> - {}
> + {
> + }
Is that really what we want?
I think those examples are sufficient to illustrate the problems with
automatic conversion from one correct format to another correct format (as
opposed to fixing cases where the formatting is unambiguously incorrect,
which covers plenty of the changes in this patch).
--
Joseph S. Myers
jos...@codesourcery.com
Re: [PATCH] Fix libcpp ICE (PR preprocessor/60736)
On 11/19/2015 12:02 AM, Jakub Jelinek wrote: 2015-11-18 Jakub Jelinek PR preprocessor/60736 * include/cpplib.h (cpp_errno_filename): New prototype. * errors.c (cpp_errno): Don't handle msgid "" specially, use _(msgid) instead of msgid as argument to cpp_error. (cpp_errno_filename): New function. * files.c (read_file_guts): Use cpp_errno_filename instead of cpp_errno. (open_file_failed): Likewise. Use file->name if file->path is NULL in diagnostics. Ok. Is it worth it to put Ian's mini-testcase of "#include " into the testsuite? Bernd
Re: [PATCH, 10/16] Add pass_oacc_kernels pass group in passes.def
On 17/11/15 15:53, Tom de Vries wrote:
And the above LIM example
is none for why you need two LIM passes...
Indeed. I'm planning a separate reply to explain in more detail the need
for the two pass_lims.
I.
I managed to get rid of the two pass_lims for the motivating example
that I used until now (goacc/kernels-double-reduction.c). I found that
by adding a pass_dominator instance after pass_ch, I could get rid of
the second pass_lim (and pass_copyprop as well).
But... then I wrote a counter example
(goacc/kernels-double-reduction-n.c), and I'm back at two pass_lims (and
two pass_dominators).
Also I've split the pass group into a bit before and after pass_fre.
So, the current pass group looks like:
...
NEXT_PASS (pass_build_ealias);
/* Pass group that runs when the function is an offloaded function
containing oacc kernels loops. Part 1. */
NEXT_PASS (pass_oacc_kernels);
PUSH_INSERT_PASSES_WITHIN (pass_oacc_kernels)
/* We need pass_ch here, because pass_lim has no effect on
exit-first loops (PR65442). Ideally we want to remove both
this pass instantiation, and the reverse transformation
transform_to_exit_first_loop_alt, which is done in
pass_parallelize_loops_oacc_kernels. */
NEXT_PASS (pass_ch);
POP_INSERT_PASSES ()
NEXT_PASS (pass_fre);
/* Pass group that runs when the function is an offloaded function
containing oacc kernels loops. Part 2. */
NEXT_PASS (pass_oacc_kernels2);
PUSH_INSERT_PASSES_WITHIN (pass_oacc_kernels2)
/* We use pass_lim to rewrite in-memory iteration and reduction
variable accesses in loops into local variables accesses. */
NEXT_PASS (pass_lim);
NEXT_PASS (pass_dominator, false /* may_peel_loop_headers_p */);
NEXT_PASS (pass_lim);
NEXT_PASS (pass_dominator, false /* may_peel_loop_headers_p */);
NEXT_PASS (pass_dce);
NEXT_PASS (pass_parallelize_loops_oacc_kernels);
NEXT_PASS (pass_expand_omp_ssa);
POP_INSERT_PASSES ()
NEXT_PASS (pass_merge_phi);
...
II.
The motivating test-case kernels-double-reduction-n.c:
...
#include
#define N 500
unsigned int a[N][N];
void __attribute__((noinline,noclone))
foo (unsigned int n)
{
int i, j;
unsigned int sum = 1;
#pragma acc kernels copyin (a[0:n]) copy (sum)
{
for (i = 0; i < n; ++i)
for (j = 0; j < n; ++j)
sum += a[i][j];
}
if (sum != 5001)
abort ();
}
...
III.
Before first pass_lim. Note no phis on inner or outer loop header for
iteration varables or reduction variable:
...
:
_5 = *.omp_data_i_4(D).i;
*_5 = 0;
_44 = *.omp_data_i_4(D).n;
_45 = *_44;
if (_45 != 0)
goto ;
else
goto ;
: outer loop header
_12 = *.omp_data_i_4(D).j;
*_12 = 0;
if (_45 != 0)
goto ;
else
goto ;
: inner loop header, latch
_19 = *.omp_data_i_4(D).a;
_21 = *_5;
_23 = *_12;
_24 = *_19[_21][_23];
_25 = *.omp_data_i_4(D).sum;
sum.0_26 = *_25;
sum.1_27 = _24 + sum.0_26;
*_25 = sum.1_27;
_33 = _23 + 1;
*_12 = _33;
j.2_16 = (unsigned int) _33;
if (j.2_16 < _45)
goto ;
else
goto ;
: outer loop latch
_36 = *_5;
_38 = _36 + 1;
*_5 = _38;
i.3_9 = (unsigned int) _38;
if (i.3_9 < _45)
goto ;
else
goto ;
:
return;
...
IV.
After first pass_lim/pass_dom pair. Note there are phis on the inner
loop header for the reduction and the iteration variable, but not on the
outer loop header:
...
:
_5 = *.omp_data_i_4(D).i;
*_5 = 0;
_44 = *.omp_data_i_4(D).n;
_45 = *_44;
if (_45 != 0)
goto ;
else
goto ;
:
_12 = *.omp_data_i_4(D).j;
_19 = *.omp_data_i_4(D).a;
D__lsm.10_50 = *_12;
D__lsm.11_51 = 0;
_25 = *.omp_data_i_4(D).sum;
: outer loop header
D__lsm.10_20 = 0;
D__lsm.11_22 = 1;
_21 = *_5;
D__lsm.12_28 = *_25;
D__lsm.13_30 = 0;
goto ;
: inner loop header, latch
# D__lsm.10_47 = PHI <0(5), _33(7)>
# D__lsm.12_49 = PHI
_23 = D__lsm.10_47;
_24 = *_19[_21][D__lsm.10_47];
sum.0_26 = D__lsm.12_49;
sum.1_27 = _24 + D__lsm.12_49;
D__lsm.12_31 = sum.1_27;
D__lsm.13_32 = 1;
_33 = D__lsm.10_47 + 1;
D__lsm.10_14 = _33;
D__lsm.11_15 = 1;
j.2_16 = (unsigned int) _33;
if (j.2_16 < _45)
goto ;
else
goto ;
: outer loop latch
# D__lsm.10_35 = PHI <_33(7)>
# D__lsm.11_37 = PHI <1(7)>
# D__lsm.12_7 = PHI
# D__lsm.13_8 = PHI <1(7)>
*_25 = sum.1_27;
_36 = *_5;
_38 = _36 + 1;
*_5 = _38;
i.3_9 = (unsigned int) _38;
if (i.3_9 < _45)
goto ;
else
goto ;
:
# D__lsm.10_10 = PHI <_33(8)>
# D__lsm.11_11 = PHI <1(8)>
*_12 = _33;
goto ;
:
return;
...
V.
After second pass_lim/pass_dom pair. Note there are phis on the inner
and outer loop header for the reduction and the iteration variables:
...
:
_5 = *.omp_data_i_4(D).i;
*_5 = 0;
_44 = *.omp_data_i_4(D).n;
_45 = *_44;
if (_45 != 0)
goto ;
else
goto ;
:
_12 = *.omp_data_i_4(D).j;
_19 = *.omp_data_i_4(D).a;
D__lsm.10_50 = *_12;
D__lsm.11_51 = 0;
_25 =
[PATCH][PR tree-optimization/68198] Avoid CFG explosion due to threading
Jump threading inherently copies blocks on the jump threading path. In
the case of all FSM threads (and in one special case in the old
threader), those copies have the same number of outgoing edges as the
original block. We don't see explosions in the old threader because
it's just too weak to create enough of these situations. But the FSM
threader is a different story
So if you have a jump threading path that passes through a block with a
multi-way branch, but does not optimize the multi-way branch itself,
copying can introduced many new edges in the CFG.
In the case of 68198, this resulted in tens of thousands of new edges in
the CFG, which in turn caused a massive slowdown in later passes of the
compiler.
The reality is if we have a multi-way branch in a jump threading path,
but do not optimize the multi-way branch at the end of the path, then
there it's likely we're not gaining much by optimizing that path (ie,
the cost of the embedded multi-way branch dwarfs the gain of eliminating
a single COND_EXPR at the end of the path). So rather than explode the
CFG, we simply don't allow that case to thread.
We accomplish this by distinguishing between multi-way branches at the
end of a jump threading path (ie, those we optimize) vs multi-way
branches that are elsewhere in the path (we pass through and thus have
all those pesky outgoing edges). When we have the latter without the
former, then we disallow threading.
Bootstrapped and regression tested on x86_64-linux-gnu. Installed on
the trunk.
Jeff
commit c8516562e0fdd75144779bf566081a398564ddd4
Author: Jeff Law
Date: Wed Nov 18 17:31:05 2015 -0700
[PATCH][PR tree-optimization/68198] Avoid CFG explosion due to threading
PR tree-optimization/68198
* tree-ssa-threadupdate.c (valid_jump_thread_path): Distinguish
between threading a multi-way branch and a thread path that contains
a multi-way branch. Disallow the case where a path contains a
multi-way branch and does not thread a multi-way branch.
(thread_through_all_blocks): Update comment.
PR tree-optimization/68198
* gcc.dg/tree-ssa/pr66752-3.c: Update expected output for VRP1.
* gcc.dg/tree-ssa/pr68198.c: New test.
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 4b788d3..26ddccc 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,12 @@
+2015-11-18 Jeff Law
+
+ PR tree-optimization/68198
+ * tree-ssa-threadupdate.c (valid_jump_thread_path): Distinguish
+ between threading a multi-way branch and a thread path that contains
+ a multi-way branch. Disallow the case where a path contains a
+ multi-way branch and does not thread a multi-way branch.
+ (thread_through_all_blocks): Update comment.
+
2015-11-18 Joseph Myers
PR c/65083
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 8109c33..48f536f 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2015-11-18 Jeff Law
+
+PR tree-optimization/68198
+ * gcc.dg/tree-ssa/pr66752-3.c: Update expected output for VRP1.
+ * gcc.dg/tree-ssa/pr68198.c: New test.
+
2015-11-18 Steven G. Kargl
PR fortran/59910
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr66752-3.c
b/gcc/testsuite/gcc.dg/tree-ssa/pr66752-3.c
index 577a489..1f27b1a 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/pr66752-3.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr66752-3.c
@@ -32,8 +32,10 @@ foo (int N, int c, int b, int *a)
pt--;
}
-/* There are 3 FSM jump threading opportunities. */
-/* { dg-final { scan-tree-dump-times "FSM" 3 "vrp1"} } */
+/* There are 3 FSM jump threading opportunities, one of which will
+ get cancelled. */
+/* { dg-final { scan-tree-dump-times "Registering FSM" 3 "vrp1"} } */
+/* { dg-final { scan-tree-dump-times "Cancelling FSM" 1 "vrp1"} } */
/* There should be no assignments or references to FLAG. */
/* { dg-final { scan-tree-dump-not "flag" "optimized"} } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr68198.c
b/gcc/testsuite/gcc.dg/tree-ssa/pr68198.c
new file mode 100644
index 000..ddd3c76
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr68198.c
@@ -0,0 +1,43 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-vrp1-details" } */
+
+extern void abort (void);
+
+typedef union tree_node *tree;
+union tree_node
+{
+ int code;
+ tree chain;
+ int omp_code;
+}
+bitmap_head;
+
+extern int c_omp_predetermined_sharing (tree);
+
+tree
+c_finish_omp_clauses (tree clauses)
+{
+ tree c, t, *pc = &clauses;
+ for (pc = &clauses, c = clauses; c; c = *pc)
+{
+ unsigned char remove = 0;
+ switch (((c->omp_code)))
+ {
+ case 1:
+ if (t->code != 42)
+ remove = 1;
+ switch (c_omp_predetermined_sharing (t))
+ {
+ case 2:
+ abort ();
+ }
+ }
+ if (remove)
+ *pc = c->chain;
+}
+}
+
+/*
Re: PR 68393: Handle SUBREG_PROMOTED_VAR_P in expand_direct_optab_fn
On 11/18/2015 05:32 PM, Richard Sandiford wrote: Do the usual dance when assigning to SUBREG_PROMOTED_VAR_P destinations: first convert to the outer mode, then extend to the inner mode. This fixes the powerpc64le bootstrap failure reported in PR 68393. Ok. Bernd
Re: [PATCH] Disable shrink-wrapping for ix86_static_chain_on_stack functions (PR target/67770)
On 11/19/2015 12:18 AM, Jakub Jelinek wrote: As the testcase shows, shrink-wrapping is incompatible with ix86_static_chain_on_stack, where we rely on the very first instruction in the (nested) function to be pushl %esi and use alternate entry point right after that pushl instruction (one byte after the start of the nested function). If shrink-wrapping moves the prologue somewhere else, then this is no longer true. PR target/67770 * config/i386/i386.md (simple_return): Disable if ix86_static_chain_on_stack is true. * gcc.target/i386/pr67770.c: New test. Looks reasonable. Ok. Bernd
Re: [PATCH] Fix ifcvt one_cmpl_abs optimization (PR rtl-optimization/68376)
On 11/18/2015 11:44 PM, Jakub Jelinek wrote: noce_try_abs optimizes these cases normal abs, which doesn't really care if the original condition is x < 0 ? -x : x or x <= 0 ? -x : x, but also the x < 0 ? ~x : x case. But in this case it is significant whether for x == 0 ~x or x applies; the following patch limits the one_cmpl optimization to those cases where the optimized expression computes the same value. Ok. Bernd
Re: Combined constructs' clause splitting
On 19/11/15 01:03, Cesar Philippidis wrote: On 11/08/2015 07:45 AM, Tom de Vries wrote: On 07/11/15 12:45, Thomas Schwinge wrote: Hi! On Fri, 6 Nov 2015 15:31:23 -0800, Cesar Philippidis wrote: I've applied this patch to gomp-4_0-branch which backports most of my front end changes from trunk. Note that I found a regression while testing, which is also present in trunk. It looks like kernels-acc-loop-reduction.c is failing because I'm incorrectly propagating the reduction variable to both to the kernels and loop constructs for combined 'acc kernels loop'. The problem here is that kernels don't support the reduction clause. I'll fix that next week. Always need to consider both what the specification allows -- and thus what the front ends accept/refuse -- as well as what we might do differently, internally in later processing stages. I have not analyzed whether it makes sense to have the OMP_CLAUSE_REDUCTION of a combined "kernels loop reduction([...])" construct be attached to the outer OACC_KERNELS or inner OACC_LOOP, or duplicated for both. Tom, if you need a solution for that right now/want to restore the previous behavior (attached to innter OACC_LOOP only), here's what you should try: in gcc/c-family/c-omp.c:c_oacc_split_loop_clauses remove the special handling for OMP_CLAUSE_REDUCTION, and move it to "Loop clauses" section, Committed to gomp-4_0-branch, as attached. Can you port this patch to trunk? Originally we were attaching the reduction clause to both the acc loop and parallel construct so that the reduction variable would get a copy clause implicitly. However, Nathan later interpreted #pragma acc parallel reduction(+:var) as #pragma acc parallel reduction(+:var) private(var) Therefore, the burden is on the user to ensure that 'var' is transferred to the parallel region in an appropriate data clause. As a result, we only need to associate reductions with loops now. So your patch is good for trunk. Hi Cesar, that patch was submitted for trunk as part of the kernels support: https://gcc.gnu.org/ml/gcc-patches/2015-11/msg00994.html Thanks, - Tom
Re: Combined constructs' clause splitting
On 11/08/2015 07:45 AM, Tom de Vries wrote: > On 07/11/15 12:45, Thomas Schwinge wrote: >> Hi! >> >> On Fri, 6 Nov 2015 15:31:23 -0800, Cesar Philippidis >> wrote: >>> I've applied this patch to gomp-4_0-branch which backports most of my >>> front end changes from trunk. Note that I found a regression while >>> testing, which is also present in trunk. It looks like >>> kernels-acc-loop-reduction.c is failing because I'm incorrectly >>> propagating the reduction variable to both to the kernels and loop >>> constructs for combined 'acc kernels loop'. The problem here is that >>> kernels don't support the reduction clause. I'll fix that next week. >> >> Always need to consider both what the specification allows -- and thus >> what the front ends accept/refuse -- as well as what we might do >> differently, internally in later processing stages. I have not analyzed >> whether it makes sense to have the OMP_CLAUSE_REDUCTION of a combined >> "kernels loop reduction([...])" construct be attached to the outer >> OACC_KERNELS or inner OACC_LOOP, or duplicated for both. >> >> Tom, if you need a solution for that right now/want to restore the >> previous behavior (attached to innter OACC_LOOP only), here's what you >> should try: in gcc/c-family/c-omp.c:c_oacc_split_loop_clauses remove the >> special handling for OMP_CLAUSE_REDUCTION, and move it to "Loop clauses" >> section, > > Committed to gomp-4_0-branch, as attached. Can you port this patch to trunk? Originally we were attaching the reduction clause to both the acc loop and parallel construct so that the reduction variable would get a copy clause implicitly. However, Nathan later interpreted #pragma acc parallel reduction(+:var) as #pragma acc parallel reduction(+:var) private(var) Therefore, the burden is on the user to ensure that 'var' is transferred to the parallel region in an appropriate data clause. As a result, we only need to associate reductions with loops now. So your patch is good for trunk. Cesar
Re: [PATCH] PR fortran/59910 -- structure constructor in DATA statement
On Wed, Nov 18, 2015 at 11:23:21AM -0800, Steve Kargl wrote: > On Tue, Nov 17, 2015 at 05:01:42PM -0800, Steve Kargl wrote: > > On Tue, Nov 17, 2015 at 04:36:01PM -0800, Steve Kargl wrote: > > > On Wed, Nov 18, 2015 at 12:24:29AM +0100, Dominique d'Humières wrote: > > > > > ??? but I suspect gfc_reduce_init_expr() > > > > > may be useful for PARAMETER statements as well (need to > > > > > check this!). > > > > > > > > As in the following test > > > > > > > > module m > > > > implicit none > > > > type t > > > > integer :: i > > > > end type t > > > > type(t), dimension(2), parameter :: a1 = (/ t(1), t(2) /) > > > > type(t), dimension(1), parameter :: c = spread ( a1(1), 1, 1 ) > > > > end module m > > > > > > Yep. We again arrive at gfc_conv_array_initializer with > > > expr->expr_type == EXPR_FUNCTION, which isn't handled correctly. > > > > > > The issue seems deeply rooted in the handling of derived types, > > > which is actually worse than this! But, that is definitely for > > > another day. See PR67817. :( > > > > Ugh. gfc_simplify_spread does not actually the use of SPREAD > > here, because source->expr_type == EXPR_STRUCTURE which is not > > handled. > > Dominiq, > > I plan to commit my patch and close this PR as the patch > fixes the issue raised in the PR. I think the above > code highlights a specific issue with SPREAD, and a new PR > should be committed. This is now PR fortran/68426 -- Steve
Re: Remove noce_mem_write_may_trap_or_fault_p in ifcvt
On 11/18/2015 12:16 PM, Bernd Schmidt wrote:
For the current issue I've come to the conclusion that this kind of
analysis is irrelevant here (and that is not subject to the problem I'll
describe later), because of the use of noce_can_store_speculate. See below.
Hmm
Ripping out noce_mem_write_may_trap_or_fault_p without fixing
may_trap_or_fault_p introduces a latent code code generation issue.
I don't think so, actually. One safe option would be to rip it out and
just stop transforming this case, but let's start by looking at the code
just a bit further down, calling noce_can_store_speculate. This was
added later than the code we're discussing, and it tries to verify that
the same memory location will unconditionally be written to at a point
later than the one we're trying to convert
And if we dig into that thread, Ian suggests this isn't terribly
important anyway.
However, if we go even further upthread, we find an assertion from
Michael that this is critical for 456.hmmer and references BZ 27313.
https://gcc.gnu.org/ml/gcc-patches/2007-08/msg01978.html
Sadly, no testcase was included.
(but why aren't we testing
for prior writes?).
Oversight I suspect. Essentially you want to know if the store is
anticipated (occurs on all paths to a given point). There's a similar
term for always occurs on all paths leaving a given point, but I can't
remember it offhand.
So... I was about to propose the attached patch, which also fixes some
oversights in the can_store_speculate path: we shouldn't allow autoinc
addresses here. The added test in noce_can_store_speculate_p is not
quite necessary given that the same one is also added to
memory_must_be_modified_in_insn_p, but it avoids the need for an insn
walk in cases where it isn't necessary. This bootstrapped and tested ok
on x86_64-linux.
Right.
But then, I looked at noce_can_store_speculate again, and it seems
broken. We walk over the post-dominators of the block, see if we find a
store, and fail if something modifies the address:
Egad. That's clearly wrong. Post domination means that paths to the
exit must go through the post-dominator. A path not
for (dominator = get_immediate_dominator (CDI_POST_DOMINATORS, top_bb);
dominator != NULL;
dominator = get_immediate_dominator (CDI_POST_DOMINATORS,
dominator))
{
rtx_insn *insn;
FOR_BB_INSNS (dominator, insn)
{
[...]
if (modified_in_p (XEXP (mem, 0), insn))
return false;
}
But what if the address is modified, but not in a post-dominator block?
Let's say
if (cond)
*p = x; // this is the store we want to convert
if (other_cond)
p = some_pointer;
else
p = some_other_pointer;
*p = y; // we'll see this store which postdominates the original
// one, but not the modifications of p
Right. You essentially have to check all the blocks in the path. At
which point you might as well just run standard dataflow algorithms
rather than coding up something ad-hoc here.
So I think that algorithm doesn't work. My suggestion at this point
would be to give up on converting stores entirely (deleting
noce_can_store_speculate_p and noce_mem_write_may_trap_or_fault_p) until
someone produces a reasonable scratchpad patch.
So if it weren't for the assertion that it's critical for hmmr, I'd be
convinced that just ripping out was the right thing to do.
Can you look at 27313 and hmmr and see if there's an impact. Just maybe
the critical stuff for those is handled by the tree if converter and we
can just rip out the clearly incorrect RTL bits without regressing
anything performance-wise. If there is an impact, then I think we have
to look at either improving the tree bits (so we can remove the rtl
bits) or we have to do real dataflow analysis in the rtl bits.
jeff
[PATCH] Transactional Memory: Support __cxa_free_exception and fix exception handling.
The EH scheme that we had been using for TM / libitm doesn't work
properly. We fail to handle throwing exceptions whose constructors may
throw themselves. We also do not clean up properly in all situations
when a transactions abort while being in the process of throwing an
exception.
This patch solves this particular problem by adding a transactional
wrapper for __cxa_free_exception and changing the EH scheme in libitm.
Follow-up patches will fix other issues that we have identified. Some
of the changes to the libitm.texi ABI docs added in this patch already
take this future work into account.
Tested using the libitm testsuite on x86_64-linux.
Are the gcc/ bits OK?
gcc/cp/
* except.c (do_free_exception): Use transactional wrapper.
libitm/
* testsuite/libitm.c++/eh-5.C: New.
* libitm.h (_ITM_cxa_free_exception): New.
* libitm.map (_ITM_cxa_free_exception): Add it.
* libitm.texi: Update ABI docs.
* libitm_i.h (gtm_transaction_cp::cxa_unthrown): Remove.
(gtm_transaction_cp::cxa_uncaught_count): Add.
(gtm_thread::cxa_unthrown): Remove.
(gtm_thread::cxa_uncaught_count_ptr): Add.
(gtm_thread::cxa_uncaught_count): Add.
(gtm_thread::drop_references_allocations): Rename to...
(gtm_thread::discard_allocation): ... this and adapt.
(gtm_thread::init_cpp_exceptions): New.
* beginend.cc (gtm_thread::gtm_thread): Adapt EH handling.
(gtm_thread::begin_transaction): Likewise.
(gtm_transaction_cp::save): Likewise.
(gtm_thread::trycommit): Likewise.
* eh_cpp.cc: Add overview comments.
(__cxa_eh_globals, __cxa_get_globals, __cxa_free_exception): Declare.
(free_any_exception, _ITM_cxa_free_exception): New.
(gtm_thread::init_cpp_exceptions): Define.
(_ITM_cxa_allocate_exception, _ITM_cxa_throw): Adapt.
(_ITM_cxa_begin_catch, _ITM_cxa_end_catch): Likewise.
(gtm_thread::revert_cpp_exceptions): Likewise.
commit 23bd34e3c8028a12705a47d13a4c7aa36bfeca60
Author: Torvald Riegel
Date: Tue Nov 3 15:38:22 2015 +0100
Support __cxa_free_exception and fix exception handling.
gcc/cp/
* except.c (do_free_exception): Use transactional wrapper.
libitm/
* testsuite/libitm.c++/eh-5.C: New.
* libitm.h (_ITM_cxa_free_exception): New.
* libitm.map (_ITM_cxa_free_exception): Add it.
* libitm.texi: Update ABI docs.
* libitm_i.h (gtm_transaction_cp::cxa_unthrown): Remove.
(gtm_transaction_cp::cxa_uncaught_count): Add.
(gtm_thread::cxa_unthrown): Remove.
(gtm_thread::cxa_uncaught_count_ptr): Add.
(gtm_thread::cxa_uncaught_count): Add.
(gtm_thread::drop_references_allocations): Rename to...
(gtm_thread::discard_allocation): ... this and adapt.
(gtm_thread::init_cpp_exceptions): New.
* beginend.cc (gtm_thread::gtm_thread): Adapt EH handling.
(gtm_thread::begin_transaction): Likewise.
(gtm_transaction_cp::save): Likewise.
(gtm_thread::trycommit): Likewise.
* eh_cpp.cc: Add overview comments.
(__cxa_eh_globals, __cxa_get_globals, __cxa_free_exception): Declare.
(free_any_exception, _ITM_cxa_free_exception): New.
(gtm_thread::init_cpp_exceptions): Define.
(_ITM_cxa_allocate_exception, _ITM_cxa_throw): Adapt.
(_ITM_cxa_begin_catch, _ITM_cxa_end_catch): Likewise.
(gtm_thread::revert_cpp_exceptions): Likewise.
diff --git a/gcc/cp/except.c b/gcc/cp/except.c
index 9b2450d..ad40436 100644
--- a/gcc/cp/except.c
+++ b/gcc/cp/except.c
@@ -662,6 +662,16 @@ do_free_exception (tree ptr)
/* Declare void __cxa_free_exception (void *) throw(). */
fn = declare_library_fn (fn, void_type_node, ptr_type_node,
ECF_NOTHROW | ECF_LEAF);
+
+ if (flag_tm)
+ {
+ tree fn2 = get_identifier ("_ITM_cxa_free_exception");
+ if (!get_global_value_if_present (fn2, &fn2))
+ fn2 = declare_library_fn (fn2, void_type_node,
+ ptr_type_node,
+ ECF_NOTHROW | ECF_LEAF | ECF_TM_PURE);
+ record_tm_replacement (fn, fn2);
+ }
}
return cp_build_function_call_nary (fn, tf_warning_or_error, ptr, NULL_TREE);
diff --git a/libitm/beginend.cc b/libitm/beginend.cc
index c3ed11b..86f7b39 100644
--- a/libitm/beginend.cc
+++ b/libitm/beginend.cc
@@ -132,6 +132,8 @@ GTM::gtm_thread::gtm_thread ()
number_of_threads_changed(number_of_threads - 1, number_of_threads);
serial_lock.write_unlock ();
+ init_cpp_exceptions ();
+
if (pthread_once(&thr_release_once, thread_exit_init))
GTM_fatal("Initializing thread release TLS key failed.");
// Any non-null value is sufficient to trigger destruction of this
@@ -383,6 +385,11 @@ GTM::gtm_thread::begin_transaction (uint32_t prop, const gtm_jmpbuf *jb)
#endif
}
+ // Log the number of uncaught exceptions if we might have to roll back this
+ // state.
+ if (tx->cxa_uncaught_count_ptr != 0)
+tx->cxa_uncaught_count
[PATCH] Disable shrink-wrapping for ix86_static_chain_on_stack functions (PR target/67770)
Hi!
As the testcase shows, shrink-wrapping is incompatible with
ix86_static_chain_on_stack, where we rely on the very first instruction
in the (nested) function to be pushl %esi and use alternate entry
point right after that pushl instruction (one byte after the start of
the nested function). If shrink-wrapping moves the prologue somewhere else,
then this is no longer true.
Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk/5.3?
2015-11-18 Jakub Jelinek
PR target/67770
* config/i386/i386.md (simple_return): Disable if
ix86_static_chain_on_stack is true.
* gcc.target/i386/pr67770.c: New test.
--- gcc/config/i386/i386.md.jj 2015-11-14 19:36:03.0 +0100
+++ gcc/config/i386/i386.md 2015-11-18 17:49:51.648115096 +0100
@@ -12187,10 +12187,14 @@ (define_expand "return"
;; We need to disable this for TARGET_SEH, as otherwise
;; shrink-wrapped prologue gets enabled too. This might exceed
;; the maximum size of prologue in unwind information.
+;; Also disallow shrink-wrapping if using stack slot to pass the
+;; static chain pointer - the first instruction has to be pushl %esi
+;; and it can't be moved around, as we use alternate entry points
+;; in that case.
(define_expand "simple_return"
[(simple_return)]
- "!TARGET_SEH"
+ "!TARGET_SEH && !ix86_static_chain_on_stack"
{
if (crtl->args.pops_args)
{
--- gcc/testsuite/gcc.target/i386/pr67770.c.jj 2015-11-18 17:38:13.940956205
+0100
+++ gcc/testsuite/gcc.target/i386/pr67770.c 2015-11-18 17:53:22.354143509
+0100
@@ -0,0 +1,40 @@
+/* PR target/67770 */
+/* { dg-do run { target ia32 } } */
+/* { dg-require-effective-target trampolines } */
+/* { dg-options "-O2" } */
+
+#ifndef NO_TRAMPOLINES
+__attribute__ ((noinline)) void
+foo (int i, void (* __attribute__ ((regparm (3))) bar) (int))
+{
+ bar (i);
+}
+#endif
+
+int
+main ()
+{
+#ifndef NO_TRAMPOLINES
+ int p = 0;
+
+ __attribute__ ((regparm (3), noinline)) void
+ bar (int i)
+ {
+if (__builtin_expect (i, 0))
+ ++p;
+ }
+
+ foo (0, bar);
+ bar (0);
+
+ if (p != 0)
+__builtin_abort ();
+
+ foo (1, bar);
+ bar (1);
+
+ if (p != 2)
+__builtin_abort ();
+#endif
+ return 0;
+}
Jakub
Re: [PATCH] Add clang-format config to contrib folder
Martin, thanks for getting this patch out. I like the patch.
Jeff, clang-format has scripts that allow formatting only the lines
touched by a patch.
It also has a script to integrate with git:
https://llvm.org/svn/llvm-project/cfe/trunk/tools/clang-format/git-clang-format
We could use those scripts in a commit hook to automatically enforce
correct formatting of new patches.
Sebastian
On Wed, Nov 18, 2015 at 8:10 AM, Martin Liška wrote:
> Hello.
>
> Following patch adds a clang-format config file that should respect the GNU
> coding standards.
> As the file is not part of build process, I hope the patch can be applied
> even though
> we've just skipped to stage3? The patch adds a hunk to Makefile which can
> create symlink
> to the root directory of the GCC compiler. The clang-format automatically
> loads style from
> the configuration file.
>
> clang-format (version 3.8) provides rich variety of configuration options
> that can
> ensure the GNU coding style.
>
> Limitations:
> + placement of opening brace of an initializer can't be requested
> + sometimes, '(' is the trailing symbol at the end of a line, which can look
> weird
>
> As we've been continuously converting our source base to C++, the
> clang-format should
> provide better results than a collection of regular expressions
> (check_GNU_style.sh).
>
> As a reference file I attach gcc/tree-ssa-uninit.c file.
> Feel free to comment the suggested configuration file.
>
> Thanks,
> Martin
>
[PATCH] Fix libcpp ICE (PR preprocessor/60736)
Hi!
In some cases, e.g. when using #include with
/usr/include/ not containing that header, open_file_failed is called
with NULL file->path; trying to print that doesn't really work, we should
use file->name in that case instead. Though in other cases open_file_failed
is called with both file->name and file->path non-NULL and in that case
file->path is preferrable for the diagnostics.
While looking at this, I've noticed a localization problem.
cpp_errno function hasn't been using _() on the message it printed,
actually, it would localize the "%s: %s" string and not localize
the first string and localize the second one (xstrerror).
As the argument is called msgid, the arguments have been extracted
into the *.pot files.
Furthermore, cpp_errno has been called in some cases with a string literal,
which should be translated, and in other cases with a filename, which should
not be translated.
So, appart from fixing the first issue by using file->path ? file->path :
file->name
in 3 spots instead of just file->path, the patch introduces a new function
like cpp_errno that does not translate the argument (meant to be used with
filenames) and cpp_errno itself (which localizes the argument).
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/5.3?
2015-11-18 Jakub Jelinek
PR preprocessor/60736
* include/cpplib.h (cpp_errno_filename): New prototype.
* errors.c (cpp_errno): Don't handle msgid "" specially, use
_(msgid) instead of msgid as argument to cpp_error.
(cpp_errno_filename): New function.
* files.c (read_file_guts): Use cpp_errno_filename instead of
cpp_errno.
(open_file_failed): Likewise. Use file->name if file->path is NULL
in diagnostics.
--- libcpp/include/cpplib.h.jj 2015-11-14 19:36:30.0 +0100
+++ libcpp/include/cpplib.h 2015-11-18 15:27:47.668327139 +0100
@@ -986,6 +986,9 @@ extern bool cpp_warning_syshdr (cpp_read
/* Output a diagnostic with "MSGID: " preceding the
error string of errno. No location is printed. */
extern bool cpp_errno (cpp_reader *, int, const char *msgid);
+/* Similarly, but with "FILENAME: " instead of "MSGID: ", where
+ the filename is not localized. */
+extern bool cpp_errno_filename (cpp_reader *, int, const char *filename);
/* Same as cpp_error, except additionally specifies a position as a
(translation unit) physical line and physical column. If the line is
--- libcpp/errors.c.jj 2015-11-14 19:36:30.0 +0100
+++ libcpp/errors.c 2015-11-18 15:36:57.091571158 +0100
@@ -230,8 +230,18 @@ cpp_warning_with_line_syshdr (cpp_reader
bool
cpp_errno (cpp_reader *pfile, int level, const char *msgid)
{
- if (msgid[0] == '\0')
-msgid = _("stdout");
+ return cpp_error (pfile, level, "%s: %s", _(msgid), xstrerror (errno));
+}
+
+/* Print a warning or error, depending on the value of LEVEL. Include
+ information from errno. Unlike cpp_errno, the argument is a filename
+ that is not localized, but "" is replaced with localized "stdout". */
+
+bool
+cpp_errno_filename (cpp_reader *pfile, int level, const char *filename)
+{
+ if (filename[0] == '\0')
+filename = _("stdout");
- return cpp_error (pfile, level, "%s: %s", msgid, xstrerror (errno));
+ return cpp_error (pfile, level, "%s: %s", filename, xstrerror (errno));
}
--- libcpp/files.c.jj 2015-05-19 15:52:28.0 +0200
+++ libcpp/files.c 2015-11-18 15:28:25.749789558 +0100
@@ -715,7 +715,7 @@ read_file_guts (cpp_reader *pfile, _cpp_
if (count < 0)
{
- cpp_errno (pfile, CPP_DL_ERROR, file->path);
+ cpp_errno_filename (pfile, CPP_DL_ERROR, file->path);
free (buf);
return false;
}
@@ -1053,7 +1053,8 @@ open_file_failed (cpp_reader *pfile, _cp
/* If the preprocessor output (other than dependency information) is
being used, we must also flag an error. */
if (CPP_OPTION (pfile, deps.need_preprocessor_output))
- cpp_errno (pfile, CPP_DL_FATAL, file->path);
+ cpp_errno_filename (pfile, CPP_DL_FATAL,
+ file->path ? file->path : file->name);
}
else
{
@@ -1067,9 +1068,11 @@ open_file_failed (cpp_reader *pfile, _cp
if (CPP_OPTION (pfile, deps.style) == DEPS_NONE
|| print_dep
|| CPP_OPTION (pfile, deps.need_preprocessor_output))
- cpp_errno (pfile, CPP_DL_FATAL, file->path);
+ cpp_errno_filename (pfile, CPP_DL_FATAL,
+ file->path ? file->path : file->name);
else
- cpp_errno (pfile, CPP_DL_WARNING, file->path);
+ cpp_errno_filename (pfile, CPP_DL_WARNING,
+ file->path ? file->path : file->name);
}
}
Jakub
[C++ PATCH] Issue hard error even with -fpermissive for certain goto violations (PR c++/67409)
Hi!
Before Paolo's changes, e.g. identify_goto always used permerror, and then
depending on the severity of the issue either issued another permerror
describing in detail the issue, or error.
In particular, entering scope of a decl with non-trivial dtor has been
a permerror, pretty much everything else has been a hard error.
The PR Paolo has been fixing was a request to allow with -fpermissive
crossing initialization (i.e. just warn in that case, instead of hard
error), but the patch for consistency turned all the permerror; error
cases to if (permerror ()) inform, which means also other kinds of goto
errors can now be ignored with -fpermissive. But, as the testcase shows,
in certain cases we ICE on it later on; e.g. the OpenMP SESE violations
definitely should be hard errors, because of the ICEs also the entering of
try or catch blocks through goto, TM violations etc.
The following patch arranges for using error instead of permerror
in the case of the more severe violations; if a single goto violates both
something less severe (first) and then something more severe, we can emit
e.g. the jump to label ... diagnostics twice, for non-permissive once
with [-fpermissive] after it, once without, for -fpermissive once as a
warning, once as an error.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/5.3?
2015-11-18 Jakub Jelinek
PR c++/67409
* decl.c (identify_goto): Add harderr argument, call error instead of
permerror if it is true.
(check_previous_goto_1): Adjust identify_goto callers, treat all cases
but crossing initialization and entering scope of decl with non-trivial
dtor as unconditional hard errors.
(check_goto): Treat all cases but crossing initialization and entering
scope of decl with non-trivial dtor as unconditional hard errors.
* g++.dg/eh/goto3.C: New test.
--- gcc/cp/decl.c.jj2015-11-14 19:35:53.0 +0100
+++ gcc/cp/decl.c 2015-11-18 12:30:40.085342627 +0100
@@ -2970,11 +2970,24 @@ decl_jump_unsafe (tree decl)
/* A subroutine of check_previous_goto_1 to identify a branch to the user. */
static bool
-identify_goto (tree decl, const location_t *locus)
+identify_goto (tree decl, const location_t *locus, bool harderr)
{
- bool complained = (decl
-? permerror (input_location, "jump to label %qD", decl)
-: permerror (input_location, "jump to case label"));
+ bool complained;
+ if (!harderr)
+{
+ if (decl)
+ complained = permerror (input_location, "jump to label %qD", decl);
+ else
+ complained = permerror (input_location, "jump to case label");
+}
+ else
+{
+ if (decl)
+ error ("jump to label %qD", decl);
+ else
+ error ("jump to case label");
+ complained = true;
+}
if (complained && locus)
inform (*locus, " from here");
return complained;
@@ -2991,15 +3004,16 @@ check_previous_goto_1 (tree decl, cp_bin
bool exited_omp, const location_t *locus)
{
cp_binding_level *b;
- bool identified = false, complained = false;
+ bool complained = false;
+ int identified = 0;
bool saw_eh = false, saw_omp = false, saw_tm = false;
if (exited_omp)
{
- complained = identify_goto (decl, locus);
+ complained = identify_goto (decl, locus, true);
if (complained)
inform (input_location, " exits OpenMP structured block");
- identified = saw_omp = true;
+ identified = (saw_omp = true) ? 2 : 0;
}
for (b = current_binding_level; b ; b = b->level_chain)
@@ -3016,8 +3030,8 @@ check_previous_goto_1 (tree decl, cp_bin
if (!identified)
{
- complained = identify_goto (decl, locus);
- identified = true;
+ complained = identify_goto (decl, locus, false);
+ identified = 1;
}
if (complained)
{
@@ -3035,10 +3049,10 @@ check_previous_goto_1 (tree decl, cp_bin
break;
if ((b->kind == sk_try || b->kind == sk_catch) && !saw_eh)
{
- if (!identified)
+ if (identified < 2)
{
- complained = identify_goto (decl, locus);
- identified = true;
+ complained = identify_goto (decl, locus, true);
+ identified = 2;
}
if (complained)
{
@@ -3051,10 +3065,10 @@ check_previous_goto_1 (tree decl, cp_bin
}
if (b->kind == sk_omp && !saw_omp)
{
- if (!identified)
+ if (identified < 2)
{
- complained = identify_goto (decl, locus);
- identified = true;
+ complained = identify_goto (decl, locus, true);
+ identified = 2;
}
if (complained)
inform (input_location, " enters OpenMP structured block");
@@ -3062,10 +3076,10 @@ check_previous_goto_1 (tree decl, cp_bin
}
[PATCH] Fix ifcvt one_cmpl_abs optimization (PR rtl-optimization/68376)
Hi!
noce_try_abs optimizes these cases normal abs, which doesn't really
care if the original condition is x < 0 ? -x : x or x <= 0 ? -x : x,
but also the x < 0 ? ~x : x case. But in this case it is significant
whether for x == 0 ~x or x applies; the following patch limits the
one_cmpl optimization to those cases where the optimized expression
computes the same value.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/5.3?
2015-11-18 Jakub Jelinek
PR rtl-optimization/68376
* ifcvt.c (noce_try_abs): Disable one_cmpl optimization if
encountering x <= 0 ? ~x : x or x > 0 ? ~x : x.
* gcc.c-torture/execute/pr68376-1.c: New test.
* gcc.c-torture/execute/pr68376-2.c: New test.
--- gcc/ifcvt.c.jj 2015-11-14 19:35:54.0 +0100
+++ gcc/ifcvt.c 2015-11-18 11:02:48.645771477 +0100
@@ -2595,12 +2595,49 @@ noce_try_abs (struct noce_if_info *if_in
/* Work around funny ideas get_condition has wrt canonicalization.
Note that these rtx constants are known to be CONST_INT, and
- therefore imply integer comparisons. */
+ therefore imply integer comparisons.
+ The one_cmpl case is more complicated, as we want to handle
+ only x < 0 ? ~x : x or x >= 0 ? ~x : x but not
+ x <= 0 ? ~x : x or x > 0 ? ~x : x, as the latter two
+ have different result for x == 0. */
if (c == constm1_rtx && GET_CODE (cond) == GT)
-;
+{
+ if (one_cmpl && negate)
+ return FALSE;
+}
else if (c == const1_rtx && GET_CODE (cond) == LT)
-;
- else if (c != CONST0_RTX (GET_MODE (b)))
+{
+ if (one_cmpl && !negate)
+ return FALSE;
+}
+ else if (c == CONST0_RTX (GET_MODE (b)))
+{
+ if (one_cmpl)
+ switch (GET_CODE (cond))
+ {
+ case GT:
+ if (!negate)
+ return FALSE;
+ break;
+ case GE:
+ /* >= 0 is the same case as above > -1. */
+ if (negate)
+ return FALSE;
+ break;
+ case LT:
+ if (negate)
+ return FALSE;
+ break;
+ case LE:
+ /* <= 0 is the same case as above < 1. */
+ if (!negate)
+ return FALSE;
+ break;
+ default:
+ return FALSE;
+ }
+}
+ else
return FALSE;
/* Determine what sort of operation this is. */
--- gcc/testsuite/gcc.c-torture/execute/pr68376-1.c.jj 2015-11-18
11:12:33.251522987 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr68376-1.c 2015-11-18
11:12:24.0 +0100
@@ -0,0 +1,24 @@
+/* PR rtl-optimization/68376 */
+
+int a, b, c = 1;
+signed char d;
+
+int
+main ()
+{
+ for (; a < 1; a++)
+for (; b < 1; b++)
+ {
+ signed char e = ~d;
+ if (d < 1)
+ e = d;
+ d = e;
+ if (!c)
+ __builtin_abort ();
+ }
+
+ if (d != 0)
+__builtin_abort ();
+
+ return 0;
+}
--- gcc/testsuite/gcc.c-torture/execute/pr68376-2.c.jj 2015-11-18
11:12:36.209481252 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr68376-2.c 2015-11-18
11:10:55.0 +0100
@@ -0,0 +1,41 @@
+/* PR rtl-optimization/68376 */
+
+extern void abort (void);
+
+__attribute__((noinline, noclone)) int
+f1 (int x)
+{
+ return x < 0 ? ~x : x;
+}
+
+__attribute__((noinline, noclone)) int
+f2 (int x)
+{
+ return x < 0 ? x : ~x;
+}
+
+__attribute__((noinline, noclone)) int
+f3 (int x)
+{
+ return x <= 0 ? ~x : x;
+}
+
+__attribute__((noinline, noclone)) int
+f4 (int x)
+{
+ return x <= 0 ? x : ~x;
+}
+
+int
+main ()
+{
+ if (f1 (5) != 5 || f1 (-5) != 4 || f1 (0) != 0)
+abort ();
+ if (f2 (5) != -6 || f2 (-5) != -5 || f2 (0) != -1)
+abort ();
+ if (f3 (5) != 5 || f3 (-5) != 4 || f3 (0) != -1)
+abort ();
+ if (f4 (5) != -6 || f4 (-5) != -5 || f4 (0) != 0)
+abort ();
+ return 0;
+}
Jakub
Re: [C PATCH] Don't leak C_MAYBE_CONST_EXPRs into fold() (PR c/68412)
On Wed, 18 Nov 2015, Marek Polacek wrote: > diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c > index c18c307..48c1a98 100644 > --- gcc/c/c-typeck.c > +++ gcc/c/c-typeck.c > @@ -3512,7 +3512,11 @@ parser_build_binary_op (location_t location, enum > tree_code code, > code1, arg1.value, code2, arg2.value); > >if (warn_tautological_compare) > -warn_tautological_cmp (location, code, arg1.value, arg2.value); > +warn_tautological_cmp (location, code, > +/* This function will try to fold both > + args, so don't leak C_MAYBE_CONST_EXPR. */ > +remove_c_maybe_const_expr (arg1.value), > +remove_c_maybe_const_expr (arg2.value)); remove_c_maybe_const_expr doesn't seem to be quite what you want. Apart from this not being a case covered by the comment on the function, you're ignoring the possibility of the side effects in the C_MAYBE_CONST_EXPR_PRE. So I think you want something else: if either argument is a C_MAYBE_CONST_EXPR whose C_MAYBE_CONST_EXPR_PRE is non-NULL and has side-effects, don't run the comparison, and otherwise it's OK to go down to the C_MAYBE_CONST_EXPR_EXPR. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH 01/02] PR/62314: add ability to add fixit-hints
On Wed, 2015-11-18 at 14:57 -0700, Jeff Law wrote:
> On 11/10/2015 09:35 AM, David Malcolm wrote:
> > This patch adds the ability to add "fix-it hints" to a rich_location,
> > which will be displayed when the corresponding diagnostic is printed.
> >
> > It does not actually add any fix-it hints (that comes in the second
> > patch), but it adds test coverage of the machinery and printing,
> > by using the existing diagnostic_plugin_test_show_locus to inject
> > some meaningless fixit hints, and to verify the output.
> >
> > For now, add a nasty linker kludge in layout::print_any_fixits for
> > the sake of diagnostic_plugin_test_show_locus.
> >
> > Successfully bootstrapped®rtested the pair of patches on
> > x86_64-pc-linux-gnu (on top of the 10-patch diagnostics kit).
> >
> > OK for trunk?
> >
> > gcc/ChangeLog:
> > PR/62314
> > * diagnostic-show-locus.c (colorizer::set_fixit_hint): New.
> > (class layout): Update comment
> > (layout::print_any_fixits): New method.
> > (layout::move_to_column): New method.
> > (diagnostic_show_locus): Add call to layout.print_any_fixits.
> >
> > gcc/testsuite/ChangeLog:
> > PR/62314
> > * gcc.dg/plugin/diagnostic-test-show-locus-ascii-bw.c
> > (test_fixit_insert): New.
> > (test_fixit_remove): New.
> > (test_fixit_replace): New.
> > * gcc.dg/plugin/diagnostic-test-show-locus-ascii-color.c
> > (test_fixit_insert): New.
> > (test_fixit_remove): New.
> > (test_fixit_replace): New.
> > * gcc.dg/plugin/diagnostic_plugin_test_show_locus.c
> > (test_show_locus): Add tests of rendering fixit hints.
> >
> > libcpp/ChangeLog:
> > PR/62314
> > * include/line-map.h (source_range::intersects_line_p): New
> > method.
> > (rich_location::~rich_location): New.
> > (rich_location::add_fixit_insert): New method.
> > (rich_location::add_fixit_remove): New method.
> > (rich_location::add_fixit_replace): New method.
> > (rich_location::get_num_fixit_hints): New accessor.
> > (rich_location::get_fixit_hint): New accessor.
> > (rich_location::MAX_FIXIT_HINTS): New constant.
> > (rich_location::m_num_fixit_hints): New field.
> > (rich_location::m_fixit_hints): New field.
> > (class fixit_hint): New class.
> > (class fixit_insert): New class.
> > (class fixit_remove): New class.
> > (class fixit_replace): New class.
> > * line-map.c (source_range::intersects_line_p): New method.
> > (rich_location::rich_location): Add initialization of
> > m_num_fixit_hints to both ctors.
> > (rich_location::~rich_location): New.
> > (rich_location::add_fixit_insert): New method.
> > (rich_location::add_fixit_remove): New method.
> > (rich_location::add_fixit_replace): New method.
> > (fixit_insert::fixit_insert): New.
> > (fixit_insert::~fixit_insert): New.
> > (fixit_insert::affects_line_p): New.
> > (fixit_remove::fixit_remove): New.
> > (fixit_remove::affects_line_p): New.
> > (fixit_replace::fixit_replace): New.
> > (fixit_replace::~fixit_replace): New.
> > (fixit_replace::affects_line_p): New.
>
> > +
> > + /* Nasty workaround to convince the linker to add
> > + rich_location::add_fixit_insert
> > + rich_location::add_fixit_remove
> > + rich_location::add_fixit_replace
> > + to cc1 for use by diagnostic_plugin_test_show_locus,
> > + before anything in cc1 is using them.
> > +
> > + This conditional should never hold, but hopefully the compiler can't
> > + figure that out. */
> > + if ('.' == global_dc->caret_chars[0])
> > +{
> > + rich_location dummy (line_table, UNKNOWN_LOCATION);
> > + dummy.add_fixit_insert (UNKNOWN_LOCATION, "");
> > + dummy.add_fixit_remove
> > + (source_range::from_location (UNKNOWN_LOCATION));
> > + dummy.add_fixit_replace
> > + (source_range::from_location (UNKNOWN_LOCATION), "");
> > +}
> > +}
> So "the kludge" is presumably going to be removed based on later
> comments in this patch's thread.
Yes.
> What is the purpose of the #if 0 code in the various tests? Did you
> mean to leave those in?
Presumably you're referring to the bodies of the functions
test_fixit_insert
test_fixit_remove
test_fixit_replace
within:
gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-bw.c
gcc/testsuite/gcc.dg/plugin/diagnostic-test-show-locus-color.c
where the bodies are purely of the form:
{
#if 0
some code, containing dejagnu directives.
#endif
}
Although this looks weird, it's deliberate, and follows the pattern
earlier in those test files: the diagnostics are injected by the plugin,
not by cc1. The plugin gives us a way of unit-testing how
diagnostic_show_locus handles the various ways of calling into the
diagnostic API, isolating it from the details of any particular real
diagnostic in cc1, and from the details of how to get real
location/range information.
Hence we inject calls to the diagnostic API via the plugin,
[Ada] Fix missing layout information in ASIS mode
We simply fail to recurse into nested packages.
Tested on x86_64-suse-linux, applied on the mainline.
2015-11-18 Eric Botcazou
* gcc-interface/trans.c (elaborate_all_entities_for_package): New
function extracted from... Recurse on packages.
(elaborate_all_entities): ...here. Call it on packages.
--
Eric BotcazouIndex: gcc-interface/trans.c
===
--- gcc-interface/trans.c (revision 230575)
+++ gcc-interface/trans.c (working copy)
@@ -8353,7 +8353,69 @@ gnat_gimplify_stmt (tree *stmt_p)
}
}
-/* Force references to each of the entities in packages withed by GNAT_NODE.
+/* Force a reference to each of the entities in GNAT_PACKAGE recursively.
+
+ This routine is exclusively called in type_annotate mode, to compute DDA
+ information for types in withed units, for ASIS use. */
+
+static void
+elaborate_all_entities_for_package (Entity_Id gnat_package)
+{
+ Entity_Id gnat_entity;
+
+ for (gnat_entity = First_Entity (gnat_package);
+ Present (gnat_entity);
+ gnat_entity = Next_Entity (gnat_entity))
+{
+ const Entity_Kind kind = Ekind (gnat_entity);
+
+ /* We are interested only in entities visible from the main unit. */
+ if (!Is_Public (gnat_entity))
+ continue;
+
+ /* Skip stuff internal to the compiler. */
+ if (Convention (gnat_entity) == Convention_Intrinsic)
+ continue;
+ if (kind == E_Operator)
+ continue;
+ if (IN (kind, Subprogram_Kind) && Is_Intrinsic_Subprogram (gnat_entity))
+ continue;
+
+ /* Skip named numbers. */
+ if (IN (kind, Named_Kind))
+ continue;
+
+ /* Skip generic declarations. */
+ if (IN (kind, Generic_Unit_Kind))
+ continue;
+
+ /* Skip package bodies. */
+ if (kind == E_Package_Body)
+ continue;
+
+ /* Skip limited views that point back to the main unit. */
+ if (IN (kind, Incomplete_Kind)
+ && From_Limited_With (gnat_entity)
+ && In_Extended_Main_Code_Unit (Non_Limited_View (gnat_entity)))
+ continue;
+
+ /* Skip types that aren't frozen. */
+ if (IN (kind, Type_Kind) && !Is_Frozen (gnat_entity))
+ continue;
+
+ /* Recurse on real packages that aren't in the main unit. */
+ if (kind == E_Package)
+ {
+ if (No (Renamed_Entity (gnat_entity))
+ && !In_Extended_Main_Code_Unit (gnat_entity))
+ elaborate_all_entities_for_package (gnat_entity);
+ }
+ else
+ gnat_to_gnu_entity (gnat_entity, NULL_TREE, 0);
+}
+}
+
+/* Force a reference to each of the entities in packages withed by GNAT_NODE.
Operate recursively but check that we aren't elaborating something more
than once.
@@ -8363,7 +8425,7 @@ gnat_gimplify_stmt (tree *stmt_p)
static void
elaborate_all_entities (Node_Id gnat_node)
{
- Entity_Id gnat_with_clause, gnat_entity;
+ Entity_Id gnat_with_clause;
/* Process each unit only once. As we trace the context of all relevant
units transitively, including generic bodies, we may encounter the
@@ -8381,35 +8443,17 @@ elaborate_all_entities (Node_Id gnat_nod
&& !present_gnu_tree (Library_Unit (gnat_with_clause))
&& Library_Unit (gnat_with_clause) != Library_Unit (Cunit (Main_Unit)))
{
- elaborate_all_entities (Library_Unit (gnat_with_clause));
+ Node_Id gnat_unit = Library_Unit (gnat_with_clause);
+ Entity_Id gnat_entity = Entity (Name (gnat_with_clause));
- if (Ekind (Entity (Name (gnat_with_clause))) == E_Package)
- {
- for (gnat_entity = First_Entity (Entity (Name (gnat_with_clause)));
- Present (gnat_entity);
- gnat_entity = Next_Entity (gnat_entity))
- if (Is_Public (gnat_entity)
- && Convention (gnat_entity) != Convention_Intrinsic
- && Ekind (gnat_entity) != E_Package
- && Ekind (gnat_entity) != E_Package_Body
- && Ekind (gnat_entity) != E_Operator
- && !(IN (Ekind (gnat_entity), Type_Kind)
- && !Is_Frozen (gnat_entity))
- && !(IN (Ekind (gnat_entity), Incomplete_Kind)
- && From_Limited_With (gnat_entity)
- && In_Extended_Main_Code_Unit
- (Non_Limited_View (gnat_entity)))
- && !((Ekind (gnat_entity) == E_Procedure
- || Ekind (gnat_entity) == E_Function)
- && Is_Intrinsic_Subprogram (gnat_entity))
- && !IN (Ekind (gnat_entity), Named_Kind)
- && !IN (Ekind (gnat_entity), Generic_Unit_Kind))
- gnat_to_gnu_entity (gnat_entity, NULL_TREE, 0);
- }
- else if (Ekind (Entity (Name (gnat_with_clause))) == E_Generic_Package)
+ elaborate_all_entities (gnat_unit);
+
+ if (Ekind (gnat_entity) == E_Package)
+ elaborate_all_entities_for_package (gnat_entity);
+
+ else if (Ekind (gnat_entity) == E_Generic_Package)
{
- Node_Id gnat_body
- = Corresponding_Body (Unit (Library_Unit (gnat_with_clause)));
+ Node_Id gnat_body = Corresponding_Body (Unit (gnat_unit));
/* Retrieve compilation unit node of generic body. */
while (Present (gnat_body)
Re: Add out-of-line versions of some functions (PR c/65083)
On 11/13/2015 03:15 AM, Joseph Myers wrote: PR c/65083 notes that some functions in are normal functions, not generic functions, and so need to have out-of-line copies that can be called when macro expansion is suppressed (unlike the generic functions where DR#419 makes it undefined if you suppress a macro expansion). This patch adds such out-of-line definitions in libatomic for those six functions, at a new LIBATOMIC_1.2 symbol version, as trivial wrappers to the macros, along with declarations of those functions in . Tests are added that are based on the corresponding tests for the macros, but with parentheses around the function names to force the out-of-line functions to be used. Bootstrapped with no regressions on x86_64-pc-linux-gnu. OK to commit? gcc: 2015-11-13 Joseph Myers PR c/65083 * ginclude/stdatomic.h (atomic_thread_fence, atomic_signal_fence) (atomic_flag_test_and_set, atomic_flag_test_and_set_explicit) (atomic_flag_clear, atomic_flag_clear_explicit): Declare as functions before defining as macros. gcc/testsuite: 2015-11-13 Joseph Myers PR c/65083 * gcc.dg/atomic/stdatomic-fence-2.c, gcc.dg/atomic/stdatomic-flag-2.c: New tests. libatomic: 2015-11-13 Joseph Myers PR c/65083 * fence.c, flag.c: New files. * Makefile.am (libatomic_la_SOURCES): Add fence.c and flag.c. * Makefile.in: Regenerate. * configure.ac (libtool_VERSION): Change to 3:0:2. * configure: Regenerate. * libatomic.map (LIBATOMIC_1.2): New symbol version. OK. Thanks, Jeff
Re: [PATCH 01/02] PR/62314: add ability to add fixit-hints
On 11/10/2015 09:35 AM, David Malcolm wrote:
This patch adds the ability to add "fix-it hints" to a rich_location,
which will be displayed when the corresponding diagnostic is printed.
It does not actually add any fix-it hints (that comes in the second
patch), but it adds test coverage of the machinery and printing,
by using the existing diagnostic_plugin_test_show_locus to inject
some meaningless fixit hints, and to verify the output.
For now, add a nasty linker kludge in layout::print_any_fixits for
the sake of diagnostic_plugin_test_show_locus.
Successfully bootstrapped®rtested the pair of patches on
x86_64-pc-linux-gnu (on top of the 10-patch diagnostics kit).
OK for trunk?
gcc/ChangeLog:
PR/62314
* diagnostic-show-locus.c (colorizer::set_fixit_hint): New.
(class layout): Update comment
(layout::print_any_fixits): New method.
(layout::move_to_column): New method.
(diagnostic_show_locus): Add call to layout.print_any_fixits.
gcc/testsuite/ChangeLog:
PR/62314
* gcc.dg/plugin/diagnostic-test-show-locus-ascii-bw.c
(test_fixit_insert): New.
(test_fixit_remove): New.
(test_fixit_replace): New.
* gcc.dg/plugin/diagnostic-test-show-locus-ascii-color.c
(test_fixit_insert): New.
(test_fixit_remove): New.
(test_fixit_replace): New.
* gcc.dg/plugin/diagnostic_plugin_test_show_locus.c
(test_show_locus): Add tests of rendering fixit hints.
libcpp/ChangeLog:
PR/62314
* include/line-map.h (source_range::intersects_line_p): New
method.
(rich_location::~rich_location): New.
(rich_location::add_fixit_insert): New method.
(rich_location::add_fixit_remove): New method.
(rich_location::add_fixit_replace): New method.
(rich_location::get_num_fixit_hints): New accessor.
(rich_location::get_fixit_hint): New accessor.
(rich_location::MAX_FIXIT_HINTS): New constant.
(rich_location::m_num_fixit_hints): New field.
(rich_location::m_fixit_hints): New field.
(class fixit_hint): New class.
(class fixit_insert): New class.
(class fixit_remove): New class.
(class fixit_replace): New class.
* line-map.c (source_range::intersects_line_p): New method.
(rich_location::rich_location): Add initialization of
m_num_fixit_hints to both ctors.
(rich_location::~rich_location): New.
(rich_location::add_fixit_insert): New method.
(rich_location::add_fixit_remove): New method.
(rich_location::add_fixit_replace): New method.
(fixit_insert::fixit_insert): New.
(fixit_insert::~fixit_insert): New.
(fixit_insert::affects_line_p): New.
(fixit_remove::fixit_remove): New.
(fixit_remove::affects_line_p): New.
(fixit_replace::fixit_replace): New.
(fixit_replace::~fixit_replace): New.
(fixit_replace::affects_line_p): New.
+
+ /* Nasty workaround to convince the linker to add
+ rich_location::add_fixit_insert
+ rich_location::add_fixit_remove
+ rich_location::add_fixit_replace
+ to cc1 for use by diagnostic_plugin_test_show_locus,
+ before anything in cc1 is using them.
+
+ This conditional should never hold, but hopefully the compiler can't
+ figure that out. */
+ if ('.' == global_dc->caret_chars[0])
+{
+ rich_location dummy (line_table, UNKNOWN_LOCATION);
+ dummy.add_fixit_insert (UNKNOWN_LOCATION, "");
+ dummy.add_fixit_remove
+ (source_range::from_location (UNKNOWN_LOCATION));
+ dummy.add_fixit_replace
+ (source_range::from_location (UNKNOWN_LOCATION), "");
+}
+}
So "the kludge" is presumably going to be removed based on later
comments in this patch's thread.
What is the purpose of the #if 0 code in the various tests? Did you
mean to leave those in?
I think this is probably OK. Though I am concerned that with blobs of
the tests commented out that it's not being tested as thoroughly as you
think it has :-)
Jeff
[Ada] Remove more redundant checks in loops
This improves the infrastructure present in gigi to remove redundant checks,
i.e. checks that can never fail at run time, in loops, either directly or by
exposing more opportunities for the GIMPLE optimizers. The patch is large but
it also cleans up the GENERIC code generated by gigi, in particular removes a
bunch of superfluous VIEW_CONVERT_EXPRs, so it's worth having IMO.
Tested on x86_64-suse-linux, applied on the mainline.
2015-11-18 Eric Botcazou
* gcc-interface/ada-tree.h (DECL_INVARIANT_P): New macro.
* gcc-interface/gigi.h (enum standard_datatypes): Remove
ADT_longjmp_decl and add ADT_not_handled_by_others_decl.
(longjmp_decl): Delete.
(not_handled_by_others_decl): New macro.
(build_simple_component_ref): Delete.
(build_component_ref): Adjust prototype.
* gcc-interface/decl.c (gnat_to_gnu_entity): Adjust calls to
build_component_ref.
(gnat_to_gnu_field): Set DECL_INVARIANT_P on discriminants
without default value.
* gcc-interface/trans.c (gigi): Reorder initialization sequence
and add not_handled_by_others_decl.
(Attribute_to_gnu): Adjust calls to build_component_ref.
(Subprogram_Body_to_gnu): Likewise.
(Call_to_gnu): Likewise.
(Exception_Handler_to_gnu_sjlj): Likewise.
(gnat_to_gnu): Likewise.
(range_check_info_d): Add inserted_cond field.
(Loop_Statement_to_gnu): Make two passes on the recorded range checks.
(build_noreturn_cond): New static function.
(Raise_Error_to_gnu): Record range checks in loops at -O1 and above.
(make_invariant): New static function.
(Loop_Statement_to_gnu): Use it to compute invariant expressions for
the loop bounds if possible, but do not require it if loop unswitching
is enabled.
* gcc-interface/utils.c (convert_to_fat_pointer): Likewise.
(convert): Likewise.
(maybe_unconstrained_array): Likewise. Call it instead of
build_simple_component_ref and add guard for CONSTRUCTORs.
(unchecked_convert): Likewise.
* gcc-interface/utils2.c (compare_fat_pointers): Likewise.
(build_simple_component_ref): Remove COMPONENT parameter, unify
code dealing with VIEW_CONVERT_EXPR and make it more general,
remove special treatment for CONSTRUCTORs of template types.
(build_component_ref): Remove COMPONENT parameter and adjust call
to build_simple_component_ref.
(maybe_wrap_malloc): Likewise.
(build_allocator): Likewise.
(gnat_invariant_expr): Look through overflow checks, deal with
addition and subtraction of constants and take into account
DECL_INVARIANT_P for the COMPONENT_REF case.
2015-11-18 Eric Botcazou
* gnat.dg/loop_optimization19.adb: New test.
* gnat.dg/loop_optimization20.adb: Likewise.
* gnat.dg/loop_optimization21.ad[sb]: Likewise.
--
Eric Botcazou-- { dg-do compile }
-- { dg-options "-O -fdump-tree-optimized" }
procedure Loop_Optimization19 is
type Array_T is array (Positive range <>) of Integer;
type Obj_T (Length : Natural) is
record
Elements : Array_T (1 .. Length);
end record;
type T is access Obj_T;
function Equal (S1, S2 : T) return Boolean;
pragma No_Inline (Equal);
function Equal (S1, S2 : T) return Boolean is
begin
if S1.Length = S2.Length then
for I in 1 .. S1.Length loop
if S1.Elements (I) /= S2.Elements (I) then
return False;
end if;
end loop;
return True;
else
return False;
end if;
end Equal;
A : T := new Obj_T (Length => 10);
B : T := new Obj_T (Length => 20);
C : T := new Obj_T (Length => 30);
begin
if Equal (A, B) then
raise Program_Error;
else
if Equal (B, C) then
raise Program_Error;
end if;
end if;
end;
-- { dg-final { scan-tree-dump-not "Index_Check" "optimized" } }
-- { dg-do compile }
-- { dg-options "-O -fdump-tree-optimized" }
procedure Loop_Optimization20 is
type Array_T is array (Positive range <>) of Integer;
type Obj_T (Length : Natural) is
record
Elements : Array_T (1 .. Length);
end record;
type T is access Obj_T;
function Is_Null (S1 : Obj_T) return Boolean;
pragma No_Inline (Is_Null);
function Is_Null (S1 : Obj_T) return Boolean is
begin
for I in 1 .. S1.Length loop
if S1.Elements (I) /= 0 then
return False;
end if;
end loop;
return True;
end;
A : T := new Obj_T'(Length => 10, Elements => (others => 0));
begin
if not Is_Null (A.all) then
raise Program_Error;
end if;
end;
-- { dg-final { scan-tree-dump-not "Index_Check" "optimized" } }
-- { dg-do compile }
-- { dg-options "-O -fdump-tree-optimized" }
package body Loop_Optimization21 is
function Min (X : in Item_Vector) return Item is
Tmp_Min : Item;
begin
Tmp_Min := X (X'First);
for I in X'First + 1
Re: POWERPC64_TOC_POINTER_ALIGNMENT
On Wed, Nov 18, 2015 at 10:35:36AM -0500, David Edelsohn wrote:
> On Tue, Nov 17, 2015 at 9:32 PM, Alan Modra wrote:
> > On Tue, Nov 17, 2015 at 07:53:18PM -0500, Michael Meissner wrote:
> >> Here is the temporary patch I'm using to get past rs6000.c. But I suspect
> >> the
> >> TOC alignment should never be 256.
> >
> > Yes, it should be. Recent GNU ld aligns .TOC. to a 256 byte boundary.
> > I have this patch in my tree.
> >
> > diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> > index abc8eaa..e3ec042 100644
> > --- a/gcc/config/rs6000/rs6000.c
> > +++ b/gcc/config/rs6000/rs6000.c
> > @@ -8059,12 +8059,17 @@ rs6000_cannot_force_const_mem (machine_mode mode
> > ATTRIBUTE_UNUSED, rtx x)
> > static bool
> > use_toc_relative_ref (rtx sym, machine_mode mode)
> > {
> > + /* Silence complaint that the POWERPC64_TOC_POINTER_ALIGNMENT test
> > + is always true. */
> > +#pragma GCC diagnostic push
> > +#pragma GCC diagnostic ignored "-Wtype-limits"
> >return ((constant_pool_expr_p (sym)
> >&& ASM_OUTPUT_SPECIAL_POOL_ENTRY_P (get_pool_constant (sym),
> >get_pool_mode (sym)))
> > || (TARGET_CMODEL == CMODEL_MEDIUM
> > && SYMBOL_REF_LOCAL_P (sym)
> > && GET_MODE_SIZE (mode) <= POWERPC64_TOC_POINTER_ALIGNMENT));
> > +#pragma GCC diagnostic pop
> > }
> >
> > /* Our implementation of LEGITIMIZE_RELOAD_ADDRESS. Returns a value to
>
> I have applied Alan's work-around for now until there is a more robust
> solution.
>
> Thanks, David
>
Note, the patch that was applied breaks being compiled with older compilers,
such as the GCC 4.4.7 compiler that is installed on my x86_64 laptop, and which
I build powerpc cross compilers from. The older GCC does not allow these
pragmas to be used in the middle of a function.
I think the following patch should be applied to move the #pragmas outside of
the function.
Note, you will need the patch that is attached to PR 68393 in order to
bootstrap the compiler.
2015-11-18 Michael Meissner
* config/rs6000/rs6000.c (use_toc_relative_ref): Move #pragmas
outside of the function to accomidate older GCC's.
--
Michael Meissner, IBM
IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA
email: meiss...@linux.vnet.ibm.com, phone: +1 (978) 899-4797
Index: gcc/config/rs6000/rs6000.c
===
--- gcc/config/rs6000/rs6000.c (revision 230573)
+++ gcc/config/rs6000/rs6000.c (working copy)
@@ -7991,22 +7991,24 @@ rs6000_cannot_force_const_mem (machine_m
that we have put in the TOC, or for cmodel=medium, if the SYMBOL_REF
can be addressed relative to the toc pointer. */
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wtype-limits"
+
static bool
use_toc_relative_ref (rtx sym, machine_mode mode)
{
/* Silence complaint that the POWERPC64_TOC_POINTER_ALIGNMENT test
is always true. */
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wtype-limits"
return ((constant_pool_expr_p (sym)
&& ASM_OUTPUT_SPECIAL_POOL_ENTRY_P (get_pool_constant (sym),
get_pool_mode (sym)))
|| (TARGET_CMODEL == CMODEL_MEDIUM
&& SYMBOL_REF_LOCAL_P (sym)
&& GET_MODE_SIZE (mode) <= POWERPC64_TOC_POINTER_ALIGNMENT));
-#pragma GCC diagnostic pop
}
+#pragma GCC diagnostic pop
+
/* Our implementation of LEGITIMIZE_RELOAD_ADDRESS. Returns a value to
replace the input X, or the original X if no replacement is called for.
The output parameter *WIN is 1 if the calling macro should goto WIN,
[gomp4] backport ptx changes from trunk
I've committed this to gomp4 branch. It removes some extraneous pieces from
nvptx.c and backports comments and some minor reworking that got applied to trunk.
nathan
(nvptx_neuter_pars): Backport from trunk.
Index: gcc/config/nvptx/nvptx.c
===
--- gcc/config/nvptx/nvptx.c (revision 230571)
+++ gcc/config/nvptx/nvptx.c (working copy)
@@ -66,14 +66,6 @@
#include "tree-phinodes.h"
#include "cfgloop.h"
#include "fold-const.h"
-#include "cfghooks.h"
-#include "dumpfile.h"
-#include "dominance.h"
-#include "cfg.h"
-#include "tree-cfg.h"
-#include "gimple-ssa.h"
-#include "ssa-iterators.h"
-#include "tree-into-ssa.h"
/* This file should be included last. */
#include "target-def.h"
@@ -2003,7 +1995,6 @@ nvptx_print_operand_address (FILE *file,
A -- print an address space identifier for a MEM
c -- print an opcode suffix for a comparison operator, including a type code
f -- print a full reg even for something that must always be split
- R -- print an address space specified by CONST_INT
S -- print a shuffle kind specified by CONST_INT
t -- print a type opcode suffix, promoting QImode to 32 bits
T -- print a type size in bits
@@ -2056,13 +2047,6 @@ nvptx_print_operand (FILE *file, rtx x,
fprintf (file, "%s", nvptx_ptx_type_from_mode (op_mode, false));
break;
-case 'R':
- {
- addr_space_t as = UINTVAL (x);
- fputs (nvptx_section_from_addr_space (as), file);
- }
- break;
-
case 'S':
{
unsigned kind = UINTVAL (x);
@@ -3216,9 +3200,24 @@ nvptx_find_sese (auto_vec &
basic_block to = regions[ix].second;
if (from)
- fprintf (dump_file, "%s %d{%d->%d}", comma, ix,
- from->index, to->index);
+ {
+ fprintf (dump_file, "%s %d{%d", comma, ix, from->index);
+ if (to != from)
+ fprintf (dump_file, "->%d", to->index);
+
+ int color = BB_GET_SESE (from)->color;
+
+ /* Print the blocks within the region (excluding ends). */
+ FOR_EACH_BB_FN (block, cfun)
+ {
+ bb_sese *sese = BB_GET_SESE (block);
+ if (sese && sese->color == color
+ && block != from && block != to)
+ fprintf (dump_file, ".%d", block->index);
+ }
+ fprintf (dump_file, "}");
+ }
comma = ",";
}
fprintf (dump_file, "\n\n");
@@ -3712,11 +3711,11 @@ nvptx_neuter_pars (parallel *par, unsign
if (neuter_mask)
{
- int ix;
- int len;
-
+ int ix, len;
+
if (nvptx_optimize)
{
+ /* Neuter whole SESE regions. */
bb_pair_vec_t regions;
nvptx_find_sese (par->blocks, regions);
@@ -3734,6 +3733,7 @@ nvptx_neuter_pars (parallel *par, unsign
}
else
{
+ /* Neuter each BB individually. */
len = par->blocks.length ();
for (ix = 0; ix != len; ix++)
{
@@ -3742,7 +3742,6 @@ nvptx_neuter_pars (parallel *par, unsign
nvptx_single (neuter_mask, block, block);
}
}
-
}
if (skip_mask)
@@ -3913,7 +3912,7 @@ nvptx_record_offload_symbol (tree decl)
tree attr = get_oacc_fn_attrib (decl);
tree dims = TREE_VALUE (attr);
unsigned ix;
-
+
fprintf (asm_out_file, "//:FUNC_MAP \"%s\"",
IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)));
@@ -4085,6 +4084,7 @@ nvptx_expand_cmp_swap (tree exp, rtx tar
return target;
}
+
/* Codes for all the NVPTX builtins. */
enum nvptx_builtins
{
@@ -4319,7 +4319,7 @@ nvptx_generate_vector_shuffle (location_
gimplify_assign (dest_var, expr, seq);
}
-/* Lazily generate the global lock var decl and return its addresss. */
+/* Lazily generate the global lock var decl and return its address. */
static tree
nvptx_global_lock_addr ()
@@ -4353,7 +4353,7 @@ nvptx_global_lock_addr ()
guess = actual;
write = guess OP myval;
actual = cmp&swap (ptr, guess, write)
- } while (actual bit-differnt-to guess);
+ } while (actual bit-different-to guess);
return write;
This relies on a cmp&swap instruction, which is available for 32-
[ptx] remove workaround
I've committed this to trunk. Primarily it removes code working around the lack
of default dimension setting -- I committed the defaulting code a little while
ago and missed this piece. Fixed up some whitespace and ARG_UNUSED that I
noticed along the way.
nathan
2015-11-18 Nathan Sidwell
* config/nvptx/nvptx.c (nvptx_process_pars): Fix whitespace.
(nvptx_record_offload_symbol): Remove code compensating for lack
of default dimension handling.
(nvptx_goacc_validate_dims): Remove incorrect ARG_UNUSED markers.
Index: gcc/config/nvptx/nvptx.c
===
--- gcc/config/nvptx/nvptx.c (revision 230570)
+++ gcc/config/nvptx/nvptx.c (working copy)
@@ -3657,7 +3657,7 @@ nvptx_process_pars (parallel *par)
if (par->mask & GOMP_DIM_MASK (GOMP_DIM_MAX))
/* No propagation needed for a call. */;
- else if (par->mask & GOMP_DIM_MASK (GOMP_DIM_WORKER))
+ else if (par->mask & GOMP_DIM_MASK (GOMP_DIM_WORKER))
{
nvptx_wpropagate (false, par->forked_block, par->forked_insn);
nvptx_wpropagate (true, par->forked_block, par->fork_insn);
@@ -3694,7 +3694,7 @@ nvptx_neuter_pars (parallel *par, unsign
if ((outer | me) & GOMP_DIM_MASK (mode))
{} /* Mode is partitioned: no neutering. */
else if (!(modes & GOMP_DIM_MASK (mode)))
- {} /* Mode is not used: nothing to do. */
+ {} /* Mode is not used: nothing to do. */
else if (par->inner_mask & GOMP_DIM_MASK (mode)
|| !par->forked_insn)
/* Partitioned in inner parallels, or we're not a partitioned
@@ -3910,31 +3910,17 @@ nvptx_record_offload_symbol (tree decl)
case FUNCTION_DECL:
{
tree attr = get_oacc_fn_attrib (decl);
- tree dims = NULL_TREE;
+ tree dims = TREE_VALUE (attr);
unsigned ix;
- if (attr)
- dims = TREE_VALUE (attr);
fprintf (asm_out_file, "//:FUNC_MAP \"%s\"",
IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (decl)));
- for (ix = 0; ix != GOMP_DIM_MAX; ix++)
+ for (ix = 0; ix != GOMP_DIM_MAX; ix++, dims = TREE_CHAIN (dims))
{
- int size = 1;
+ int size = TREE_INT_CST_LOW (TREE_VALUE (dims));
- /* TODO: This check can go away once the dimension default
- machinery is merged to trunk. */
- if (dims)
- {
- tree dim = TREE_VALUE (dims);
-
- if (dim)
- size = TREE_INT_CST_LOW (dim);
-
- gcc_assert (!TREE_PURPOSE (dims));
- dims = TREE_CHAIN (dims);
- }
-
+ gcc_assert (!TREE_PURPOSE (dims));
fprintf (asm_out_file, ", %#x", size);
}
@@ -4186,8 +4172,7 @@ nvptx_expand_builtin (tree exp, rtx targ
routine might spawn a loop. It is negative for non-routines. */
static bool
-nvptx_goacc_validate_dims (tree ARG_UNUSED (decl), int *ARG_UNUSED (dims),
- int ARG_UNUSED (fn_level))
+nvptx_goacc_validate_dims (tree decl, int dims[], int fn_level)
{
bool changed = false;
Debug iterators operator->
Hi I always wanted to fix those operators implementation so that it just use underlying iterator -> operator like for the other operators. And I also notice the @todo on it today so maybe it is still time to fix those. * include/debug/safe_iterator.h (_Safe_iterator<>::operator->()): Implement using underlying iterator same operator. * include/debug/safe_local_iterator.h (_Safe_local_iterator<>::operator->()): Likewise. Tested under Linux x86_64 debug mode of course. François diff --git a/libstdc++-v3/include/debug/safe_iterator.h b/libstdc++-v3/include/debug/safe_iterator.h index a8bee21..ed08d19 100644 --- a/libstdc++-v3/include/debug/safe_iterator.h +++ b/libstdc++-v3/include/debug/safe_iterator.h @@ -274,7 +274,6 @@ namespace __gnu_debug /** * @brief Iterator dereference. * @pre iterator is dereferenceable - * @todo Make this correct w.r.t. iterators that return proxies */ pointer operator->() const _GLIBCXX_NOEXCEPT @@ -282,7 +281,7 @@ namespace __gnu_debug _GLIBCXX_DEBUG_VERIFY(this->_M_dereferenceable(), _M_message(__msg_bad_deref) ._M_iterator(*this, "this")); - return std::__addressof(*base()); + return base().operator->(); } // -- Input iterator requirements -- diff --git a/libstdc++-v3/include/debug/safe_local_iterator.h b/libstdc++-v3/include/debug/safe_local_iterator.h index 350a1d2..f60b6e9 100644 --- a/libstdc++-v3/include/debug/safe_local_iterator.h +++ b/libstdc++-v3/include/debug/safe_local_iterator.h @@ -236,7 +236,6 @@ namespace __gnu_debug /** * @brief Iterator dereference. * @pre iterator is dereferenceable - * @todo Make this correct w.r.t. iterators that return proxies */ pointer operator->() const @@ -244,7 +243,7 @@ namespace __gnu_debug _GLIBCXX_DEBUG_VERIFY(this->_M_dereferenceable(), _M_message(__msg_bad_deref) ._M_iterator(*this, "this")); - return std::__addressof(*base()); + return base().operator->(); } // -- Input iterator requirements --
Re: [PATCH] Fix ICE with mangling aliases (PR c++/67354)
On 11/18/2015 03:52 AM, Jakub Jelinek wrote: If changing at_eof for this is too big hack, perhaps we could have a different bool just to affect the mangling aliases (and set it to true in generate_mangling_aliases or so). Let's do that. Jason
[Ada] Enable -fstrict-overflow at all optimization levels
The Ada compiler generates overflow checks by default (as required by the
language) so it makes sense to enable -fstrict-overflow at all optimization
levels; this mainly affects -O1 in practice.
Tested on x86_64-suse-linux, applied on the mainline.
2015-11-18 Eric Botcazou
* gcc-interface/misc.c: Move global variables to the top of the file.
(gnat_handle_option): Remove obsolete ATTRIBUTE_UNUSED markers.
(gnat_init_options): Minor tweak.
(gnat_post_options): Set -fstrict-overflow if not done by the user.
(internal_error_function): Minor reformatting.
2015-11-18 Eric Botcazou
* gnat.dg/opt52.adb: New test.
--
Eric BotcazouIndex: gcc-interface/misc.c
===
--- gcc-interface/misc.c (revision 230557)
+++ gcc-interface/misc.c (working copy)
@@ -62,9 +62,28 @@ void *callgraph_info_file = NULL;
unsigned int save_argc;
const char **save_argv;
-/* GNAT argc and argv. */
+/* GNAT argc and argv generated by the binder for all Ada programs. */
extern int gnat_argc;
-extern char **gnat_argv;
+extern const char **gnat_argv;
+
+/* Ada code requires variables for these settings rather than elements
+ of the global_options structure because they are imported. */
+int gnat_encodings = 0;
+
+#undef optimize
+int optimize;
+
+#undef optimize_size
+int optimize_size;
+
+#undef flag_compare_debug
+int flag_compare_debug;
+
+#undef flag_short_enums
+int flag_short_enums;
+
+#undef flag_stack_check
+enum stack_check_type flag_stack_check = NO_STACK_CHECK;
#ifdef __cplusplus
extern "C" {
@@ -118,9 +137,8 @@ gnat_option_lang_mask (void)
are marked as Ada-specific. Return true on success or false on failure. */
static bool
-gnat_handle_option (size_t scode, const char *arg ATTRIBUTE_UNUSED, int value,
- int kind ATTRIBUTE_UNUSED, location_t loc ATTRIBUTE_UNUSED,
- const struct cl_option_handlers *handlers ATTRIBUTE_UNUSED)
+gnat_handle_option (size_t scode, const char *arg, int value, int kind,
+ location_t loc, const struct cl_option_handlers *handlers)
{
enum opt_code code = (enum opt_code) scode;
@@ -164,8 +182,8 @@ gnat_handle_option (size_t scode, const
Ada_handle_option_auto (&global_options, &global_options_set,
scode, arg, value,
- gnat_option_lang_mask (), kind,
- loc, handlers, global_dc);
+ gnat_option_lang_mask (), kind, loc,
+ handlers, global_dc);
return true;
}
@@ -194,11 +212,9 @@ gnat_init_options (unsigned int decoded_
??? back_end.adb should not rely on this; instead, it should work with
decoded options without such reparsing, to ensure consistency in how
options are decoded. */
- unsigned int i;
-
save_argv = XNEWVEC (const char *, 2 * decoded_options_count + 1);
save_argc = 0;
- for (i = 0; i < decoded_options_count; i++)
+ for (unsigned int i = 0; i < decoded_options_count; i++)
{
size_t num_elements = decoded_options[i].canonical_option_num_elements;
@@ -223,25 +239,12 @@ gnat_init_options (unsigned int decoded_
}
save_argv[save_argc] = NULL;
- gnat_argv = (char **) xmalloc (sizeof (save_argv[0]));
- gnat_argv[0] = xstrdup (save_argv[0]); /* name of the command */
+ /* Pass just the name of the command through the regular channel. */
+ gnat_argv = (const char **) xmalloc (sizeof (char *));
+ gnat_argv[0] = xstrdup (save_argv[0]);
gnat_argc = 1;
}
-/* Ada code requires variables for these settings rather than elements
- of the global_options structure. */
-#undef optimize
-#undef optimize_size
-#undef flag_compare_debug
-#undef flag_short_enums
-#undef flag_stack_check
-int gnat_encodings = 0;
-int optimize;
-int optimize_size;
-int flag_compare_debug;
-int flag_short_enums;
-enum stack_check_type flag_stack_check = NO_STACK_CHECK;
-
/* Settings adjustments after switches processing by the back-end.
Note that the front-end switches processing (Scan_Compiler_Arguments)
has not been done yet at this point! */
@@ -262,6 +265,10 @@ gnat_post_options (const char **pfilenam
if (!global_options_set.x_flag_diagnostics_show_caret)
global_dc->show_caret = false;
+ /* Set strict overflow by default for Ada. */
+ if (!global_options_set.x_flag_strict_overflow)
+global_options.x_flag_strict_overflow = true;
+
/* Warn only if STABS is not the default: we don't want to emit a warning if
the user did not use a -gstabs option. */
if (PREFERRED_DEBUGGING_TYPE != DBX_DEBUG && write_symbols == DBX_DEBUG)
@@ -287,8 +294,8 @@ gnat_post_options (const char **pfilenam
/* Here is the function to handle the compiler error processing in GCC. */
static void
-internal_error_function (diagnostic_context *context,
- const char *msgid, va_list *ap)
+internal_error_function (diagnostic_context *context, const char *msgid,
+ va_list *ap)
{
text_info tinfo;
char *buffer, *p, *loc;
-- { dg-do compile }
-- { dg-options "
[PATCH] Fix declaration of pthread-structs in s-osinte-rtems.ads (ada/68169)
Hello,
The paperwork seems to have gone through.
Here is the patch again for the 4.9.x, 5.x and trunk respectively.
I just pulled the head of the corresponding branches and created a new diff, so
it should apply properly.
Best regards,
JanIndex: gcc/ada/ChangeLog
===
--- gcc/ada/ChangeLog (Revision 230563)
+++ gcc/ada/ChangeLog (Arbeitskopie)
@@ -1,3 +1,9 @@
+2015-11-18 Jan Sommer
+
+ * s-oscons-tmplt.c: Generate pthread constants for RTEMS
+ * s-osinte-rtems.ads: Declare pthread structs as opaque types in Ada
+ Fixes PR ada/68169
+
2015-10-09 Eric Botcazou
* gcc-interface/Make-lang.in: Make sure that GNAT1_OBJS and not just
Index: gcc/ada/s-oscons-tmplt.c
===
--- gcc/ada/s-oscons-tmplt.c (Revision 230563)
+++ gcc/ada/s-oscons-tmplt.c (Arbeitskopie)
@@ -154,7 +154,7 @@ pragma Style_Checks ("M32766");
# include <_types.h>
#endif
-#ifdef __linux__
+#if defined (__linux__) || defined (__rtems__)
# include
# include
#endif
@@ -1441,7 +1441,8 @@ CND(CLOCK_THREAD_CPUTIME_ID, "Thread CPU clock")
CNS(CLOCK_RT_Ada, "")
#endif
-#if defined (__APPLE__) || defined (__linux__) || defined (DUMMY)
+#if defined (__APPLE__) || defined (__linux__) || defined (__rtems__) || \
+ defined (DUMMY)
/*
-- Sizes of pthread data types
@@ -1484,7 +1485,7 @@ CND(PTHREAD_RWLOCKATTR_SIZE, "pthread_rwlockattr_t
CND(PTHREAD_RWLOCK_SIZE, "pthread_rwlock_t")
CND(PTHREAD_ONCE_SIZE, "pthread_once_t")
-#endif /* __APPLE__ || __linux__ */
+#endif /* __APPLE__ || __linux__ || __rtems__*/
/*
Index: gcc/ada/s-osinte-rtems.ads
===
--- gcc/ada/s-osinte-rtems.ads (Revision 230563)
+++ gcc/ada/s-osinte-rtems.ads (Arbeitskopie)
@@ -51,6 +51,8 @@
-- It is designed to be a bottom-level (leaf) package.
with Interfaces.C;
+with System.OS_Constants;
+
package System.OS_Interface is
pragma Preelaborate;
@@ -60,6 +62,7 @@ package System.OS_Interface is
subtype rtems_id is Interfaces.C.unsigned;
subtype intis Interfaces.C.int;
+ subtype char is Interfaces.C.char;
subtype short is Interfaces.C.short;
subtype long is Interfaces.C.long;
subtype unsigned is Interfaces.C.unsigned;
@@ -68,7 +71,6 @@ package System.OS_Interface is
subtype unsigned_char is Interfaces.C.unsigned_char;
subtype plain_char is Interfaces.C.plain_char;
subtype size_t is Interfaces.C.size_t;
-
---
-- Errno --
---
@@ -76,11 +78,11 @@ package System.OS_Interface is
function errno return int;
pragma Import (C, errno, "__get_errno");
- EAGAIN: constant := 11;
- EINTR : constant := 4;
- EINVAL: constant := 22;
- ENOMEM: constant := 12;
- ETIMEDOUT : constant := 116;
+ EAGAIN: constant := System.OS_Constants.EAGAIN;
+ EINTR : constant := System.OS_Constants.EINTR;
+ EINVAL: constant := System.OS_Constants.EINVAL;
+ ENOMEM: constant := System.OS_Constants.ENOMEM;
+ ETIMEDOUT : constant := System.OS_Constants.ETIMEDOUT;
-
-- Signals --
@@ -448,6 +450,7 @@ package System.OS_Interface is
ss_low_priority : int;
ss_replenish_period : timespec;
ss_initial_budget : timespec;
+ sched_ss_max_repl : int;
end record;
pragma Convention (C, struct_sched_param);
@@ -621,43 +624,34 @@ private
end record;
pragma Convention (C, timespec);
- CLOCK_REALTIME : constant clockid_t := 1;
- CLOCK_MONOTONIC : constant clockid_t := 4;
+ CLOCK_REALTIME : constant clockid_t := System.OS_Constants.CLOCK_REALTIME;
+ CLOCK_MONOTONIC : constant clockid_t := System.OS_Constants.CLOCK_MONOTONIC;
+ subtype char_array is Interfaces.C.char_array;
+
type pthread_attr_t is record
- is_initialized : int;
- stackaddr : System.Address;
- stacksize : int;
- contentionscope : int;
- inheritsched: int;
- schedpolicy : int;
- schedparam : struct_sched_param;
- cputime_clocked_allowed : int;
- detatchstate: int;
+ Data : char_array (1 .. OS_Constants.PTHREAD_ATTR_SIZE);
end record;
pragma Convention (C, pthread_attr_t);
+ for pthread_attr_t'Alignment use Interfaces.C.double'Alignment;
type pthread_condattr_t is record
- flags : int;
- process_shared : int;
+ Data : char_array (1 .. OS_Constants.PTHREAD_CONDATTR_SIZE);
end record;
pragma Convention (C, pthread_condattr_t);
+ for pthread_condattr_t'Alignment use Interfaces.C.double'Alignment;
type pthread_mutexattr_t is record
- is_initialized : int;
- process_shared : int;
- prio_ceiling: int;
- protocol: int;
- mutex_type : int;
- recursive : int;
- end record;
+
Re: [patch] FreeBSD x86_64/i386 apply r125920
On 16.11.15 23:20, Andreas Tobler wrote: Hi all, I'm going to apply the below patch to trunk if there are no objections. There are no regressions. From our pov, we should be in sync with Linux here. Also, I'm going to push this one out to 5.x after one week. Committed on trunk as r230565 2015-11-16 Andreas Tobler * config/i386/freebsd.h (ASM_OUTPUT_MAX_SKIP_ALIGN): Bring in the commit from r125920 for FreeBSD.
Re: [PATCHES, PING*5] Enhance standard DWARF for Ada
On 10/20/2015 04:13 PM, Pierre-Marie de Rodat wrote: Sorry about the slow review on these patches. In future please feel free to ping me as often as once a week. + /* DWARF operations all work on signed integers. Note that this will no longer be the case in DWARF 5, where stack elements have associated types. Let's add a comment about that so perhaps we can remove the workaround in a future release. + /* ??? Set of all DW_OP_nop operations we remove: is it really a good thing + to free them, or should we instead let the garbage collect do it? */ Might as well free them if we know they're garbage, it lets us reuse that memory sooner. + /* Trailing nops from loc_descritor_from_tree (if any) cannot be removed missing 'p' in loc_descriptor_from_tree. + /* When translating a function into a DWARF procedure, contains the frame + offset *before* evaluating this operation. It is -1 when not yet + initialized. */ + int dw_loc_frame_offset; + /* For DW_OP_call* operations: contains the number of stack slots that were + added overall when returning from the procedure (so it's negative if the + procedure removes slots). */ + int dw_loc_frame_offset_increment; I'm not excited about adding another couple of words to every loc insn for uses that occur so rarely. dw_loc_frame_offset seems to be primarily used for checking, can we make it conditional on ENABLE_CHECKING and use a hash_set to remember already visited nodes? Instead of dw_loc_frame_offset_increment, can we look up the number of arguments from the callee? + /* Return a type to used in the debug info instead of TYPE, or NULL_TREE to "to use" + /* Arbitrary scale factors cannot be describe in standard DWARF, "described" Jason
Re: Re: OpenACC declare directive updates
On 11/08/2015 08:53 PM, James Norris wrote: > The attached patch and ChangeLog reflect the updates from your > review: https://gcc.gnu.org/ml/gcc-patches/2015-11/msg00714.html > and Cesar's review: > https://gcc.gnu.org/ml/gcc-patches/2015-11/msg00885.html. > > With the changes made in this patch I think I'm handling the > situation that you pointed out here correctly: > > "Also, wonder about BLOCK stmt in Fortran, that can give you variables that > don't live through the whole function, but only a portion of it even in > Fortran." What block stmt? The most recent version of Fortran OpenACC 2.0a supports is 2003. The block construct is a 2008 feature. I don't think that's applicable to this version. Jim, maybe you should add an error message for variables defined in blocks. Thinking about this some more, I wonder if we should emit an error if any acc constructs are used inside blocks? That's probably overly pessimistic though. Cesar
[nios2, committed] remove duplicates in || expression
I've checked in this patch to fix PR 68410. David, thanks for pointing out the think-o here. -Sandra 2015-11-18 Sandra Loosemore PR target/68410 * config/nios2/nios2.c (cdx_and_immed): Remove duplicate tests from || expression. Index: gcc/config/nios2/nios2.c === --- gcc/config/nios2/nios2.c (revision 230549) +++ gcc/config/nios2/nios2.c (working copy) @@ -4120,8 +4120,8 @@ cdx_and_immed (rtx op) HOST_WIDE_INT ival = INTVAL (op); return (ival == 1 || ival == 2 || ival == 3 || ival == 4 || ival == 8 || ival == 0xf || ival == 0x10 - || ival == 0x10 || ival == 0x1f || ival == 0x20 - || ival == 0x3f || ival == 0x3f || ival == 0x7f + || ival == 0x1f || ival == 0x20 + || ival == 0x3f || ival == 0x7f || ival == 0x80 || ival == 0xff || ival == 0x7ff || ival == 0xff00 || ival == 0x); }
Re: [PATCH] Avoid useless work in loop vectorization
On 13/11/15 08:41, Richard Biener wrote: Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk. Richard. 2015-11-13 Richard Biener * tree-vect-loop.c (vect_analyze_loop_2): Add fatal parameter. Signal fatal failure if early checks fail. (vect_analyze_loop): If vect_analyze_loop_2 fails fatally do not bother testing further vector sizes. It seems that on AArch64 this causes: FAIL: gcc.dg/vect/vect-outer-1-big-array.c -flto -ffat-lto-objects scan-tree-dump-times vect "grouped access in outer loop" 2 FAIL: gcc.dg/vect/vect-outer-1-big-array.c scan-tree-dump-times vect "grouped access in outer loop" 2 FAIL: gcc.dg/vect/vect-outer-1.c -flto -ffat-lto-objects scan-tree-dump-times vect "grouped access in outer loop" 2 FAIL: gcc.dg/vect/vect-outer-1.c scan-tree-dump-times vect "grouped access in outer loop" 2 FAIL: gcc.dg/vect/vect-outer-1a-big-array.c -flto -ffat-lto-objects scan-tree-dump-times vect "grouped access in outer loop" 2 FAIL: gcc.dg/vect/vect-outer-1a-big-array.c scan-tree-dump-times vect "grouped access in outer loop" 2 FAIL: gcc.dg/vect/vect-outer-1a.c -flto -ffat-lto-objects scan-tree-dump-times vect "grouped access in outer loop" 2 FAIL: gcc.dg/vect/vect-outer-1a.c scan-tree-dump-times vect "grouped access in outer loop" 2 FAIL: gcc.dg/vect/vect-outer-1b-big-array.c -flto -ffat-lto-objects scan-tree-dump-times vect "grouped access in outer loop" 2 FAIL: gcc.dg/vect/vect-outer-1b-big-array.c scan-tree-dump-times vect "grouped access in outer loop" 2 FAIL: gcc.dg/vect/vect-outer-1b.c -flto -ffat-lto-objects scan-tree-dump-times vect "grouped access in outer loop" 2 FAIL: gcc.dg/vect/vect-outer-1b.c scan-tree-dump-times vect "grouped access in outer loop" 2 FAIL: gcc.dg/vect/vect-outer-2b.c -flto -ffat-lto-objects scan-tree-dump-times vect "grouped access in outer loop" 2 FAIL: gcc.dg/vect/vect-outer-2b.c scan-tree-dump-times vect "grouped access in outer loop" 2 FAIL: gcc.dg/vect/vect-outer-3b.c -flto -ffat-lto-objects scan-tree-dump-times vect "grouped access in outer loop" 4 FAIL: gcc.dg/vect/vect-outer-3b.c scan-tree-dump-times vect "grouped access in outer loop" 4 Still there on r230556, I haven't dug any further yet. Thanks, Alan
RE: [gomp4.1] Handle linear clause modifiers in declare simd
Yes, I will send the updated version to Jakub when I come back from SC15. I am updating it based on discussion with Jakub. Thanks, Xinmin -Original Message- From: Jakub Jelinek [mailto:ja...@redhat.com] Sent: Wednesday, November 18, 2015 9:41 AM To: Ilya Verbin; Tian, Xinmin Cc: gcc-patches@gcc.gnu.org; Kirill Yukhin; andrew.n.senkev...@gmail.com Subject: Re: [gomp4.1] Handle linear clause modifiers in declare simd On Wed, Nov 18, 2015 at 08:30:53PM +0300, Ilya Verbin wrote: > Could you please point to where the new ABI additions are documented? > I can't find R/L/U parameter types in [1] and [2]. > > [1] > https://sourceware.org/glibc/wiki/libmvec?action=AttachFile&do=view&ta > rget=VectorABI.txt [2] > https://groups.google.com/forum/#!topic/x86-64-abi/LmppCfN1rZ4 Xinmin should have those in the Intel ABI pdf, not sure if the latest pdf is publicly available or not. Jakub
Re: [PATCH] PR fortran/59910 -- structure constructor in DATA statement
On Tue, Nov 17, 2015 at 05:01:42PM -0800, Steve Kargl wrote: > On Tue, Nov 17, 2015 at 04:36:01PM -0800, Steve Kargl wrote: > > On Wed, Nov 18, 2015 at 12:24:29AM +0100, Dominique d'Humières wrote: > > > > ??? but I suspect gfc_reduce_init_expr() > > > > may be useful for PARAMETER statements as well (need to > > > > check this!). > > > > > > As in the following test > > > > > > module m > > > implicit none > > > type t > > > integer :: i > > > end type t > > > type(t), dimension(2), parameter :: a1 = (/ t(1), t(2) /) > > > type(t), dimension(1), parameter :: c = spread ( a1(1), 1, 1 ) > > > end module m > > > > Yep. We again arrive at gfc_conv_array_initializer with > > expr->expr_type == EXPR_FUNCTION, which isn't handled correctly. > > > > The issue seems deeply rooted in the handling of derived types, > > which is actually worse than this! But, that is definitely for > > another day. See PR67817. :( > > Ugh. gfc_simplify_spread does not actually the use of SPREAD > here, because source->expr_type == EXPR_STRUCTURE which is not > handled. Dominiq, I plan to commit my patch and close this PR as the patch fixes the issue raised in the PR. I think the above code highlights a specific issue with SPREAD, and a new PR should be committed. -- Steve
Re: [patch, avr] Add new Atmel AVR devices
2015-11-18 9:36 GMT+03:00 Sivanupandi, Pitchumani : > Attached patch adds new Atmel devices to avr-gcc. > If Ok, could someone commit please? I do not have commit access. > > Regards, > Pitchumani > > gcc/ChangeLog > > 2015-11-18 Pitchumani Sivanupandi > > * config/avr/avr-mcus.def: Add new avr4 devices atmega48pb and > atmega88pb. Add new avr5 devices ata5791, ata8210, ata8510, > atmega168pb > and atmega328pb. > * doc/avr-mmcu.texi: Regenerate. > Committed. Denis.
Re: Remove noce_mem_write_may_trap_or_fault_p in ifcvt
Ok, so this is a thorny problem. I thought I had a solution, and I'll
start by describing it, but in the end I think it doesn't actually work.
On 11/06/2015 10:09 PM, Jeff Law wrote:
On 11/06/2015 12:30 PM, Bernd Schmidt wrote:
Well, I think if MEM_READONLY_P is insufficient (and I guess people
could cast away const - bother), then the current
noce_mem_write_may_trap_or_fault_p should be simplified to "return
true;" and eliminated. We could try to special case stack accesses
within a known limit later on.
Maybe it doesn't even matter given that we call noce_can_store_speculate
immediately after this test.
may_trap_or_fault_p already has this kind of knowledge. It just doesn't
know if the memory is a read or a write. Hence my suggestion we should
fix may_trap_or_fault_p.
For the current issue I've come to the conclusion that this kind of
analysis is irrelevant here (and that is not subject to the problem I'll
describe later), because of the use of noce_can_store_speculate. See below.
Ripping out noce_mem_write_may_trap_or_fault_p without fixing
may_trap_or_fault_p introduces a latent code code generation issue.
I don't think so, actually. One safe option would be to rip it out and
just stop transforming this case, but let's start by looking at the code
just a bit further down, calling noce_can_store_speculate. This was
added later than the code we're discussing, and it tries to verify that
the same memory location will unconditionally be written to at a point
later than the one we're trying to convert (but why aren't we testing
for prior writes?). That should make any may_trap test unnecessary, and
we can just simply delete the call to
noce_mem_write_may_trap_or_fault_p. (We could leave such a test in as a
compile-time focused early-out, but it may actually be too conservative.
We may have an address which we can't prove safe in isolation, but if we
know that we also have an unconditional store, it must be safe.)
So... I was about to propose the attached patch, which also fixes some
oversights in the can_store_speculate path: we shouldn't allow autoinc
addresses here. The added test in noce_can_store_speculate_p is not
quite necessary given that the same one is also added to
memory_must_be_modified_in_insn_p, but it avoids the need for an insn
walk in cases where it isn't necessary. This bootstrapped and tested ok
on x86_64-linux.
But then, I looked at noce_can_store_speculate again, and it seems
broken. We walk over the post-dominators of the block, see if we find a
store, and fail if something modifies the address:
for (dominator = get_immediate_dominator (CDI_POST_DOMINATORS, top_bb);
dominator != NULL;
dominator = get_immediate_dominator (CDI_POST_DOMINATORS,
dominator))
{
rtx_insn *insn;
FOR_BB_INSNS (dominator, insn)
{
[...]
if (modified_in_p (XEXP (mem, 0), insn))
return false;
}
But what if the address is modified, but not in a post-dominator block?
Let's say
if (cond)
*p = x; // this is the store we want to convert
if (other_cond)
p = some_pointer;
else
p = some_other_pointer;
*p = y; // we'll see this store which postdominates the original
// one, but not the modifications of p
So I think that algorithm doesn't work. My suggestion at this point
would be to give up on converting stores entirely (deleting
noce_can_store_speculate_p and noce_mem_write_may_trap_or_fault_p) until
someone produces a reasonable scratchpad patch.
Bernd
* alias.c (memory_must_be_modified_in_insn_p): Return false for
addresses with side effects.
* ifcvt.c (noce_mem_write_may_trap_or_fault_p): Delete function.
(noce_can_store_speculate_p): Return false for addresses with side
effects.
(noce_process_if_block): Rely only on noce_can_store_speculate_p.
Index: gcc/alias.c
===
--- gcc/alias.c (revision 230541)
+++ gcc/alias.c (working copy)
@@ -2974,6 +2974,10 @@ memory_must_be_modified_in_insn_p (const
{
if (!INSN_P (insn))
return false;
+ /* Our rtx_equal_p tests will not do the right thing in this case. */
+ if (side_effects_p (XEXP (mem, 0)))
+return false;
+
insn = PATTERN (insn);
if (GET_CODE (insn) == SET)
return set_dest_equal_p (insn, mem);
@@ -2984,7 +2988,7 @@ memory_must_be_modified_in_insn_p (const
{
rtx sub = XVECEXP (insn, 0, i);
if (GET_CODE (sub) == SET
- && set_dest_equal_p (sub, mem))
+ && set_dest_equal_p (sub, mem))
return true;
}
}
Index: gcc/ifcvt.c
===
--- gcc/ifcvt.c (revision 230541)
+++ gcc/ifcvt.c (working copy)
@@ -2919,59 +2919,6 @@ noce_operand_ok (const_rtx op)
return ! may_trap_p (op);
}
-/* Return true if a write into MEM may trap or fault. */
-
-static bool
-noce_mem_write_may_trap_or_fault_p (const_rtx mem)
-{
- rtx addr;
-
- if (MEM_READONLY_P
Re: [PATCH] PR fortran/59910 -- structure constructor in DATA statement
On 11/17/2015 12:34 PM, Steve Kargl wrote: > Here's what looks like a fairly simple patch, but it leads > to a question. Why does gfortran not try to reduce the > components in a structure constructor in general? I've > hidden the gfc_reduce_init_expr() behind a check for a > DATA statement, but I suspect gfc_reduce_init_expr() > may be useful for PARAMETER statements as well (need to > check this!). > > Anyway, the patch has been built and tested on x86_64-*-freebsd. > A slightly different patch was built and tested on i386-*-freebsd. > > OK to commit? > OK, Can't answer your question above at the moment. Jerry
[PATCH] [4.9] Re: [PATCH][5] Backport ISL 0.15 support
On 12.10.2015 12:58, Richard Biener wrote:
This backports the patch to allow bootstrapping with ISL 0.15 to the
GCC 5 branch (the GCC 4.9 branch will require backporting of some
dependencies).
I don't think so. 4.8 and 4.9 don't use as much ISL code as 5 does. I had a
look at the backport and came up with something which is just mechanical changes
for the cpp conditional.
The version check in the toplevel configure needs to be extended. I'm currently
not checking non matching isl and cloog versions.
Now build for 4.9 using ISL 0.15 and 0.14. Ok to commit to the branch?
Matthias
2015-11-18 Matthias Klose
* configure.ac: Permit also ISL 0.15 with CLooG.
* configure: Regenerate.
gcc/
2015-11-18 Matthias Klose
Backport from the gcc-5-branch
Backport from mainline
2015-07-21 Mike Frysinger
Bernhard Reutner-Fischer
* configure.ac: Add check for new options in isl-0.15.
* config.in, configure: Rebuilt.
* graphite-blocking.c: Include
* graphite-interchange.c, graphite-poly.c: Likewise.
* graphhite-scop-detection.c, graphite-sese-to-poly.c: Likewise.
* graphite.c, graphite-poly.c: Likewise.
* graphite-dependences.c: Include .
(max_number_of_out_dimensions): Returns isl_stat.
(extend_schedule_1): Likewise
(extend_schedule): Corresponding changes.
* graphite-optimize-isl.c: Include and
.
(getSingleMap): Change return type of isl_stat.
(optimize_isl): Conditionally use
isl_options_set_schedule_serialize_sccs.
* graphite-poly.h (isl_stat, isl_stat_ok): Define fallbacks
if not HAVE_ISL_OPTIONS_SET_SCHEDULE_SERIALIZE_SCCS.
Index: configure.ac
===
--- configure.ac (revision 230544)
+++ configure.ac (working copy)
@@ -1660,6 +1660,9 @@
ISL_CHECK_VERSION(0,12)
if test "${gcc_cv_isl}" = no ; then
ISL_CHECK_VERSION(0,14)
+if test "${gcc_cv_isl}" = no ; then
+ ISL_CHECK_VERSION(0,15)
+fi
fi
fi
fi
Index: gcc/config.in
===
--- gcc/config.in (revision 230544)
+++ gcc/config.in (working copy)
@@ -1223,6 +1223,12 @@
#endif
+/* Define if isl_options_set_schedule_serialize_sccs exists. */
+#ifndef USED_FOR_TARGET
+#undef HAVE_ISL_OPTIONS_SET_SCHEDULE_SERIALIZE_SCCS
+#endif
+
+
/* Define if isl_schedule_constraints_compute_schedule exists. */
#ifndef USED_FOR_TARGET
#undef HAVE_ISL_SCHED_CONSTRAINTS_COMPUTE_SCHEDULE
Index: gcc/configure.ac
===
--- gcc/configure.ac (revision 230544)
+++ gcc/configure.ac (working copy)
@@ -5535,6 +5535,8 @@
# Check whether isl_schedule_constraints_compute_schedule is available;
# it's new in ISL-0.13.
+ # Check whether isl_options_set_schedule_serialize_sccs is available;
+ # it's new in ISL-0.15.
saved_CFLAGS="$CFLAGS"
CFLAGS="$CFLAGS $ISLINC"
saved_LIBS="$LIBS"
@@ -5547,6 +5549,13 @@
[ac_has_isl_schedule_constraints_compute_schedule=no])
AC_MSG_RESULT($ac_has_isl_schedule_constraints_compute_schedule)
+ AC_MSG_CHECKING([Checking for isl_options_set_schedule_serialize_sccs])
+ AC_TRY_LINK([#include ],
+ [isl_options_set_schedule_serialize_sccs (NULL, 0);],
+ [ac_has_isl_options_set_schedule_serialize_sccs=yes],
+ [ac_has_isl_options_set_schedule_serialize_sccs=no])
+ AC_MSG_RESULT($ac_has_isl_options_set_schedule_serialize_sccs)
+
LIBS="$saved_LIBS"
CFLAGS="$saved_CFLAGS"
@@ -5554,6 +5563,11 @@
AC_DEFINE(HAVE_ISL_SCHED_CONSTRAINTS_COMPUTE_SCHEDULE, 1,
[Define if isl_schedule_constraints_compute_schedule exists.])
fi
+
+ if test x"$ac_has_isl_options_set_schedule_serialize_sccs" = x"yes"; then
+ AC_DEFINE(HAVE_ISL_OPTIONS_SET_SCHEDULE_SERIALIZE_SCCS, 1,
+ [Define if isl_options_set_schedule_serialize_sccs exists.])
+ fi
fi
Index: gcc/graphite-blocking.c
===
--- gcc/graphite-blocking.c (revision 230544)
+++ gcc/graphite-blocking.c (working copy)
@@ -24,6 +24,7 @@
#include "config.h"
#ifdef HAVE_cloog
+#include
#include
#include
#include
Index: gcc/graphite-dependences.c
===
--- gcc/graphite-dependences.c (revision 230544)
+++ gcc/graphite-dependences.c (working copy)
@@ -22,6 +22,7 @@
#include "config.h"
#ifdef HAVE_cloog
+#include
#include
#include
#include
@@ -183,7 +184,7 @@
/* Helper function used on each MAP of a isl_union_map. Computes the
maximal output dimension. */
-static int
+static isl_stat
max_number_of_out_dimensions (__isl_take isl_map *map, void *user)
{
int global_max = *((int *) user);
@@ -195,7 +196,7 @@
isl_map_free (map);
isl_space_free (space);
- return 0;
+ return isl_stat_ok;
}
/* Extends the output dimension of MAP to MAX dimensions. */
@@ -219,
Re: [1/2] OpenACC routine support
On 11/10/2015 12:16 AM, Jakub Jelinek wrote:
> On Mon, Nov 09, 2015 at 09:28:47PM -0800, Cesar Philippidis wrote:
>> Here's the patch that Nathan was referring to. I ended up introducing a
>> boolean variable named first in the various functions which call
>> finalize_oacc_routines. The problem the original approach was having was
>> that the routine clauses is only applied to the first function
>> declarator in a declaration list. By using 'first', which is set to true
>> if the current declarator is the first in a sequence of declarators, I
>> was able to defer setting parser->oacc_routine to NULL.
>
> The #pragma omp declare simd has identical restrictions, but doesn't need
> to add any of the first parameters to the C++ parser.
> So, what are you doing differently that you need it? Handling both
> differently is a consistency issue, and unnecessary additional complexity to
> the parser.
I reworked how acc routines are handed in this patch to be more similar
to #pragma omp declare simd. Things get kind of messy though. For
starters, I had to add a new tree clauses member to
cp_omp_declare_simd_data. This serves two purposes:
* It allows the c++ FE to record the location of the first
#pragma acc routine, which is nice because it allows test cases to
be shared with the c FE.
* Unlike omp declare simd, only one acc routine may be associated with
a function decl. This meant that I had to defer attaching the acc
geometry and 'omp target' attributes to cp_finalize_oacc_routine
instead of in cp_parser_late_parsing_oacc_routine like in omp. So
what happens is, cp_parser_late_parsing_oacc_routine ends up
creating a function geometry clause.
I don't really like this approach. I did try to postpone parsing the
clauses till cp_finalize_oacc_routine, but that got messy. Plus, while
I'd be able to remove the clauses field from cp_omp_declare_simd_data,
we'd still need a location_t field for cp_ensure_no_oacc_routine.
Is this OK for trunk?
Cesar
2015-11-17 Cesar Philippidis
gcc/cp/
* parser.h (struct cp_omp_declare_simd_data): Add clauses member.
(struct cp_parser): Change type the of oacc_routine to
cp_omp_declare_simd_data.
* parser.c (cp_ensure_no_oacc_routine): Rework to use
cp_omp_declare_simd_data.
(cp_parser_simple_declaration): Remove boolean first. Update call to
cp_parser_init_declarator. Don't NULL out oacc_routine.
(cp_parser_init_declarator): Remove boolean first parameter. Update
calls to cp_finalize_oacc_routine.
(cp_parser_late_return_type_opt): Handle acc routines.
(cp_parser_member_declaration): Remove first variable. Handle
acc routines like omp declare simd.
(cp_parser_function_definition_from_specifiers_and_declarator): Update
call to cp_finalize_oacc_routine.
(cp_parser_single_declaration): Update call to
cp_parser_init_declarator.
(cp_parser_save_member_function_body): Remove first_decl parameter.
Update call to cp_finalize_oacc_routine.
(cp_parser_finish_oacc_routine): Delete.
(cp_parser_oacc_routine): Rework to use cp_omp_declare_simd_data.
(cp_parser_late_parsing_oacc_routine): New function.
(cp_finalize_oacc_routine): Remove first argument. Add more error
handling and set the acc routine and 'omp declare target' attributes.
(cp_parser_pragma): Remove unnecessary call to
cp_ensure_no_oacc_routine.
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 0e1116b..8de3bce 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -241,7 +241,7 @@ static bool cp_parser_omp_declare_reduction_exprs
static tree cp_parser_cilk_simd_vectorlength
(cp_parser *, tree, bool);
static void cp_finalize_oacc_routine
- (cp_parser *, tree, bool, bool);
+ (cp_parser *, tree, bool);
/* Manifest constants. */
#define CP_LEXER_BUFFER_SIZE ((256 * 1024) / sizeof (cp_token))
@@ -1318,13 +1318,21 @@ cp_finalize_omp_declare_simd (cp_parser *parser, tree fndecl)
}
}
-/* Diagnose if #pragma omp routine isn't followed immediately
- by function declaration or definition. */
+/* Diagnose if #pragma acc routine isn't followed immediately by function
+ declaration or definition. */
static inline void
cp_ensure_no_oacc_routine (cp_parser *parser)
{
- cp_finalize_oacc_routine (parser, NULL_TREE, false, true);
+ if (parser->oacc_routine && !parser->oacc_routine->error_seen)
+{
+ tree clauses = parser->oacc_routine->clauses;
+ location_t loc = OMP_CLAUSE_LOCATION (TREE_PURPOSE(clauses));
+
+ error_at (loc, "%<#pragma oacc routine%> not followed by function "
+ "declaration or definition");
+ parser->oacc_routine = NULL;
+}
}
/* Decl-specifiers. */
@@ -2130,7 +2138,7 @@ static tree cp_parser_decltype
static tree cp_parser_init_declarator
(cp_parser *, cp_decl_specifier_seq *, vec *,
- bool, bool, int, bool *, tree *, bool, location_t *);
+ bool, bool, int, bool *, tree *, location_t *);
static cp_declarator *cp_parser_declarator
(cp_parser *, cp_parser_declarator_kind, int *, bool
[nvptx] SESE region optimization
I've applied this to trunk, it adds the SESE region neutering from the gomp4 branch. The general idea is that when neutering partitioned execution, one can skip from the entry of an SESE region straight to its exit, rather than do each block in the region individually. Finding the regions involves: 1) Finding cycle-equivalent blocks. These are blocks where a code path that reaches one of them is guaranteed to reach all of them. This uses the algorithm described by 'Finding Regions Fast: Single Entry Single Exit and control Regions in Linear Time' Johnson, Pearson & Pingali. (their algorithm doesn't actually find SESE regions, just the cycle-equivalent nodes) 2) Once those are found, we apply a DFS flood fill coloring algorithm, to determine the minimal set of SESE regions that cover the set of blocks we're interested in. The order of discovery allows us to determine which is the entry node and which is the exit node. Knowing the size of each equivalence set allows us to determine when we've found the exit node of an SESE region, or whether we're merely at a 'neck' within a larger region. One tricky bit is that we're not dealing with the entire function graph, but a subset of it described by the partitioned loop structure. I deal with this by treating all edges that traverse the partitioned region boundary as connected back to the function entry block (i.e. form a loop), when detecting SESE regions. For the coloring algorithm we traverse them, but don't color outside the partitioned region (such traversal can never be within an SESE region). nathan 2015-11-18 Nathan Sidwell gcc/ * config/nvptx/nvptx.c (bb_pair_t, bb_pair_vec_t): New types. (pseudo_node_t, struct bracket, bracket_vec_t): New types. (struct bb_sese): New struct. (bb_sese::~bb_sese, bb_sese::append, bb_sese::remove): New. (BB_GET_SESE, BB_SET_SESE): Define. (nvptx_sese_number, nvptx_sese_pseudo, nvptx_sese_color): New. (nvptx_find_sese): New. (nvptx_neuter_pars): Find SESE regions when optimizing. gcc/testsuite/ * gcc.dg/goacc/nvptx-sese-1.c: New. Index: gcc/config/nvptx/nvptx.c === --- gcc/config/nvptx/nvptx.c (revision 230549) +++ gcc/config/nvptx/nvptx.c (working copy) @@ -2605,6 +2605,631 @@ nvptx_discover_pars (bb_insn_map_t *map) return par; } +/* Analyse a group of BBs within a partitioned region and create N + Single-Entry-Single-Exit regions. Some of those regions will be + trivial ones consisting of a single BB. The blocks of a + partitioned region might form a set of disjoint graphs -- because + the region encloses a differently partitoned sub region. + + We use the linear time algorithm described in 'Finding Regions Fast: + Single Entry Single Exit and control Regions in Linear Time' + Johnson, Pearson & Pingali. That algorithm deals with complete + CFGs, where a back edge is inserted from END to START, and thus the + problem becomes one of finding equivalent loops. + + In this case we have a partial CFG. We complete it by redirecting + any incoming edge to the graph to be from an arbitrary external BB, + and similarly redirecting any outgoing edge to be to that BB. + Thus we end up with a closed graph. + + The algorithm works by building a spanning tree of an undirected + graph and keeping track of back edges from nodes further from the + root in the tree to nodes nearer to the root in the tree. In the + description below, the root is up and the tree grows downwards. + + We avoid having to deal with degenerate back-edges to the same + block, by splitting each BB into 3 -- one for input edges, one for + the node itself and one for the output edges. Such back edges are + referred to as 'Brackets'. Cycle equivalent nodes will have the + same set of brackets. + + Determining bracket equivalency is done by maintaining a list of + brackets in such a manner that the list length and final bracket + uniquely identify the set. + + We use coloring to mark all BBs with cycle equivalency with the + same color. This is the output of the 'Finding Regions Fast' + algorithm. Notice it doesn't actually find the set of nodes within + a particular region, just unorderd sets of nodes that are the + entries and exits of SESE regions. + + After determining cycle equivalency, we need to find the minimal + set of SESE regions. Do this with a DFS coloring walk of the + complete graph. We're either 'looking' or 'coloring'. When + looking, and we're in the subgraph, we start coloring the color of + the current node, and remember that node as the start of the + current color's SESE region. Every time we go to a new node, we + decrement the count of nodes with thet color. If it reaches zero, + we remember that node as the end of the current color's SESE region + and return to 'looking'. Otherwise we color the node the current + color. + +
[Ada] Fix ICE on on indirect renaming of address at -O
Addresses cannot be directly subject to a renaming, but they can nevertheless
be renamed through a constant and thus need to be dealt with as other rvalues.
Tested on x86_64-suse-linux, applied on the mainline.
2015-11-18 Eric Botcazou
* gcc-interface/decl.c (gnat_to_gnu_entity) : Use case #1
for the renaming of an address.
2015-11-18 Eric Botcazou
* gnat.dg/renaming7.adb: New test.
* gnat.dg/renaming7_pkg.ads: New helper.
--
Eric BotcazouIndex: gcc-interface/decl.c
===
--- gcc-interface/decl.c (revision 230557)
+++ gcc-interface/decl.c (working copy)
@@ -963,8 +963,8 @@ gnat_to_gnu_entity (Entity_Id gnat_entit
function call is a constant object. Therefore, it can be the
inner object of a constant renaming and the renaming must be
fully instantiated, i.e. it cannot be a reference to (part of)
- an existing object. And treat null expressions, constructors
- and literals the same way. */
+ an existing object. And treat other rvalues (addresses, null
+ expressions, constructors and literals) the same way. */
tree inner = gnu_expr;
while (handled_component_p (inner) || CONVERT_EXPR_P (inner))
inner = TREE_OPERAND (inner, 0);
@@ -974,6 +974,7 @@ gnat_to_gnu_entity (Entity_Id gnat_entit
inner = TREE_OPERAND (inner, 1);
if ((TREE_CODE (inner) == CALL_EXPR
&& !call_is_atomic_load (inner))
+ || TREE_CODE (inner) == ADDR_EXPR
|| TREE_CODE (inner) == NULL_EXPR
|| TREE_CODE (inner) == CONSTRUCTOR
|| CONSTANT_CLASS_P (inner))
-- { dg-do compile }
-- { dg-options "-O" }
with Renaming7_Pkg; use Renaming7_Pkg;
with System;
procedure Renaming7 is
C : constant System.Address := A'Address;
D : System.Address renames C;
begin
null;
end;
package Renaming7_Pkg is
A : Integer;
end Renaming7_Pkg;
[Ada] Fix stack usage increase for renaming of indexed component
This fixes an oversight in the new fold_constant_decl_in_expr function, which causes the code generated by the compiler for the renaming of an indexed component whose prefix is a constant object and whose index is not to use significantly more stack than needed. Tested on x86_64-suse-linux, applied on the mainline and 5.x branch. 2015-11-18 Eric Botcazou * gcc-interface/trans.c (fold_constant_decl_in_expr) : If the index is not itself constant then bail out. -- Eric BotcazouIndex: gcc-interface/trans.c === --- gcc-interface/trans.c (revision 230542) +++ gcc-interface/trans.c (working copy) @@ -975,6 +975,9 @@ fold_constant_decl_in_expr (tree exp) case ARRAY_REF: case ARRAY_RANGE_REF: + /* If the index is not itself constant, then nothing can be folded. */ + if (!TREE_CONSTANT (TREE_OPERAND (exp, 1))) + return exp; op0 = fold_constant_decl_in_expr (TREE_OPERAND (exp, 0)); if (op0 == TREE_OPERAND (exp, 0)) return exp;
Re: [gomp4.1] Handle linear clause modifiers in declare simd
On Wed, Nov 18, 2015 at 08:30:53PM +0300, Ilya Verbin wrote: > Could you please point to where the new ABI additions are documented? > I can't find R/L/U parameter types in [1] and [2]. > > [1] > https://sourceware.org/glibc/wiki/libmvec?action=AttachFile&do=view&target=VectorABI.txt > [2] https://groups.google.com/forum/#!topic/x86-64-abi/LmppCfN1rZ4 Xinmin should have those in the Intel ABI pdf, not sure if the latest pdf is publicly available or not. Jakub
Re: [PATCH] Add clang-format config to contrib folder
On 11/18/2015 10:34 AM, Manuel López-Ibáñez wrote: Which is a sad demonstration of how the refusal of GCC's FEs being re-used for other purposes by GNU tools (and others) was and is a mistake, and it is leading to GNU tools being replaced by LLVM-based ones (ultimately affecting GCC and GDB themselves). I think most developers would agree at this point. Some may have always agreed. But it was the decision of the FSF long ago to design GCC in this way and anyone working on GCC had to abide by that decision. Things are changing, but undoing that fundamental design decision is a *lot* of work. Don't expect it to land anytime soon. jeff
Re: [PATCH] Add clang-format config to contrib folder
On 18/11/15 17:05, Jeff Law wrote: As we've been continuously converting our source base to C++, the clang-format should provide better results than a collection of regular expressions (check_GNU_style.sh). As a reference file I attach gcc/tree-ssa-uninit.c file. Feel free to comment the suggested configuration file. This is fine. Given that gnu-indent seems to muck up C++ badly in my experience, clang-format may be a better long term solution. I'd really like to get to a point one day where formatting is a commit hook so that things are always kept properly formatted. Which is a sad demonstration of how the refusal of GCC's FEs being re-used for other purposes by GNU tools (and others) was and is a mistake, and it is leading to GNU tools being replaced by LLVM-based ones (ultimately affecting GCC and GDB themselves). Cheers, Manuel.
Re: [gomp4.1] Handle linear clause modifiers in declare simd
Hi!
On Wed, Jul 01, 2015 at 12:55:38 +0200, Jakub Jelinek wrote:
> I've committed following patch, which per the new ABI additions
> mangles and handles the various new linear clause modifiers in
> declare simd functions. The vectorizer side is not done yet,
>
> [...]
>
> @@ -14195,12 +14216,25 @@ simd_clone_mangle (struct cgraph_node *n
> {
>struct cgraph_simd_clone_arg arg = clone_info->args[n];
>
> - if (arg.arg_type == SIMD_CLONE_ARG_TYPE_UNIFORM)
> - pp_character (&pp, 'u');
> - else if (arg.arg_type == SIMD_CLONE_ARG_TYPE_LINEAR_CONSTANT_STEP)
> + switch (arg.arg_type)
> {
> - gcc_assert (arg.linear_step != 0);
> + case SIMD_CLONE_ARG_TYPE_UNIFORM:
> + pp_character (&pp, 'u');
> + break;
> + case SIMD_CLONE_ARG_TYPE_LINEAR_CONSTANT_STEP:
> pp_character (&pp, 'l');
> + goto mangle_linear;
> + case SIMD_CLONE_ARG_TYPE_LINEAR_REF_CONSTANT_STEP:
> + pp_character (&pp, 'R');
> + goto mangle_linear;
> + case SIMD_CLONE_ARG_TYPE_LINEAR_VAL_CONSTANT_STEP:
> + pp_character (&pp, 'L');
> + goto mangle_linear;
> + case SIMD_CLONE_ARG_TYPE_LINEAR_UVAL_CONSTANT_STEP:
> + pp_character (&pp, 'U');
> + goto mangle_linear;
> + mangle_linear:
> + gcc_assert (arg.linear_step != 0);
Could you please point to where the new ABI additions are documented?
I can't find R/L/U parameter types in [1] and [2].
[1]
https://sourceware.org/glibc/wiki/libmvec?action=AttachFile&do=view&target=VectorABI.txt
[2] https://groups.google.com/forum/#!topic/x86-64-abi/LmppCfN1rZ4
Thanks,
-- Ilya
Re: [PATCH] Add clang-format config to contrib folder
On 11/18/2015 07:10 AM, Martin Liška wrote:
Hello.
Following patch adds a clang-format config file that should respect the GNU
coding standards.
As the file is not part of build process, I hope the patch can be applied even
though
we've just skipped to stage3? The patch adds a hunk to Makefile which can
create symlink
to the root directory of the GCC compiler. The clang-format automatically loads
style from
the configuration file.
clang-format (version 3.8) provides rich variety of configuration options that
can
ensure the GNU coding style.
Limitations:
+ placement of opening brace of an initializer can't be requested
+ sometimes, '(' is the trailing symbol at the end of a line, which can look
weird
As we've been continuously converting our source base to C++, the clang-format
should
provide better results than a collection of regular expressions
(check_GNU_style.sh).
As a reference file I attach gcc/tree-ssa-uninit.c file.
Feel free to comment the suggested configuration file.
This is fine. Given that gnu-indent seems to muck up C++ badly in my
experience, clang-format may be a better long term solution. I'd really
like to get to a point one day where formatting is a commit hook so that
things are always kept properly formatted.
jeff
Re: [PATCH, VECTOR ABI] Add __attribute__((__simd__)) to GCC.
On 11/18/2015 07:11 AM, Kirill Yukhin wrote:
Hello Andreas, Devid.
On 18 Nov 10:45, Andreas Schwab wrote:
Kirill Yukhin writes:
diff --git a/gcc/testsuite/c-c++-common/attr-simd.c
b/gcc/testsuite/c-c++-common/attr-simd.c
new file mode 100644
index 000..b4eda34
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/attr-simd.c
@@ -0,0 +1,32 @@
+/* { dg-do compile } */
+/* { dg-options "-fdump-tree-optimized" } */
+
+__attribute__((__simd__))
+extern
+int simd_attr (void)
+{
+ return 0;
+}
+
+/* { dg-final { scan-tree-dump "simd_attr\[ \\t\]simdclone|vector" "optimized"
} } */
On ia64:
FAIL: c-c++-common/attr-simd.c -Wc++-compat scan-tree-dump optimized "simd_attr[
\\t]simdclone|vector"
FAIL: c-c++-common/attr-simd.c -Wc++-compat scan-tree-dump optimized "simd_attr2[
\\t]simdclone|vector"
$ grep simd_attr attr-simd.c.194t.optimized
;; Function simd_attr (simd_attr, funcdef_no=0, decl_uid=1389, cgraph_uid=0,
symbol_order=0)
simd_attr ()
;; Function simd_attr2 (simd_attr2, funcdef_no=1, decl_uid=1392, cgraph_uid=1,
symbol_order=1)
simd_attr2 ()
As far as vABI is supported on x86_64/i?86 only, I am going to enable mentioned
`scan-tree-dump' only
for these targets. This should cure both IA64 and Power.
Concerning attr-simd-3.c. It is known issue: PR68158.
And I believe this test should work everywhere as far as PR is resolved.
I'll put xfail into the test.
Which will lead to (in g++.log):
gcc/testsuite/c-c++-common/attr-simd-3.c:5:48: warning: '__simd__' attribute
does not apply to types\
[-Wattributes]^M
output is:
gcc/testsuite/c-c++-common/attr-simd-3.c:5:48: warning: '__simd__' attribute
does not apply to types\
[-Wattributes]^M
XFAIL: c-c++-common/attr-simd-3.c -std=gnu++98 PR68158 (test for errors, line
5)
FAIL: c-c++-common/attr-simd-3.c -std=gnu++98 (test for excess errors)
Excess errors:
gcc/testsuite/c-c++-common/attr-simd-3.c:5:48: warning: '__simd__' attribute
does not apply to types\
[-Wattributes]
Patch in the bottom.
gcc/tessuite/
* c-c++-common/attr-simd-3.c: Put xfail (PR68158) on dg-error.
* c-c++-common/attr-simd.c: Limit scan of dump to x86_64/i?86.
This is fine. Though we might consider whether or not we need another
target-supports if we find ourselves sprinkling too many target
conditionals on these tests.
jeff
Re: [PATCH] Fix memory leaks in tree-ssa-uninit.c
On 11/18/2015 07:23 AM, Martin Liška wrote: On 11/13/2015 08:19 PM, Jeff Law wrote: On 11/13/2015 09:58 AM, Martin Liška wrote: On 11/13/2015 05:32 PM, Jeff Law wrote: On 11/13/2015 05:50 AM, Martin Liška wrote: Hello. Patch survives regbootstrap on x86_64-linux-gnu. Ready for trunk? Thanks, Martin 0001-Fix-memory-leaks-in-tree-ssa-uninit.c.patch From 54851503251dee7a8bd074485db262715e628728 Mon Sep 17 00:00:00 2001 From: marxin Date: Fri, 13 Nov 2015 12:23:22 +0100 Subject: [PATCH] Fix memory leaks in tree-ssa-uninit.c gcc/ChangeLog: 2015-11-13 Martin Liska * tree-ssa-uninit.c (convert_control_dep_chain_into_preds): Fix GNU coding style. (find_def_preds): Use auto_vec. (destroy_predicate_vecs): Change signature of the function. (prune_uninit_phi_opnds_in_unrealizable_paths): Use the new signature. (simplify_preds_4): Use destroy_predicate_vecs instread of just releasing preds vector. (normalize_preds): Likewise. (is_use_properly_guarded): Use new signature of destroy_predicate_vecs. (find_uninit_use): Likewise. OK. FWIW, there's all kinds of spaces vs tabs issues in this file. I'm curious why you chose to fix convert_control_dep_chain_into_preds, but didn't change any others. Hi Jeff. Thanks for confirmation, you are right, it's full of coding style issues. I can change these if it would be desired? It's probably a good thing to do. Given it'd strictly be formatting, I'd even consider it post-stage1. jeff Hello. I'm sending the re-formatted source file. The source file was formatted by clang-format ([1]) and some hunk were manually re-formatted. No change in behavior. Patch can regbootstrap on x86_64-linux-gnu. Ready for trunk? OK. jeff
Re: [C PATCH] Don't leak C_MAYBE_CONST_EXPRs into fold() (PR c/68412)
On Wed, Nov 18, 2015 at 05:24:34PM +0100, Bernd Schmidt wrote: > That seems to change the behaviour of the code though. Most of the code in > warn_tautological_cmp only looks at the unfolded form. Are you sure this is > safe, for example wrt. the from_macro_expansion_at tests or the > CONVERT_EXPR_P ones? Of course the second version is wrong wrt CONVERT_EXPR_P, because it would regress g++.dg/warn/Wtautological-compare.C -- with the fold() in the C++ FE we'd get rid of the CONVERT_EXPR_P completely and we'd warn. Sorry :(. So I guess scratch the second version and let's get back to the first one. Jason's r230471 was a correct thing to do. Marek
Re: [Patch,testsuite]: Fix the tree-ssa/split-path-1.c testcase
On 11/18/2015 07:21 AM, Ajit Kumar Agarwal wrote: Hello Jeff: Please ignore my previous mails as they bounced back. Sorry for that. I have fixed the problem with the testcase. The splitting path optimization remains intact. Attached is the patch. The problem was related to the testcase as the loop bound goes beyond the malloced array. There was also a problem with accessing the elements of EritePtr. ChangeLog: 2015-11-18 Ajit Agarwal * gcc.dg/tree-ssa/split-path-1.c: Fix the testcase. THanks. Installed. I wasn't sure why you needed , but I left it as-is. jeff
Re: [PATCH 1/4 v2][AArch64] Generalize CCMP support
On 11/17/2015 07:35 PM, Wilco Dijkstra wrote: I've updated the comments and documentation, see below. I hope it makes sense now - returning rtx that computes the same value as the tree expression we're emitting is the most useful thing one can do. The target-independent part is ok. Note that some of the documentation parts were word-wrapped; please check that you've not exceeded the line lengths, and make sure you send future patches in a way that doesn't suffer from that (text/plain attachments for example). Bernd
PR 68393: Handle SUBREG_PROMOTED_VAR_P in expand_direct_optab_fn
Do the usual dance when assigning to SUBREG_PROMOTED_VAR_P destinations:
first convert to the outer mode, then extend to the inner mode.
This fixes the powerpc64le bootstrap failure reported in PR 68393.
Tested that it fixes the powerpc64le-linux-gnu breakage. Also tested
on x86_64-linux-gnu and powerpc64-linux-gnu. OK to install?
Thanks,
Richard
PS. Sorry for the delay in fixing this. I posted a patch to bugzilla
yesterday to fix the case where the modes of the lhs and .md pattern
are different, but we need the same behaviour when the modes are the same.
gcc/
PR bootstrap/68393
* internal-fn.c (expand_direct_optab_fn): Handle SUBREG_PROMOTED_VAR_P
destinations.
diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c
index df3b7dc..bc77bdc 100644
--- a/gcc/internal-fn.c
+++ b/gcc/internal-fn.c
@@ -2124,14 +2124,30 @@ expand_direct_optab_fn (internal_fn fn, gcall *stmt,
direct_optab optab,
expand_insn (icode, nargs + 1, ops);
if (!rtx_equal_p (lhs_rtx, ops[0].value))
{
- if (INTEGRAL_TYPE_P (lhs_type))
- /* Convert the operand to the required type, which is useful
- for things that return an int regardless of the size of
- the input. If the value produced by the instruction is
- smaller than required, assume that it is signed. */
- convert_move (lhs_rtx, ops[0].value, 0);
- else
+ /* If the return value has an integral type, convert the instruction
+result to that type. This is useful for things that return an
+int regardless of the size of the input. If the instruction result
+is smaller than required, assume that it is signed.
+
+If the return value has a nonintegral type, its mode must match
+the instruction result. */
+ if (GET_CODE (lhs_rtx) == SUBREG && SUBREG_PROMOTED_VAR_P (lhs_rtx))
+ {
+ /* If this is a scalar in a register that is stored in a wider
+mode than the declared mode, compute the result into its
+declared mode and then convert to the wider mode. */
+ gcc_checking_assert (INTEGRAL_TYPE_P (lhs_type));
+ rtx tmp = convert_to_mode (GET_MODE (lhs_rtx), ops[0].value, 0);
+ convert_move (SUBREG_REG (lhs_rtx), tmp,
+ SUBREG_PROMOTED_SIGN (lhs_rtx));
+ }
+ else if (GET_MODE (lhs_rtx) == GET_MODE (ops[0].value))
emit_move_insn (lhs_rtx, ops[0].value);
+ else
+ {
+ gcc_checking_assert (INTEGRAL_TYPE_P (lhs_type));
+ convert_move (lhs_rtx, ops[0].value, 0);
+ }
}
}
[PATCH 4/4 v2][AArch64] Cost CCMP instruction sequences to choose better expand order
(v2 cleans up enum use)
This patch adds CCMP selection based on rtx costs. This is based on Jiong's
already approved patch
https://gcc.gnu.org/ml/gcc-patches/2015-09/msg01434.html with some minor
refactoring and enum cleanup plus the tests updated.
OK for commit?
ChangeLog:
2015-11-13 Jiong Wang
2015-11-18 Wilco Dijkstra
gcc/
* ccmp.c (expand_ccmp_expr_1): Cost the instruction sequences
generated from different expand order. Cleanup enum use.
gcc/testsuite/
* gcc.target/aarch64/ccmp_1.c: Update test.
---
gcc/ccmp.c| 65
---
gcc/testsuite/gcc.target/aarch64/ccmp_1.c | 15 ++-
2 files changed, 64 insertions(+), 16 deletions(-)
diff --git a/gcc/ccmp.c b/gcc/ccmp.c
index 3698a7d..0c677fd 100644
--- a/gcc/ccmp.c
+++ b/gcc/ccmp.c
@@ -51,6 +51,7 @@ along with GCC; see the file COPYING3. If not see
#include "tree-outof-ssa.h"
#include "cfgexpand.h"
#include "ccmp.h"
+#include "predict.h"
/* The following functions expand conditional compare (CCMP) instructions.
Here is a short description about the over all algorithm:
@@ -88,7 +89,7 @@ ccmp_candidate_p (gimple *g)
tree rhs = gimple_assign_rhs_to_tree (g);
tree lhs, op0, op1;
gimple *gs0, *gs1;
- enum tree_code tcode, tcode0, tcode1;
+ tree_code tcode, tcode0, tcode1;
tcode = TREE_CODE (rhs);
if (tcode != BIT_AND_EXPR && tcode != BIT_IOR_EXPR)
@@ -135,10 +136,10 @@ ccmp_candidate_p (gimple *g)
PREP_SEQ returns all insns to prepare opearands for compare.
GEN_SEQ returns all compare insns. */
static rtx
-expand_ccmp_next (gimple *g, enum tree_code code, rtx prev,
+expand_ccmp_next (gimple *g, tree_code code, rtx prev,
rtx *prep_seq, rtx *gen_seq)
{
- enum rtx_code rcode;
+ rtx_code rcode;
int unsignedp = TYPE_UNSIGNED (TREE_TYPE (gimple_assign_rhs1 (g)));
gcc_assert (code == BIT_AND_EXPR || code == BIT_IOR_EXPR);
@@ -165,13 +166,15 @@ expand_ccmp_next (gimple *g, enum tree_code code, rtx
prev,
static rtx
expand_ccmp_expr_1 (gimple *g, rtx *prep_seq, rtx *gen_seq)
{
+ rtx prep_seq_1, gen_seq_1;
+ rtx prep_seq_2, gen_seq_2;
tree exp = gimple_assign_rhs_to_tree (g);
- enum tree_code code = TREE_CODE (exp);
+ tree_code code = TREE_CODE (exp);
gimple *gs0 = get_gimple_for_ssa_name (TREE_OPERAND (exp, 0));
gimple *gs1 = get_gimple_for_ssa_name (TREE_OPERAND (exp, 1));
rtx tmp;
- enum tree_code code0 = gimple_assign_rhs_code (gs0);
- enum tree_code code1 = gimple_assign_rhs_code (gs1);
+ tree_code code0 = gimple_assign_rhs_code (gs0);
+ tree_code code1 = gimple_assign_rhs_code (gs1);
gcc_assert (code == BIT_AND_EXPR || code == BIT_IOR_EXPR);
gcc_assert (gs0 && gs1 && is_gimple_assign (gs0) && is_gimple_assign
(gs1));
@@ -180,19 +183,53 @@ expand_ccmp_expr_1 (gimple *g, rtx *prep_seq, rtx
*gen_seq)
{
if (TREE_CODE_CLASS (code1) == tcc_comparison)
{
- int unsignedp0;
- enum rtx_code rcode0;
+ int unsignedp0, unsignedp1;
+ rtx_code rcode0, rcode1;
+ int speed_p = optimize_insn_for_speed_p ();
+ rtx tmp2, ret, ret2;
+ unsigned cost1 = MAX_COST;
+ unsigned cost2 = MAX_COST;
unsignedp0 = TYPE_UNSIGNED (TREE_TYPE (gimple_assign_rhs1 (gs0)));
+ unsignedp1 = TYPE_UNSIGNED (TREE_TYPE (gimple_assign_rhs1 (gs1)));
rcode0 = get_rtx_code (code0, unsignedp0);
+ rcode1 = get_rtx_code (code1, unsignedp1);
- tmp = targetm.gen_ccmp_first (prep_seq, gen_seq, rcode0,
+ tmp = targetm.gen_ccmp_first (&prep_seq_1, &gen_seq_1, rcode0,
gimple_assign_rhs1 (gs0),
gimple_assign_rhs2 (gs0));
- if (!tmp)
+
+ tmp2 = targetm.gen_ccmp_first (&prep_seq_2, &gen_seq_2, rcode1,
+gimple_assign_rhs1 (gs1),
+gimple_assign_rhs2 (gs1));
+
+ if (!tmp && !tmp2)
return NULL_RTX;
- return expand_ccmp_next (gs1, code, tmp, prep_seq, gen_seq);
+ if (tmp != NULL)
+ {
+ ret = expand_ccmp_next (gs1, code, tmp, &prep_seq_1,
&gen_seq_1);
+ cost1 = seq_cost (safe_as_a (prep_seq_1),
speed_p);
+ cost1 += seq_cost (safe_as_a (gen_seq_1),
speed_p);
+ }
+ if (tmp2 != NULL)
+ {
+ ret2 = expand_ccmp_next (gs0, code, tmp2, &prep_seq_2,
+ &gen_seq_2);
+ cost2 = seq_cost (safe_as_a (prep_seq_2),
speed_p);
+ cost2 += seq_cost (safe_as_a (gen_seq_2),
speed_p);
+ }
+
+ if (cost2 < cost1)
+ {
+ *prep_seq = prep_seq_2;
+ *gen_seq = gen_seq_2;
+ return ret2;
+ }
+
+ *prep_seq = prep_seq_1;
+ *gen_seq = gen_seq_1;
+ return ret;
}
else
[PATCH 2/4 v2][AArch64] Add support for FCCMP
(v2 version removes 4 enums) This patch adds support for FCCMP. This is trivial with the new CCMP representation - remove the restriction of FP in ccmp.c and add FCCMP patterns. Add a test to ensure FCCMP/FCCMPE are emitted as expected. OK for commit? ChangeLog: 2015-11-18 Wilco Dijkstra * gcc/ccmp.c (ccmp_candidate_p): Remove integer-only restriction. * gcc/config/aarch64/aarch64.md (fccmp): New pattern. (fccmpe): Likewise. (fcmp): Rename to fcmp and globalize pattern. (fcmpe): Likewise. * gcc/config/aarch64/aarch64.c (aarch64_gen_ccmp_first): Add FP support. (aarch64_gen_ccmp_next): Add FP support. gcc/testsuite/ * gcc.target/aarch64/ccmp_1.c: New testcase. --- gcc/ccmp.c| 6 --- gcc/config/aarch64/aarch64.c | 24 + gcc/config/aarch64/aarch64.md | 34 - gcc/testsuite/gcc.target/aarch64/ccmp_1.c | 84 +++ 4 files changed, 140 insertions(+), 8 deletions(-) create mode 100644 gcc/testsuite/gcc.target/aarch64/ccmp_1.c diff --git a/gcc/ccmp.c b/gcc/ccmp.c index 58ac126..3698a7d 100644 --- a/gcc/ccmp.c +++ b/gcc/ccmp.c @@ -112,12 +112,6 @@ ccmp_candidate_p (gimple *g) || gimple_bb (gs0) != gimple_bb (g)) return false; - if (!(INTEGRAL_TYPE_P (TREE_TYPE (gimple_assign_rhs1 (gs0))) - || POINTER_TYPE_P (TREE_TYPE (gimple_assign_rhs1 (gs0 - || !(INTEGRAL_TYPE_P (TREE_TYPE (gimple_assign_rhs1 (gs1))) - || POINTER_TYPE_P (TREE_TYPE (gimple_assign_rhs1 (gs1) -return false; - tcode0 = gimple_assign_rhs_code (gs0); tcode1 = gimple_assign_rhs_code (gs1); if (TREE_CODE_CLASS (tcode0) == tcc_comparison diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index c8bee3b..db4d190 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -12398,6 +12398,18 @@ aarch64_gen_ccmp_first (rtx *prep_seq, rtx *gen_seq, icode = CODE_FOR_cmpdi; break; +case SFmode: + cmp_mode = SFmode; + cc_mode = aarch64_select_cc_mode ((rtx_code) code, op0, op1); + icode = cc_mode == CCFPEmode ? CODE_FOR_fcmpesf : CODE_FOR_fcmpsf; + break; + +case DFmode: + cmp_mode = DFmode; + cc_mode = aarch64_select_cc_mode ((rtx_code) code, op0, op1); + icode = cc_mode == CCFPEmode ? CODE_FOR_fcmpedf : CODE_FOR_fcmpdf; + break; + default: end_sequence (); return NULL_RTX; @@ -12461,6 +12473,18 @@ aarch64_gen_ccmp_next (rtx *prep_seq, rtx *gen_seq, rtx prev, int cmp_code, icode = CODE_FOR_ccmpdi; break; +case SFmode: + cmp_mode = SFmode; + cc_mode = aarch64_select_cc_mode ((rtx_code) cmp_code, op0, op1); + icode = cc_mode == CCFPEmode ? CODE_FOR_fccmpesf : CODE_FOR_fccmpsf; + break; + +case DFmode: + cmp_mode = DFmode; + cc_mode = aarch64_select_cc_mode ((rtx_code) cmp_code, op0, op1); + icode = cc_mode == CCFPEmode ? CODE_FOR_fccmpedf : CODE_FOR_fccmpdf; + break; + default: end_sequence (); return NULL_RTX; diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md index fab65c6..7d728b5 100644 --- a/gcc/config/aarch64/aarch64.md +++ b/gcc/config/aarch64/aarch64.md @@ -279,6 +279,36 @@ [(set_attr "type" "alus_sreg,alus_imm,alus_imm")] ) +(define_insn "fccmp" + [(set (match_operand:CCFP 1 "cc_register" "") + (if_then_else:CCFP + (match_operator 4 "aarch64_comparison_operator" + [(match_operand 0 "cc_register" "") + (const_int 0)]) + (compare:CCFP + (match_operand:GPF 2 "register_operand" "w") + (match_operand:GPF 3 "register_operand" "w")) + (match_operand 5 "immediate_operand")))] + "TARGET_FLOAT" + "fccmp\\t%2, %3, %k5, %m4" + [(set_attr "type" "fcmp")] +) + +(define_insn "fccmpe" + [(set (match_operand:CCFPE 1 "cc_register" "") +(if_then_else:CCFPE + (match_operator 4 "aarch64_comparison_operator" + [(match_operand 0 "cc_register" "") + (const_int 0)]) + (compare:CCFPE + (match_operand:GPF 2 "register_operand" "w") + (match_operand:GPF 3 "register_operand" "w")) + (match_operand 5 "immediate_operand")))] + "TARGET_FLOAT" + "fccmpe\\t%2, %3, %k5, %m4" + [(set_attr "type" "fcmp")] +) + ;; Expansion of signed mod by a power of 2 using CSNEG. ;; For x0 % n where n is a power of 2 produce: ;; negs x1, x0 @@ -2794,7 +2824,7 @@ [(set_attr "type" "alus_sreg,alus_imm,alus_imm")] ) -(define_insn "*cmp" +(define_insn "fcmp" [(set (reg:CCFP CC_REGNUM) (compare:CCFP (match_operand:GPF 0 "register_operand" "w,w") (match_operand:GPF 1 "aarch64_fp_compare_operand" "Y,w")))] @@ -2805,7 +2835,7 @@ [(set_attr "type" "fcmp")] ) -(define_insn "*cmpe" +(define_insn "fcmpe" [(set (reg:CCFPE CC_REGNUM) (compare:CCFPE (
Re: teach delay folding in c++ about OACC_LOOPs
On 11/18/15 10:14, Cesar Philippidis wrote: Jason, Your recent delay folding patch broke libgomp.oacc-c++/loop-auto-1.c. It looks like you forgot to handle OACC_LOOP in cp_fold_r. You probably didn't notice this because Nathan committed his auto acc loop patch just before you applied your patch. I'm not sure why only that test is affected though. Is this patch ok for trunk? Hey, once again you've saved me an investigation :) I'd say this counts as obvious, so go ahead and commit. nathan
RE: [PATCH 1/4 v2][AArch64] Generalize CCMP support
Bernd Schmidt wrote: > Sent: 17 November 2015 22:16 > To: Wilco Dijkstra; gcc-patches@gcc.gnu.org > Subject: Re: [PATCH 1/4][AArch64] Generalize CCMP support > > On 11/13/2015 05:02 PM, Wilco Dijkstra wrote: > > * gcc/ccmp.c (expand_ccmp_expr): Extract cmp_code from return > value > > of > > expand_ccmp_expr_1. > > I was trying to review this part of the patch in isolation and got very confused > because the patch also changes the return values of the ccmp target hooks, > but does not update the documentation. > > In this file, return values are also underdocumented in function comments. I've updated the comments and documentation, see below. I hope it makes sense now - returning rtx that computes the same value as the tree expression we're emitting is the most useful thing one can do. > Also, > > > + enum rtx_code cmp_code = GET_CODE (tmp); > > Lose the "enum". Elsewhere in the patch too. > > Other than that this part is probably fine (leaving the aarch64 part to the > appropriate maintainers), but please resubmit with these issues fixed. I removed the enum here and updated the other patches as well. Wilco 2015-11-18 Wilco Dijkstra * gcc/target.def (gen_ccmp_first): Update documentation. (gen_ccmp_next): Likewise. * gcc/doc/tm.texi (gen_ccmp_first): Update documentation. (gen_ccmp_next): Likewise. * gcc/ccmp.c (expand_ccmp_expr): Extract cmp_code from return value of expand_ccmp_expr_1. Improve comments. * gcc/config/aarch64/aarch64.md (ccmp_and): Use if_then_else for ccmp. (ccmp_ior): Remove pattern. (cmp): Remove expand. (cmp): Globalize pattern. (cstorecc4): Use cc_register. (movcc): Remove ccmp_cc_register check. * gcc/config/aarch64/aarch64.c (aarch64_get_condition_code_1): Simplify after removal of CC_DNE/* modes. (aarch64_ccmp_mode_to_code): Remove. (aarch64_print_operand): Remove 'K' case. Merge 'm' and 'M' cases. In 'k' case use integer as condition. (aarch64_nzcv_codes): Remove inverted cases. (aarch64_code_to_ccmode): Remove. (aarch64_gen_ccmp_first): Use cmp pattern directly. Return the correct comparison with CC register to be used in folowing CCMP/branch/CSEL. (aarch64_gen_ccmp_next): Use previous comparison and mode in CCMP pattern. Return the comparison with CC register. Invert conditions when bitcode is OR. * gcc/config/aarch64/aarch64-modes.def: Remove CC_DNE/* modes. * gcc/config/aarch64/predicates.md (ccmp_cc_register): Remove. --- gcc/ccmp.c | 21 ++- gcc/config/aarch64/aarch64-modes.def | 10 -- gcc/config/aarch64/aarch64.c | 305 --- gcc/config/aarch64/aarch64.md| 68 ++-- gcc/config/aarch64/predicates.md | 17 -- gcc/doc/tm.texi | 36 ++--- gcc/target.def | 36 ++--- 7 files changed, 128 insertions(+), 365 deletions(-) diff --git a/gcc/ccmp.c b/gcc/ccmp.c index 20348d9..58ac126 100644 --- a/gcc/ccmp.c +++ b/gcc/ccmp.c @@ -65,6 +65,10 @@ along with GCC; see the file COPYING3. If not see - gen_ccmp_first expands the first compare in CCMP. - gen_ccmp_next expands the following compares. + Both hooks return a comparison with the CC register that is equivalent + to the value of the gimple comparison. This is used by the next CCMP + and in the final conditional store. + * We use cstorecc4 pattern to convert the CCmode intermediate to the integer mode result that expand_normal is expecting. @@ -130,10 +134,12 @@ ccmp_candidate_p (gimple *g) return false; } -/* PREV is the CC flag from precvious compares. The function expands the - next compare based on G which ops previous compare with CODE. +/* PREV is a comparison with the CC register which represents the + result of the previous CMP or CCMP. The function expands the + next compare based on G which is ANDed/ORed with the previous + compare depending on CODE. PREP_SEQ returns all insns to prepare opearands for compare. - GEN_SEQ returnss all compare insns. */ + GEN_SEQ returns all compare insns. */ static rtx expand_ccmp_next (gimple *g, enum tree_code code, rtx prev, rtx *prep_seq, rtx *gen_seq) @@ -226,7 +232,7 @@ expand_ccmp_expr_1 (gimple *g, rtx *prep_seq, rtx *gen_seq) return NULL_RTX; } -/* Main entry to expand conditional compare statement G. +/* Main entry to expand conditional compare statement G. Return NULL_RTX if G is not a legal candidate or expand fail. Otherwise return the target. */ rtx @@ -249,9 +255,10 @@ expand_ccmp_expr (gimple *g) enum insn_code icode; enum machine_mode cc_mode = CCmode; tree lhs = gimple_assign_lhs (g); + rtx_code cmp_code = GET_CODE (tmp); #ifdef SELECT_CC_MODE - cc_mode = SE
Re: [C PATCH] Don't leak C_MAYBE_CONST_EXPRs into fold() (PR c/68412)
On 11/18/2015 05:16 PM, Marek Polacek wrote: Actually, no, I think we should do this instead. +++ gcc/c-family/c-common.c @@ -1924,7 +1924,7 @@ warn_tautological_cmp (location_t loc, enum tree_code code, tree lhs, tree rhs) /* We do not warn for constants because they are typical of macro expansions that test for features, sizeof, and similar. */ - if (CONSTANT_CLASS_P (fold (lhs)) || CONSTANT_CLASS_P (fold (rhs))) + if (CONSTANT_CLASS_P (lhs) || CONSTANT_CLASS_P (rhs)) return; /* Don't warn for e.g. diff --git gcc/cp/call.c gcc/cp/call.c index 8cdda62..77c2936 100644 --- gcc/cp/call.c +++ gcc/cp/call.c @@ -5741,7 +5741,7 @@ build_new_op_1 (location_t loc, enum tree_code code, int flags, tree arg1, maybe_warn_bool_compare (loc, code, fold (arg1), fold (arg2)); if (complain & tf_warning && warn_tautological_compare) - warn_tautological_cmp (loc, code, arg1, arg2); + warn_tautological_cmp (loc, code, fold (arg1), fold (arg2)); /* Fall through. */ case PLUS_EXPR: case MINUS_EXPR: That seems to change the behaviour of the code though. Most of the code in warn_tautological_cmp only looks at the unfolded form. Are you sure this is safe, for example wrt. the from_macro_expansion_at tests or the CONVERT_EXPR_P ones? Bernd
Re: [PATCH, 10/16] Add pass_oacc_kernels pass group in passes.def
On November 18, 2015 9:30:23 AM GMT+01:00, Richard Biener
wrote:
>On Tue, 17 Nov 2015, Tom de Vries wrote:
>
>> On 17/11/15 16:18, Richard Biener wrote:
>> > > > IMHO autopar needs to handle induction itself.
>> > > >
>> > > >I'm not sure what you mean. Could you elaborate? Autopar
>handles
>> > > induction
>> > > >variables, but it doesn't handle exit phis reading the final
>value of the
>> > > >induction variable. Is that what you want fixed? How?
>> > Yes. Perform final value replacement.
>> >
>>
>> I see. Calling scev_const_prop in pass_parallelize_loops_oacc_kernels
>seems to
>> work fine.
>>
>> Doing the same for pass_parallelize_loops like this:
>> ...
>> diff --git a/gcc/tree-parloops.c b/gcc/tree-parloops.c
>> index 17415a8..d944395 100644
>> --- a/gcc/tree-parloops.c
>> +++ b/gcc/tree-parloops.c
>> @@ -2787,6 +2787,9 @@ pass_parallelize_loops::execute (function *fun)
>>if (number_of_loops (fun) <= 1)
>> return 0;
>>
>> + unsigned int sccp_todo = scev_const_prop ();
>> + gcc_assert (sccp_todo == 0);
>> +
>>if (parallelize_loops ())
>> {
>>fun->curr_properties &= ~(PROP_gimple_eomp);
>> ...
>> seems to fix PR 68373 - "autopar fails on loop exit phi with argument
>defined
>> outside loop".
>>
>> The new scev_const_prop call in autopar rewrites this phi into an
>assignment,
>> and that allows parloops to succeed:
>> ...
>> final value replacement:
>> n_2 = PHI
>> with
>> n_2 = n_4(D);
>> ...
>
>That works for me but please factor out the final value replacement
>code from scev_const_prop. I think best would be to have a
>helper that does final value replacement for a single loop so you
>can call it for loops to paralellize only.
Bonus points for fixing the dump_file to parse in:
>Parloops will fail because:
>...
>phi is n_2 = PHI
>arg of phi to exit: value n_4(D) used outside loop
>checking if it a part of reduction pattern:
s/it a/it is/
>FAILED: it is not a part of reduction
>...
TIA,
>
>Richard.
>
>> Thanks,
>> - Tom
>>
>>
Re: [C PATCH] Don't leak C_MAYBE_CONST_EXPRs into fold() (PR c/68412)
On Wed, Nov 18, 2015 at 05:03:48PM +0100, Marek Polacek wrote:
> Since C++ delayed folding, warn_tautological_cmp needs to fold its arguments.
> But sometimes this function gets C_MAYBE_CONST_EXPR from the C FE, and fold()
> duly ICEs.
>
> I was torn if I should just return from warn_tautological_cmp and not warn
> when it gets C_MAYBE_CONST_EXPR as an argument, or if I should strip
> C_MAYBE_CONST_EXPR before calling warn_tautological_cmp. I decided for the
> latter since I think warn_tautological_cmp probably should warn on code such
> as
> int i = 10;
> bool b = ({ i; }) == ({ i; });
> (ugh).
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
Actually, no, I think we should do this instead.
Ok if testing passes?
2015-11-18 Marek Polacek
PR c/68412
* c-common.c (warn_tautological_cmp): Don't fold arguments here.
* call.c (build_new_op_1): Fold arguments to warn_tautological_cmp.
* gcc.dg/pr68412.c: New test.
diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c
index f50ca48..06d857c 100644
--- gcc/c-family/c-common.c
+++ gcc/c-family/c-common.c
@@ -1924,7 +1924,7 @@ warn_tautological_cmp (location_t loc, enum tree_code
code, tree lhs, tree rhs)
/* We do not warn for constants because they are typical of macro
expansions that test for features, sizeof, and similar. */
- if (CONSTANT_CLASS_P (fold (lhs)) || CONSTANT_CLASS_P (fold (rhs)))
+ if (CONSTANT_CLASS_P (lhs) || CONSTANT_CLASS_P (rhs))
return;
/* Don't warn for e.g.
diff --git gcc/cp/call.c gcc/cp/call.c
index 8cdda62..77c2936 100644
--- gcc/cp/call.c
+++ gcc/cp/call.c
@@ -5741,7 +5741,7 @@ build_new_op_1 (location_t loc, enum tree_code code, int
flags, tree arg1,
maybe_warn_bool_compare (loc, code, fold (arg1),
fold (arg2));
if (complain & tf_warning && warn_tautological_compare)
- warn_tautological_cmp (loc, code, arg1, arg2);
+ warn_tautological_cmp (loc, code, fold (arg1), fold (arg2));
/* Fall through. */
case PLUS_EXPR:
case MINUS_EXPR:
diff --git gcc/testsuite/gcc.dg/pr68412.c gcc/testsuite/gcc.dg/pr68412.c
index e69de29..825eb63 100644
--- gcc/testsuite/gcc.dg/pr68412.c
+++ gcc/testsuite/gcc.dg/pr68412.c
@@ -0,0 +1,41 @@
+/* PR c/68412 */
+/* { dg-do compile } */
+/* { dg-options "-Wall -Wextra" } */
+
+#define M (sizeof (int) * __CHAR_BIT__)
+
+int
+fn1 (int i)
+{
+ return i == (-1 << 8); /* { dg-warning "left shift of negative value" } */
+}
+
+int
+fn2 (int i)
+{
+ return i == (1 << M); /* { dg-warning "left shift count" } */
+}
+
+int
+fn3 (int i)
+{
+ return i == 10 << (M - 1); /* { dg-warning "requires" } */
+}
+
+int
+fn4 (int i)
+{
+ return i == 1 << -1; /* { dg-warning "left shift count" } */
+}
+
+int
+fn5 (int i)
+{
+ return i == 1 >> M; /* { dg-warning "right shift count" } */
+}
+
+int
+fn6 (int i)
+{
+ return i == 1 >> -1; /* { dg-warning "right shift count" } */
+}
Marek
[C PATCH] Don't leak C_MAYBE_CONST_EXPRs into fold() (PR c/68412)
Since C++ delayed folding, warn_tautological_cmp needs to fold its arguments.
But sometimes this function gets C_MAYBE_CONST_EXPR from the C FE, and fold()
duly ICEs.
I was torn if I should just return from warn_tautological_cmp and not warn
when it gets C_MAYBE_CONST_EXPR as an argument, or if I should strip
C_MAYBE_CONST_EXPR before calling warn_tautological_cmp. I decided for the
latter since I think warn_tautological_cmp probably should warn on code such
as
int i = 10;
bool b = ({ i; }) == ({ i; });
(ugh).
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2015-11-18 Marek Polacek
PR c/68412
* c-typeck.c (parser_build_binary_op): Strip C_MAYBE_CONST_EXPR
when passing arguments to warn_tautological_compare.
* gcc.dg/pr68412.c: New test.
* gcc.dg/pr68412-2.c: New test.
diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c
index c18c307..48c1a98 100644
--- gcc/c/c-typeck.c
+++ gcc/c/c-typeck.c
@@ -3512,7 +3512,11 @@ parser_build_binary_op (location_t location, enum
tree_code code,
code1, arg1.value, code2, arg2.value);
if (warn_tautological_compare)
-warn_tautological_cmp (location, code, arg1.value, arg2.value);
+warn_tautological_cmp (location, code,
+ /* This function will try to fold both
+ args, so don't leak C_MAYBE_CONST_EXPR. */
+ remove_c_maybe_const_expr (arg1.value),
+ remove_c_maybe_const_expr (arg2.value));
if (warn_logical_not_paren
&& TREE_CODE_CLASS (code) == tcc_comparison
diff --git gcc/testsuite/gcc.dg/pr68412-2.c gcc/testsuite/gcc.dg/pr68412-2.c
index e69de29..be1dcfa 100644
--- gcc/testsuite/gcc.dg/pr68412-2.c
+++ gcc/testsuite/gcc.dg/pr68412-2.c
@@ -0,0 +1,15 @@
+/* PR c/68412 */
+/* { dg-do compile } */
+/* { dg-options "-Wall -Wextra" } */
+
+int
+fn1 (int i)
+{
+ return ({ i; }) == ({ i; }); /* { dg-warning "self-comparison always
evaluates to true" } */
+}
+
+int
+fn2 (int i)
+{
+ return ({ i + 1; }) != ({ i + 1; }); /* { dg-warning "self-comparison always
evaluates to false" } */
+}
diff --git gcc/testsuite/gcc.dg/pr68412.c gcc/testsuite/gcc.dg/pr68412.c
index e69de29..825eb63 100644
--- gcc/testsuite/gcc.dg/pr68412.c
+++ gcc/testsuite/gcc.dg/pr68412.c
@@ -0,0 +1,41 @@
+/* PR c/68412 */
+/* { dg-do compile } */
+/* { dg-options "-Wall -Wextra" } */
+
+#define M (sizeof (int) * __CHAR_BIT__)
+
+int
+fn1 (int i)
+{
+ return i == (-1 << 8); /* { dg-warning "left shift of negative value" } */
+}
+
+int
+fn2 (int i)
+{
+ return i == (1 << M); /* { dg-warning "left shift count" } */
+}
+
+int
+fn3 (int i)
+{
+ return i == 10 << (M - 1); /* { dg-warning "requires" } */
+}
+
+int
+fn4 (int i)
+{
+ return i == 1 << -1; /* { dg-warning "left shift count" } */
+}
+
+int
+fn5 (int i)
+{
+ return i == 1 >> M; /* { dg-warning "right shift count" } */
+}
+
+int
+fn6 (int i)
+{
+ return i == 1 >> -1; /* { dg-warning "right shift count" } */
+}
Marek
Re: [PATCH PR52272]Be smart when adding iv candidates
On 11/10/2015 11:19 AM, Bin.Cheng wrote: On Tue, Nov 10, 2015 at 6:06 PM, Bernd Schmidt wrote: Multi-line expressions should be wrapped in parentheses so that emacs/indent can format them automatically. Two sets of parens are needed for this. Operators should then line up appropriately. Ah, thanks for teaching. Here is the updated patch, hoping it's correct. It looks like you're waiting to check it in - Richard's earlier approval still holds. Bernd
Re: [PATCH][ARM] Do not expand movmisalign pattern if not in 32-bit mode
Ping. https://gcc.gnu.org/ml/gcc-patches/2015-11/msg01392.html Thanks, Kyrill On 11/11/15 16:10, Kyrill Tkachov wrote: Hi all, The attached testcase ICEs when compiled with -march=armv6k -mthumb -Os or any march for which -mthumb gives Thumb1: error: unrecognizable insn: } ^ (insn 13 12 14 5 (set (reg:SI 116 [ x ]) (unspec:SI [ (mem:SI (reg/v/f:SI 112 [ s ]) [0 MEM[(unsigned char *)s_1(D)]+0 S4 A8]) ] UNSPEC_UNALIGNED_LOAD)) besttry.c:9 -1 (nil)) The problem is that the expands a movmisalign pattern but the resulting unaligned loads don't match any define_insn because they are gated on unaligned_access && TARGET_32BIT. The unaligned_access expander is gated only on unaligned_access. This small patch fixes the issue by turning off unaligned_access if TARGET_32BIT is not true. We can then remove TARGET_32BIT from the unaligned load/store patterns conditions as a cleanup. Bootstrapped and tested on arm-none-linux-gnueabihf. Ok for trunk? Thanks, Kyrill 2015-11-11 Kyrylo Tkachov * config/arm/arm.c (arm_option_override): Require TARGET_32BIT for unaligned_access. * config/arm/arm.md (unaligned_loadsi): Remove redundant TARGET_32BIT from matching condition. (unaligned_loadhis): Likewise. (unaligned_loadhiu): Likewise. (unaligned_storesi): Likewise. (unaligned_storehi): Likewise. 2015-11-11 Kyrylo Tkachov * gcc.target/arm/armv6-unaligned-load-ice.c: New test.
Re: POWERPC64_TOC_POINTER_ALIGNMENT
On Tue, Nov 17, 2015 at 9:32 PM, Alan Modra wrote:
> On Tue, Nov 17, 2015 at 07:53:18PM -0500, Michael Meissner wrote:
>> Here is the temporary patch I'm using to get past rs6000.c. But I suspect
>> the
>> TOC alignment should never be 256.
>
> Yes, it should be. Recent GNU ld aligns .TOC. to a 256 byte boundary.
> I have this patch in my tree.
>
> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index abc8eaa..e3ec042 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -8059,12 +8059,17 @@ rs6000_cannot_force_const_mem (machine_mode mode
> ATTRIBUTE_UNUSED, rtx x)
> static bool
> use_toc_relative_ref (rtx sym, machine_mode mode)
> {
> + /* Silence complaint that the POWERPC64_TOC_POINTER_ALIGNMENT test
> + is always true. */
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wtype-limits"
>return ((constant_pool_expr_p (sym)
>&& ASM_OUTPUT_SPECIAL_POOL_ENTRY_P (get_pool_constant (sym),
>get_pool_mode (sym)))
> || (TARGET_CMODEL == CMODEL_MEDIUM
> && SYMBOL_REF_LOCAL_P (sym)
> && GET_MODE_SIZE (mode) <= POWERPC64_TOC_POINTER_ALIGNMENT));
> +#pragma GCC diagnostic pop
> }
>
> /* Our implementation of LEGITIMIZE_RELOAD_ADDRESS. Returns a value to
I have applied Alan's work-around for now until there is a more robust solution.
Thanks, David
Re: [patch] Fix PR middle-end/65958
> Ah. I think the patch is good and we should reflect this in the > documentation. OK, will do, thanks. -- Eric Botcazou
Re: [PATCH][ARM] PR 68149 Fix ICE in unaligned_loaddi split
Ping. https://gcc.gnu.org/ml/gcc-patches/2015-11/msg01253.html Thanks, Kyrill On 10/11/15 17:32, Kyrill Tkachov wrote: Hi all, This ICE in this PR occurs when we're trying to split unaligned_loaddi into two SImode unaligned loads. The problem is in the addressing mode. When reload was picking the addressing mode we accepted an offset of -256 because the mode in the pattern is advertised as DImode and that was accepted by the legitimate address hooks because they thought it was a NEON load (DImode is in VALID_NEON_DREG_MODE). However, the splitter wants to generate two normal SImode unaligned loads using that address, for which -256 is not valid, so we ICE in gen_lowpart. The only way unaligned_loaddi could be generated was through the gen_movmem_ldrd_strd expansion that implements a memmove using LDRD and STRD sequences. If the memmove source is not aligned we can't use LDRDs so the code generates unaligned_loaddi patterns and expects them to be split into two normal loads after reload. Similarly for unaligned store destinations. This patch just explicitly generates the two unaligned SImode loads or stores when appropriate inside gen_movmem_ldrd_strd. This makes the unaligned_loaddi and unaligned_storedi patterns unused, so we can remove them. This patch fixes the ICe in gcc.target/aarch64/advsimd-intrinsics/vldX.c seen with -mthumb -mcpu=cortex-a15 -mfpu=neon-vfpv4 -mfloat-abi=hard -mfp16-format=ieee so no new testcase is added. Bootstrapped and tested on arm-none-linux-gnueabihf. Ok for trunk? Thanks, Kyrill 2015-11-10 Kyrylo Tkachov PR target/68149 * config/arm/arm.md (unaligned_loaddi): Delete. (unaligned_storedi): Likewise. * config/arm/arm.c (gen_movmem_ldrd_strd): Don't generate unaligned DImode memory ops. Instead perform two back-to-back unalgined SImode ops.
Re: [patch] Fix PR middle-end/65958
On Wed, Nov 18, 2015 at 3:28 PM, Eric Botcazou wrote: >> Looks good to me. I also found the Arrays of Variable Length section >> in extend.texi which also refers to alloca as doing the same. We may >> want to add a note there that you should not mix both and that only >> VLAs (when not mixed with alloca) are freed at scope boundary. > > It's already there and in fact the current behavior is documented: > > There are other differences between these two methods. Space allocated > with `alloca' exists until the containing _function_ returns. The > space for a variable-length array is deallocated as soon as the array > name's scope ends. (If you use both variable-length arrays and > `alloca' in the same function, deallocation of a variable-length array > also deallocates anything more recently allocated with `alloca'.) > > so we need to amend the documentation if we go for the patch. Ah. I think the patch is good and we should reflect this in the documentation. Richard. > -- > Eric Botcazou
Re: libgomp: Compile-time error for non-portable gomp_mutex_t initialization
On Fri, Sep 25, 2015 at 17:28:25 +0200, Jakub Jelinek wrote:
> On Fri, Sep 25, 2015 at 05:04:47PM +0200, Thomas Schwinge wrote:
> > On Thu, 26 Mar 2015 23:41:30 +0300, Ilya Verbin wrote:
> > > On Thu, Mar 26, 2015 at 13:09:19 +0100, Jakub Jelinek wrote:
> > > > the current code is majorly broken. As I've said earlier, e.g. the lack
> > > > of mutex guarding gomp_target_init (which is using pthread_once
> > > > guaranteed
> > > > to be run just once) vs. concurrent GOMP_offload_register calls
> > > > (if those are run from ctors, then I guess something like dl_load_lock
> > > > ensures at least on glibc that multiple GOMP_offload_register calls
> > > > aren't
> > > > performed at the same time) in accessing/reallocating offload_images
> > > > and num_offload_images and the lack of support to register further
> > > > images after the gomp_target_init call (if you dlopen further shared
> > > > libraries) is really bad. And it would be really nice to support the
> > > > unloading.
> >
> > > Here is the latest patch for libgomp and mic plugin.
> >
> > > libgomp/
> >
> > > * target.c (register_lock): New mutex for offload image registration.
> >
> > > (GOMP_offload_register): Add mutex lock.
>
> That is definitely wrong. You'd totally break --disable-linux-futex support
> on linux and bootstrap on e.g. Solaris and various other pthread targets.
I don't quite understand, do you mean that gcc 5 and trunk are broken, because
register_lock doesn't have initialization? But it seems that bootstrap on
Solaris and other targets works fine...
> At least for ELF and dynamic linking, shared libraries that contain
> constructors that call GOMP_offload_register* should have DT_NEEDED libgomp
> and thus libgomp's constructors should be run before the constructors of
> the libraries that call GOMP_offload_register*.
So, libgomp should contain a constructor, which will call gomp_mutex_init
(®ister_lock) before any call to GOMP_offload_register*, right?
> For the targets without known zero initializer for gomp_mutex_lock, either
> there is an option to use pthread_once to make sure it is initialized once,
> or there is an option to define a macro like GOMP_MUTEX_INITIALIZER,
> defined to PTHREAD_MUTEX_INITIALIZER in config/posix/mutex.h and to
> { 0 } in config/linux/mutex.h and something like {} or whatever in
> config/rtems/mutex.h. Then for the non-automatic non-heap
> gomp_mutex_t's you could just initialize them in their initializers
> with GOMP_MUTEX_INITIALIZER.
-- Ilya
Re: [gomp4] OpenACC async clause regressions
On 22/10/15 20:27, Thomas Schwinge wrote:
diff --cc libgomp/testsuite/libgomp.oacc-c-c++-common/asyncwait-1.c
index d478ce2,22cef6d..f3b490a
--- libgomp/testsuite/libgomp.oacc-c-c++-common/asyncwait-1.c
+++ libgomp/testsuite/libgomp.oacc-c-c++-common/asyncwait-1.c
@@@ -1,4 -1,4 +1,6 @@@
/* { dg-do run { target openacc_nvidia_accel_selected } } */
++/*.
++ { dg-xfail-run-if "TODO" { *-*-* } } */
/* { dg-additional-options "-lcuda" } */
#include
This failure shows up on trunk. Should it also be xfailed there?
Thanks,
- Tom
teach delay folding in c++ about OACC_LOOPs
Jason,
Your recent delay folding patch broke libgomp.oacc-c++/loop-auto-1.c. It
looks like you forgot to handle OACC_LOOP in cp_fold_r. You probably
didn't notice this because Nathan committed his auto acc loop patch just
before you applied your patch. I'm not sure why only that test is
affected though.
Is this patch ok for trunk?
Cesar
2015-11-17 Cesar Philippidis
gcc/cp/
* cp-gimplify.c (cp_fold_r): Add support for OACC_LOOP.
diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index 8fe9e13..99d0cfb 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -933,7 +933,8 @@ cp_fold_r (tree *stmt_p, int *walk_subtrees, void *data)
code = TREE_CODE (stmt);
if (code == OMP_FOR || code == OMP_SIMD || code == OMP_DISTRIBUTE
- || code == OMP_TASKLOOP || code == CILK_FOR || code == CILK_SIMD)
+ || code == OMP_TASKLOOP || code == CILK_FOR || code == CILK_SIMD
+ || code == OACC_LOOP)
{
tree x;
int i, n;
Re: [0/7] Type promotion pass and elimination of zext/sext
On Wed, Nov 18, 2015 at 3:04 PM, Richard Biener
wrote:
> On Sat, Nov 14, 2015 at 2:15 AM, Kugan
> wrote:
>>
>> Attached is the latest version of the patch. With the patches
>> 0001-Add-new-SEXT_EXPR-tree-code.patch,
>> 0002-Add-type-promotion-pass.patch and
>> 0003-Optimize-ZEXT_EXPR-with-tree-vrp.patch.
>>
>> I did bootstrap on ppc64-linux-gnu, aarch64-linux-gnu and
>> x64-64-linux-gnu and regression testing on ppc64-linux-gnu,
>> aarch64-linux-gnu arm64-linux-gnu and x64-64-linux-gnu. I ran into three
>> issues in ppc64-linux-gnu regression testing. There are some other test
>> cases which needs adjustment for scanning for some patterns that are not
>> valid now.
>>
>> 1. rtl fwprop was going into infinite loop. Works with the following patch:
>> diff --git a/gcc/fwprop.c b/gcc/fwprop.c
>> index 16c7981..9cf4f43 100644
>> --- a/gcc/fwprop.c
>> +++ b/gcc/fwprop.c
>> @@ -948,6 +948,10 @@ try_fwprop_subst (df_ref use, rtx *loc, rtx
>> new_rtx, rtx_insn *def_insn,
>>int old_cost = 0;
>>bool ok;
>>
>> + /* Value to be substituted is the same, nothing to do. */
>> + if (rtx_equal_p (*loc, new_rtx))
>> +return false;
>> +
>>update_df_init (def_insn, insn);
>>
>>/* forward_propagate_subreg may be operating on an instruction with
>
> Which testcase was this on?
>
>> 2. gcc.dg/torture/ftrapv-1.c fails
>> This is because we are checking for the SImode trapping. With the
>> promotion of the operation to wider mode, this is i think expected. I
>> think the testcase needs updating.
>
> No, it is not expected. As said earlier you need to refrain from promoting
> integer operations that trap. You can use ! operation_no_trapping_overflow
> for this.
>
>> 3. gcc.dg/sms-3.c fails
>> It fails with -fmodulo-sched-allow-regmoves and OK when I remove it. I
>> am looking into it.
>>
>>
>> I also have the following issues based on the previous review (as posted
>> in the previous patch). Copying again for the review purpose.
>>
>> 1.
>>> you still call promote_ssa on both DEFs and USEs and promote_ssa looks
>>> at SSA_NAME_DEF_STMT of the passed arg. Please call promote_ssa just
>>> on DEFs and fixup_uses on USEs.
>>
>> I am doing this to promote SSA that are defined with GIMPLE_NOP. Is
>> there anyway to iterate over this. I have added gcc_assert to make sure
>> that promote_ssa is called only once.
>
> gcc_assert (!ssa_name_info_map->get_or_insert (def));
>
> with --disable-checking this will be compiled away so you need to do
> the assert in a separate statement.
>
>> 2.
>>> Instead of this you should, in promote_all_stmts, walk over all uses
>> doing what
>>> fixup_uses does and then walk over all defs, doing what promote_ssa does.
>>>
>>> +case GIMPLE_NOP:
>>> + {
>>> + if (SSA_NAME_VAR (def) == NULL)
>>> + {
>>> + /* Promote def by fixing its type for anonymous def. */
>>> + TREE_TYPE (def) = promoted_type;
>>> + }
>>> + else
>>> + {
>>> + /* Create a promoted copy of parameters. */
>>> + bb = single_succ (ENTRY_BLOCK_PTR_FOR_FN (cfun));
>>>
>>> I think the uninitialized vars are somewhat tricky and it would be best
>>> to create a new uninit anonymous SSA name for them. You can
>>> have SSA_NAME_VAR != NULL and def _not_ being a parameter
>>> btw.
>>
>> I experimented with get_or_create_default_def. Here we have to have a
>> SSA_NAME_VAR (def) of promoted type.
>>
>> In the attached patch I am doing the following and seems to work. Does
>> this looks OK?
>>
>> + }
>> + else if (TREE_CODE (SSA_NAME_VAR (def)) != PARM_DECL)
>> + {
>> + tree var = copy_node (SSA_NAME_VAR (def));
>> + TREE_TYPE (var) = promoted_type;
>> + TREE_TYPE (def) = promoted_type;
>> + SET_SSA_NAME_VAR_OR_IDENTIFIER (def, var);
>> + }
>
> I believe this will wreck the SSA default-def map so you should do
>
> set_ssa_default_def (cfun, SSA_NAME_VAR (def), NULL_TREE);
> tree var = create_tmp_reg (promoted_type);
> TREE_TYPE (def) = promoted_type;
> SET_SSA_NAME_VAR_OR_IDENTIFIER (def, var);
> set_ssa_default_def (cfun, var, def);
>
> instead.
>
>> I prefer to promote def as otherwise iterating over the uses and
>> promoting can look complicated (have to look at all the different types
>> of stmts again and do the right thing as It was in the earlier version
>> of this before we move to this approach)
>>
>> 3)
>>> you can also transparently handle constants for the cases where promoting
>>> is required. At the moment their handling is interwinded with the def
>> promotion
>>> code. That makes the whole thing hard to follow.
>>
>>
>> I have updated the comments with:
>>
>> +/* Promote constants in STMT to TYPE. If PROMOTE_COND_EXPR is true,
>> + promote only the constants in conditions part of the COND_EXPR.
>> +
>> + We promote the constants when the associated operands are promoted.
>> + This usually means that we promote the co
Re: [PATCH] Add clang-format config to contrib folder
On 2015.11.18 at 15:37 +0100, Markus Trippelsdorf wrote:
> On 2015.11.18 at 15:10 +0100, Martin Liška wrote:
> > Hello.
> >
> > Following patch adds a clang-format config file that should respect the GNU
> > coding standards.
> > As the file is not part of build process, I hope the patch can be applied
> > even though
> > we've just skipped to stage3? The patch adds a hunk to Makefile which can
> > create symlink
> > to the root directory of the GCC compiler. The clang-format automatically
> > loads style from
> > the configuration file.
> >
> > clang-format (version 3.8) provides rich variety of configuration options
> > that can
> > ensure the GNU coding style.
> >
> > Limitations:
> > + placement of opening brace of an initializer can't be requested
> > + sometimes, '(' is the trailing symbol at the end of a line, which can
> > look weird
> >
> > As we've been continuously converting our source base to C++, the
> > clang-format should
> > provide better results than a collection of regular expressions
> > (check_GNU_style.sh).
> >
> > As a reference file I attach gcc/tree-ssa-uninit.c file.
> > Feel free to comment the suggested configuration file.
>
> Thanks for doing this. It works fine except one problem: You should
> delete "SortIncludes: false" from the contrib/clang-format file, because
> it is the default for custom styles and doesn't work in .clang-format
> (it is a command line option instead):
>
> markus@x4 gcc % clang-format -style=file gcc-main.c
> YAML:47:15: error: unknown key 'SortIncludes'
Ah, the option was added only yesterday to clang-format!
My version wasn't new enough...
--
Markus
Re: [PATCH] Add clang-format config to contrib folder
On 2015.11.18 at 15:10 +0100, Martin Liška wrote:
> Hello.
>
> Following patch adds a clang-format config file that should respect the GNU
> coding standards.
> As the file is not part of build process, I hope the patch can be applied
> even though
> we've just skipped to stage3? The patch adds a hunk to Makefile which can
> create symlink
> to the root directory of the GCC compiler. The clang-format automatically
> loads style from
> the configuration file.
>
> clang-format (version 3.8) provides rich variety of configuration options
> that can
> ensure the GNU coding style.
>
> Limitations:
> + placement of opening brace of an initializer can't be requested
> + sometimes, '(' is the trailing symbol at the end of a line, which can look
> weird
>
> As we've been continuously converting our source base to C++, the
> clang-format should
> provide better results than a collection of regular expressions
> (check_GNU_style.sh).
>
> As a reference file I attach gcc/tree-ssa-uninit.c file.
> Feel free to comment the suggested configuration file.
Thanks for doing this. It works fine except one problem: You should
delete "SortIncludes: false" from the contrib/clang-format file, because
it is the default for custom styles and doesn't work in .clang-format
(it is a command line option instead):
markus@x4 gcc % clang-format -style=file gcc-main.c
YAML:47:15: error: unknown key 'SortIncludes'
Otherwise it looks good to me.
--
Markus
[PATCH] S/390: Clobber r1 in patterns resulting in pfpo instruction.
The attached patch fixes the S/390 patterns using the "pfpo"
instruction in s390.md. The instructions clobber r1, but the
patterns did not reflect that.
Ciao
Dominik ^_^ ^_^
--
Dominik Vogt
IBM Germany
gcc/ChangeLog
* config/s390/s390.md (GPR1_REGNUM): New constant.
("*trunc2")
("*trunc2")
("trunc2")
("trunc2")
("*extend2")
("*extend2")
("extend2")
("extend2"): Clobber r1.
gcc/testsuite/ChangeLog
* gcc.target/s390/pfpo.c: New test.
>From c7ddb09c08d4d3e4d394588aa8aa43331582db86 Mon Sep 17 00:00:00 2001
From: Dominik Vogt
Date: Tue, 17 Nov 2015 20:10:04 +0100
Subject: [PATCH] S/390: Clobber r1 in patterns resulting in pfpo
instruction.
---
gcc/config/s390/s390.md | 25 +
gcc/testsuite/gcc.target/s390/pfpo.c | 21 +
2 files changed, 38 insertions(+), 8 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/s390/pfpo.c
diff --git a/gcc/config/s390/s390.md b/gcc/config/s390/s390.md
index ea65c74..f2bb24c 100644
--- a/gcc/config/s390/s390.md
+++ b/gcc/config/s390/s390.md
@@ -301,6 +301,7 @@
[
; General purpose registers
(GPR0_REGNUM 0)
+ (GPR1_REGNUM 1)
; Floating point registers.
(FPR0_REGNUM 16)
(FPR1_REGNUM 20)
@@ -4895,7 +4896,8 @@
[(set (reg:DFP_ALL FPR0_REGNUM)
(float_truncate:DFP_ALL (reg:BFP FPR4_REGNUM)))
(use (reg:SI GPR0_REGNUM))
- (clobber (reg:CC CC_REGNUM))]
+ (clobber (reg:CC CC_REGNUM))
+ (clobber (reg:SI GPR1_REGNUM))]
"TARGET_HARD_DFP"
"pfpo")
@@ -4903,7 +4905,8 @@
[(set (reg:BFP FPR0_REGNUM)
(float_truncate:BFP (reg:DFP_ALL FPR4_REGNUM)))
(use (reg:SI GPR0_REGNUM))
- (clobber (reg:CC CC_REGNUM))]
+ (clobber (reg:CC CC_REGNUM))
+ (clobber (reg:SI GPR1_REGNUM))]
"TARGET_HARD_DFP"
"pfpo")
@@ -4914,7 +4917,8 @@
[(set (reg:DFP_ALL FPR0_REGNUM)
(float_truncate:DFP_ALL (reg:BFP FPR4_REGNUM)))
(use (reg:SI GPR0_REGNUM))
- (clobber (reg:CC CC_REGNUM))])
+ (clobber (reg:CC CC_REGNUM))
+ (clobber (reg:SI GPR1_REGNUM))])
(set (match_operand:DFP_ALL 0 "nonimmediate_operand" "")
(reg:DFP_ALL FPR0_REGNUM))]
"TARGET_HARD_DFP
@@ -4936,7 +4940,8 @@
(parallel
[(set (reg:BFP FPR0_REGNUM) (float_truncate:BFP (reg:DFP_ALL FPR4_REGNUM)))
(use (reg:SI GPR0_REGNUM))
- (clobber (reg:CC CC_REGNUM))])
+ (clobber (reg:CC CC_REGNUM))
+ (clobber (reg:SI GPR1_REGNUM))])
(set (match_operand:BFP 0 "nonimmediate_operand" "") (reg:BFP FPR0_REGNUM))]
"TARGET_HARD_DFP
&& GET_MODE_SIZE (mode) >= GET_MODE_SIZE (mode)"
@@ -4957,14 +4962,16 @@
(define_insn "*extend2"
[(set (reg:DFP_ALL FPR0_REGNUM) (float_extend:DFP_ALL (reg:BFP FPR4_REGNUM)))
(use (reg:SI GPR0_REGNUM))
- (clobber (reg:CC CC_REGNUM))]
+ (clobber (reg:CC CC_REGNUM))
+ (clobber (reg:SI GPR1_REGNUM))]
"TARGET_HARD_DFP"
"pfpo")
(define_insn "*extend2"
[(set (reg:BFP FPR0_REGNUM) (float_extend:BFP (reg:DFP_ALL FPR4_REGNUM)))
(use (reg:SI GPR0_REGNUM))
- (clobber (reg:CC CC_REGNUM))]
+ (clobber (reg:CC CC_REGNUM))
+ (clobber (reg:SI GPR1_REGNUM))]
"TARGET_HARD_DFP"
"pfpo")
@@ -4975,7 +4982,8 @@
[(set (reg:DFP_ALL FPR0_REGNUM)
(float_extend:DFP_ALL (reg:BFP FPR4_REGNUM)))
(use (reg:SI GPR0_REGNUM))
- (clobber (reg:CC CC_REGNUM))])
+ (clobber (reg:CC CC_REGNUM))
+ (clobber (reg:SI GPR1_REGNUM))])
(set (match_operand:DFP_ALL 0 "nonimmediate_operand" "")
(reg:DFP_ALL FPR0_REGNUM))]
"TARGET_HARD_DFP
@@ -4997,7 +5005,8 @@
(parallel
[(set (reg:BFP FPR0_REGNUM) (float_extend:BFP (reg:DFP_ALL FPR4_REGNUM)))
(use (reg:SI GPR0_REGNUM))
- (clobber (reg:CC CC_REGNUM))])
+ (clobber (reg:CC CC_REGNUM))
+ (clobber (reg:SI GPR1_REGNUM))])
(set (match_operand:BFP 0 "nonimmediate_operand" "") (reg:BFP FPR0_REGNUM))]
"TARGET_HARD_DFP
&& GET_MODE_SIZE (mode) < GET_MODE_SIZE (mode)"
diff --git a/gcc/testsuite/gcc.target/s390/pfpo.c b/gcc/testsuite/gcc.target/s390/pfpo.c
new file mode 100644
index 000..ff8b09a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/pfpo.c
@@ -0,0 +1,21 @@
+/* The pfpo instruction generated by this code clobbers the r1 register while
+ it was still in use. */
+
+/* { dg-do run } */
+/* { dg-options "-O0 -march=z10" } */
+
+int foo(int x)
+{
+ return x;
+}
+
+int bar(int i, float f)
+{
+ return i;
+}
+
+int main()
+{
+ _Decimal32 d = 7;
+ return bar(foo(0x10203040), (float)d) == 0x10203040 ? 0 : 1;
+}
--
2.3.0
[committed] PR 68406: Avoid problem with older host compilers
Older compilers require the Key typedef to be expanded for the traversal
function templates. This patch uses the same approach as we did for
hash_map.
Tested on x86_64-linux-gnu and applied as obvious (given the previous
hash_map patch).
Sorry for the breakage.
Richard
gcc/
PR bootstrap/68406
* hash-set.h (hash_set::traverse): Expand Key typedef.
diff --git a/gcc/hash-set.h b/gcc/hash-set.h
index 679d2b6..8a7c9a0 100644
--- a/gcc/hash-set.h
+++ b/gcc/hash-set.h
@@ -68,7 +68,7 @@ public:
/* Call the call back on each pair of key and value with the passed in
arg. */
- template
+ template
void traverse (Arg a) const
{
for (typename hash_table::iterator iter = m_table.begin ();
Re: [patch] Fix PR middle-end/65958
> Looks good to me. I also found the Arrays of Variable Length section > in extend.texi which also refers to alloca as doing the same. We may > want to add a note there that you should not mix both and that only > VLAs (when not mixed with alloca) are freed at scope boundary. It's already there and in fact the current behavior is documented: There are other differences between these two methods. Space allocated with `alloca' exists until the containing _function_ returns. The space for a variable-length array is deallocated as soon as the array name's scope ends. (If you use both variable-length arrays and `alloca' in the same function, deallocation of a variable-length array also deallocates anything more recently allocated with `alloca'.) so we need to amend the documentation if we go for the patch. -- Eric Botcazou
RE: [Patch,tree-optimization]: Add new path Splitting pass on tree ssa representation
-Original Message- From: Tom de Vries [mailto:tom_devr...@mentor.com] Sent: Wednesday, November 18, 2015 1:14 PM To: Jeff Law; Richard Biener Cc: Ajit Kumar Agarwal; GCC Patches; Vinod Kathail; Shail Aditya Gupta; Vidhumouli Hunsigida; Nagaraju Mekala Subject: Re: [Patch,tree-optimization]: Add new path Splitting pass on tree ssa representation On 14/11/15 00:35, Jeff Law wrote: > Anyway, bootstrapped and regression tested on x86_64-linux-gnu. > Installed on the trunk. > [Patch,tree-optimization]: Add new path Splitting pass on tree ssa > representation > > * Makefile.in (OBJS): Add gimple-ssa-split-paths.o > * common.opt (-fsplit-paths): New flag controlling path splitting. > * doc/invoke.texi (fsplit-paths): Document. > * opts.c (default_options_table): Add -fsplit-paths to -O2. > * passes.def: Add split_paths pass. > * timevar.def (TV_SPLIT_PATHS): New timevar. > * tracer.c: Include "tracer.h" > (ignore_bb_p): No longer static. > (transform_duplicate): New function, broken out of tail_duplicate. > (tail_duplicate): Use transform_duplicate. > * tracer.h (ignore_bb_p): Declare > (transform_duplicate): Likewise. > * tree-pass.h (make_pass_split_paths): Declare. > * gimple-ssa-split-paths.c: New file. > > * gcc.dg/tree-ssa/split-path-1.c: New test. >>I've filed PR68402 - FAIL: gcc.dg/tree-ssa/split-path-1.c execution test with >>-m32. I have fixed the above PR and the patch is submitted. https://gcc.gnu.org/ml/gcc-patches/2015-11/msg02217.html Thanks & Regards Ajit Thanks, - Tom
Re: [PATCH] Fix memory leaks in tree-ssa-uninit.c
On 11/13/2015 08:19 PM, Jeff Law wrote:
> On 11/13/2015 09:58 AM, Martin Liška wrote:
>> On 11/13/2015 05:32 PM, Jeff Law wrote:
>>> On 11/13/2015 05:50 AM, Martin Liška wrote:
Hello.
Patch survives regbootstrap on x86_64-linux-gnu.
Ready for trunk?
Thanks,
Martin
0001-Fix-memory-leaks-in-tree-ssa-uninit.c.patch
From 54851503251dee7a8bd074485db262715e628728 Mon Sep 17 00:00:00 2001
From: marxin
Date: Fri, 13 Nov 2015 12:23:22 +0100
Subject: [PATCH] Fix memory leaks in tree-ssa-uninit.c
gcc/ChangeLog:
2015-11-13 Martin Liska
* tree-ssa-uninit.c (convert_control_dep_chain_into_preds):
Fix GNU coding style.
(find_def_preds): Use auto_vec.
(destroy_predicate_vecs): Change signature of the function.
(prune_uninit_phi_opnds_in_unrealizable_paths): Use the
new signature.
(simplify_preds_4): Use destroy_predicate_vecs instread of
just releasing preds vector.
(normalize_preds): Likewise.
(is_use_properly_guarded): Use new signature of
destroy_predicate_vecs.
(find_uninit_use): Likewise.
>>> OK.
>>>
>>> FWIW, there's all kinds of spaces vs tabs issues in this file. I'm curious
>>> why you chose to fix convert_control_dep_chain_into_preds, but didn't
>>> change any others.
>>
>> Hi Jeff.
>>
>> Thanks for confirmation, you are right, it's full of coding style issues. I
>> can change these if it would be desired?
> It's probably a good thing to do. Given it'd strictly be formatting, I'd
> even consider it post-stage1.
>
> jeff
Hello.
I'm sending the re-formatted source file.
The source file was formatted by clang-format ([1]) and some hunk were manually
re-formatted.
No change in behavior.
Patch can regbootstrap on x86_64-linux-gnu.
Ready for trunk?
Thanks,
Martin
[1] https://gcc.gnu.org/ml/gcc-patches/2015-11/msg02214.html
>From bef9e38d9c63868ee569b1c3da253b8baa523eb0 Mon Sep 17 00:00:00 2001
From: marxin
Date: Wed, 18 Nov 2015 10:20:11 +0100
Subject: [PATCH 1/2] Reformat tree-ssa-uninit.c
---
gcc/tree-ssa-uninit.c | 1391 -
1 file changed, 678 insertions(+), 713 deletions(-)
diff --git a/gcc/tree-ssa-uninit.c b/gcc/tree-ssa-uninit.c
index 0709cce..fcdb774 100644
--- a/gcc/tree-ssa-uninit.c
+++ b/gcc/tree-ssa-uninit.c
@@ -35,16 +35,15 @@ along with GCC; see the file COPYING3. If not see
#include "tree-cfg.h"
/* This implements the pass that does predicate aware warning on uses of
- possibly uninitialized variables. The pass first collects the set of
- possibly uninitialized SSA names. For each such name, it walks through
- all its immediate uses. For each immediate use, it rebuilds the condition
- expression (the predicate) that guards the use. The predicate is then
+ possibly uninitialized variables. The pass first collects the set of
+ possibly uninitialized SSA names. For each such name, it walks through
+ all its immediate uses. For each immediate use, it rebuilds the condition
+ expression (the predicate) that guards the use. The predicate is then
examined to see if the variable is always defined under that same condition.
This is done either by pruning the unrealizable paths that lead to the
default definitions or by checking if the predicate set that guards the
defining paths is a superset of the use predicate. */
-
/* Pointer set of potentially undefined ssa names, i.e.,
ssa names that are defined by phi with operands that
are not defined or potentially undefined. */
@@ -56,7 +55,7 @@ static hash_set *possibly_undefined_names = 0;
#define MASK_EMPTY(mask) (mask == 0)
/* Returns the first bit position (starting from LSB)
- in mask that is non zero. Returns -1 if the mask is empty. */
+ in mask that is non zero. Returns -1 if the mask is empty. */
static int
get_mask_first_set_bit (unsigned mask)
{
@@ -75,18 +74,17 @@ get_mask_first_set_bit (unsigned mask)
static bool
has_undefined_value_p (tree t)
{
- return (ssa_undefined_value_p (t)
- || (possibly_undefined_names
- && possibly_undefined_names->contains (t)));
+ return ssa_undefined_value_p (t)
+ || (possibly_undefined_names
+ && possibly_undefined_names->contains (t));
}
-
-
/* Like has_undefined_value_p, but don't return true if TREE_NO_WARNING
is set on SSA_NAME_VAR. */
static inline bool
-uninit_undefined_value_p (tree t) {
+uninit_undefined_value_p (tree t)
+{
if (!has_undefined_value_p (t))
return false;
if (SSA_NAME_VAR (t) && TREE_NO_WARNING (SSA_NAME_VAR (t)))
@@ -112,13 +110,13 @@ uninit_undefined_value_p (tree t) {
/* Emit a warning for EXPR based on variable VAR at the point in the
program T, an SSA_NAME, is used being uninitialized. The exact
warning text is in MSGID and DATA is the gimple stmt with info about
- the location
RE: [Patch,testsuite]: Fix the tree-ssa/split-path-1.c testcase
Hello Jeff: Please ignore my previous mails as they bounced back. Sorry for that. I have fixed the problem with the testcase. The splitting path optimization remains intact. Attached is the patch. The problem was related to the testcase as the loop bound goes beyond the malloced array. There was also a problem with accessing the elements of EritePtr. ChangeLog: 2015-11-18 Ajit Agarwal * gcc.dg/tree-ssa/split-path-1.c: Fix the testcase. Signed-off-by:Ajit Agarwal ajit...@xilinx.com Thanks & Regards Ajit testcase.patch Description: testcase.patch
RFC/RFA: Fix bug with REE optimization corrupting extended registers
Hi Guys,
I recently discovered a bug in the current Redundant Extension
Elimination optimization. If the candidate extension instruction
increases the number of hard registers used, the pass does not check
to see if these extra registers are live between the definition and
the extension.
For example:
(insn 44 (set (reg:QI r11) (mem:QI (reg:HI r20)))
(insn 45 (set (reg:QI r10) (mem:QI (reg:HI r18)))
[...]
(insn 71 (set (reg:HI r14) (zero_extend:HI (reg:QI r11)))
[...]
(insn 88 (set (reg:HI r10) (zero_extend:HI (reg:QI r10)))
(This is on the RL78 target where HImode values occupy two hard
registers and QImode values only one. The bug however is generic, not
RL78 specific).
The REE pass transforms this into:
(insn 44 (set (reg:QI r11) (mem:QI (reg:HI r20)))
(insn 45 (set (reg:HI r10) (zero_extend:HI (mem:QI (reg:HI r18
[...]
(insn 71 (set (reg:HI r14) (zero_extend:HI (reg:QI r11)))
[...]
(insn 88 deleted)
Note how the new set at insn 45 clobbers the value loaded by insn 44
into r11.
The patch below is my attempt to fix this. It is not correct
however. I could not work out how to determine if a given hard
register is live at any point between two insns. So instead I used
the liveness information associated with the basic block containing
the definition. If one of the extended registers is live or becomes
live in this block, and this block is not the same block as the one
containing the extension insn, then I stop the optimization. This
works for the RL78 for all of the cases that I could find.
So ... comments please ?
Will gcc ever generate a single basic block that contains both the
definition and the extension instructions, but also where the extra
hard registers are used for another purpose as well ?
Tested with no regressions (or fixes) on an x86-pc-linux-gnu target.
Also tested with no regression and 7 fixes on an rl78-elf target.
Cheers
Nick
gcc/ChangeLog
2015-11-18 Nick Clifton
* ree.c (regs_live_between): New function.
(add_removable_extension): If the extension uses more hard
registers than the definition then check that the extra hard
registers are not live between the definition and the
extension.
Index: gcc/ree.c
===
--- gcc/ree.c (revision 230517)
+++ gcc/ree.c (working copy)
@@ -952,6 +952,49 @@
return false;
}
+/* Returns TRUE if any of the registers [start, stop) are live between DEF and
up
+ to, but not including, INSN. */
+
+static bool
+regs_live_between (rtx_insn * insn,
+ struct df_link * def,
+ unsigned int start,
+ unsigned int stop)
+{
+ basic_block bb;
+
+ /* FIXME: This is an incomplete test. It only checks the DEF and USE states
of the
+ registers in basic block of DEF. If any of them match the start-stop
range and
+ the INSN is not in the same block, then it is assumed that the registers
must be
+ live. */
+ /* These first few checks are just paranoia... */
+ if (def != NULL
+ && def->ref != NULL
+ && def->ref->base.insn_info != NULL
+ && def->ref->base.insn_info->insn != NULL
+ && (bb = BLOCK_FOR_INSN (def->ref->base.insn_info->insn)) != NULL
+ && bb != BLOCK_FOR_INSN (insn))
+{
+ struct df_lr_bb_info * bb_info = df_lr_get_bb_info (bb->index);
+ bitmap_iterator bi;
+ unsigned int reg;
+
+ EXECUTE_IF_SET_IN_BITMAP (& bb_info->def, 0, reg, bi)
+ {
+ if (reg >= start && reg < stop)
+ return true;
+ }
+
+ EXECUTE_IF_SET_IN_BITMAP (& bb_info->out, 0, reg, bi)
+ {
+ if (reg >= start && reg < stop)
+ return true;
+ }
+}
+
+ return false;
+}
+
/* Add an extension pattern that could be eliminated. */
static void
@@ -1080,6 +1123,30 @@
}
}
+ /* Fourth, if the extended version occupies more registers than
+the original version then make sure that these extra registers
+are not live between the definition and the extension. */
+ if (HARD_REGNO_NREGS (REGNO (dest), mode)
+ > HARD_REGNO_NREGS (REGNO (reg), GET_MODE (reg)))
+ {
+ unsigned int start_reg = REGNO (reg) + HARD_REGNO_NREGS (REGNO (reg),
GET_MODE (reg));
+ unsigned int stop_reg = REGNO (reg) + HARD_REGNO_NREGS (REGNO
(dest), mode);
+
+ for (def = defs; def; def = def->next)
+ {
+ if (regs_live_between (insn, def, start_reg, stop_reg))
+ {
+ if (dump_file)
+ {
+ fprintf (dump_file, "Cannot eliminate extension:\n");
+ print_rtl_single (dump_file, insn);
+ fprintf (dump_file, " because extended register(s) are
already
Re: [PATCH, VECTOR ABI] Add __attribute__((__simd__)) to GCC.
Hello Andreas, Devid.
On 18 Nov 10:45, Andreas Schwab wrote:
> Kirill Yukhin writes:
>
> > diff --git a/gcc/testsuite/c-c++-common/attr-simd.c
> > b/gcc/testsuite/c-c++-common/attr-simd.c
> > new file mode 100644
> > index 000..b4eda34
> > --- /dev/null
> > +++ b/gcc/testsuite/c-c++-common/attr-simd.c
> > @@ -0,0 +1,32 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-fdump-tree-optimized" } */
> > +
> > +__attribute__((__simd__))
> > +extern
> > +int simd_attr (void)
> > +{
> > + return 0;
> > +}
> > +
> > +/* { dg-final { scan-tree-dump "simd_attr\[ \\t\]simdclone|vector"
> > "optimized" } } */
>
> On ia64:
>
> FAIL: c-c++-common/attr-simd.c -Wc++-compat scan-tree-dump optimized
> "simd_attr[ \\t]simdclone|vector"
> FAIL: c-c++-common/attr-simd.c -Wc++-compat scan-tree-dump optimized
> "simd_attr2[ \\t]simdclone|vector"
>
> $ grep simd_attr attr-simd.c.194t.optimized
> ;; Function simd_attr (simd_attr, funcdef_no=0, decl_uid=1389, cgraph_uid=0,
> symbol_order=0)
> simd_attr ()
> ;; Function simd_attr2 (simd_attr2, funcdef_no=1, decl_uid=1392,
> cgraph_uid=1, symbol_order=1)
> simd_attr2 ()
As far as vABI is supported on x86_64/i?86 only, I am going to enable mentioned
`scan-tree-dump' only
for these targets. This should cure both IA64 and Power.
Concerning attr-simd-3.c. It is known issue: PR68158.
And I believe this test should work everywhere as far as PR is resolved.
I'll put xfail into the test.
Which will lead to (in g++.log):
gcc/testsuite/c-c++-common/attr-simd-3.c:5:48: warning: '__simd__' attribute
does not apply to types\
[-Wattributes]^M
output is:
gcc/testsuite/c-c++-common/attr-simd-3.c:5:48: warning: '__simd__' attribute
does not apply to types\
[-Wattributes]^M
XFAIL: c-c++-common/attr-simd-3.c -std=gnu++98 PR68158 (test for errors, line
5)
FAIL: c-c++-common/attr-simd-3.c -std=gnu++98 (test for excess errors)
Excess errors:
gcc/testsuite/c-c++-common/attr-simd-3.c:5:48: warning: '__simd__' attribute
does not apply to types\
[-Wattributes]
Patch in the bottom.
gcc/tessuite/
* c-c++-common/attr-simd-3.c: Put xfail (PR68158) on dg-error.
* c-c++-common/attr-simd.c: Limit scan of dump to x86_64/i?86.
>
> Andreas.
>
> --
> Andreas Schwab, SUSE Labs, sch...@suse.de
> GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
> "And now for something completely different."
diff --git a/gcc/testsuite/c-c++-common/attr-simd-3.c
b/gcc/testsuite/c-c++-common/attr-simd-3.c
index 2bbdf04..35dd4c0 100644
--- a/gcc/testsuite/c-c++-common/attr-simd-3.c
+++ b/gcc/testsuite/c-c++-common/attr-simd-3.c
@@ -2,4 +2,4 @@
/* { dg-options "-fcilkplus" } */
/* { dg-prune-output "undeclared here \\(not in a function\\)|\[^\n\r\]* was
not declared in this scope" } */
-void f () __attribute__((__simd__, __vector__)); /* { dg-error "in the same
function marked as a Cilk Plus" } */
+void f () __attribute__((__simd__, __vector__)); /* { dg-error "in the same
function marked as a Cilk Plus" "PR68158" { xfail c++ } } */
diff --git a/gcc/testsuite/c-c++-common/attr-simd.c
b/gcc/testsuite/c-c++-common/attr-simd.c
index 61974e3..7674588 100644
--- a/gcc/testsuite/c-c++-common/attr-simd.c
+++ b/gcc/testsuite/c-c++-common/attr-simd.c
@@ -11,7 +11,7 @@ int simd_attr (void)
return 0;
}
-/* { dg-final { scan-tree-dump "simd_attr\[ \\t\]simdclone|vector" "optimized"
} } */
+/* { dg-final { scan-tree-dump "simd_attr\[ \\t\]simdclone|vector" "optimized"
{ target { i?86-*-* x86_64-*-* } } } } */
/* { dg-final { scan-assembler-times "_ZGVbN4_simd_attr:" 1 { target {
i?86-*-* x86_64-*-* } } } } */
/* { dg-final { scan-assembler-times "_ZGVbM4_simd_attr:" 1 { target {
i?86-*-* x86_64-*-* } } } } */
/* { dg-final { scan-assembler-times "_ZGVcN4_simd_attr:" 1 { target {
i?86-*-* x86_64-*-* } } } } */
@@ -29,7 +29,7 @@ int simd_attr2 (void)
return 0;
}
-/* { dg-final { scan-tree-dump "simd_attr2\[ \\t\]simdclone|vector"
"optimized" } } */
+/* { dg-final { scan-tree-dump "simd_attr2\[ \\t\]simdclone|vector"
"optimized" { target { i?86-*-* x86_64-*-* } } } } */
/* { dg-final { scan-assembler-times "_ZGVbN4_simd_attr2:" 1 { target {
i?86-*-* x86_64-*-* } } } } */
/* { dg-final { scan-assembler-times "_ZGVbM4_simd_attr2:" 1 { target {
i?86-*-* x86_64-*-* } } } } */
/* { dg-final { scan-assembler-times "_ZGVcN4_simd_attr2:" 1 { target {
i?86-*-* x86_64-*-* } } } } */
[PATCH] Add clang-format config to contrib folder
Hello.
Following patch adds a clang-format config file that should respect the GNU
coding standards.
As the file is not part of build process, I hope the patch can be applied even
though
we've just skipped to stage3? The patch adds a hunk to Makefile which can
create symlink
to the root directory of the GCC compiler. The clang-format automatically loads
style from
the configuration file.
clang-format (version 3.8) provides rich variety of configuration options that
can
ensure the GNU coding style.
Limitations:
+ placement of opening brace of an initializer can't be requested
+ sometimes, '(' is the trailing symbol at the end of a line, which can look
weird
As we've been continuously converting our source base to C++, the clang-format
should
provide better results than a collection of regular expressions
(check_GNU_style.sh).
As a reference file I attach gcc/tree-ssa-uninit.c file.
Feel free to comment the suggested configuration file.
Thanks,
Martin
From 5f5eab4f9bbe76ec4d91f80dd3fb6909abd39695 Mon Sep 17 00:00:00 2001
From: marxin
Date: Wed, 18 Nov 2015 14:40:58 +0100
Subject: [PATCH] Add clang-format config to contrib folder
ChangeLog:
2015-11-18 Martin Liska
* Makefile.in: Add clang-format.
* Makefile.tpl: Likewise.
contrib/ChangeLog:
2015-11-18 Martin Liska
* clang-format: New file.
---
Makefile.in | 9 +
Makefile.tpl | 9 +
contrib/clang-format | 51 +++
3 files changed, 69 insertions(+)
create mode 100644 contrib/clang-format
diff --git a/Makefile.in b/Makefile.in
index bc2bae6..75caafd 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -2461,6 +2461,15 @@ vimrc: $(srcdir)/.local.vimrc $(srcdir)/.lvimrc
.PHONY: vimrc
+# clang-format config
+
+$(srcdir)/.clang-format:
+ $(LN_S) contrib/clang-format $@
+
+clang-format: $(srcdir)/.clang-format
+
+.PHONY: clang-format
+
# Installation targets.
.PHONY: install uninstall
diff --git a/Makefile.tpl b/Makefile.tpl
index 660e1e4..beabadc 100644
--- a/Makefile.tpl
+++ b/Makefile.tpl
@@ -889,6 +889,15 @@ vimrc: $(srcdir)/.local.vimrc $(srcdir)/.lvimrc
.PHONY: vimrc
+# clang-format config
+
+$(srcdir)/.clang-format:
+ $(LN_S) contrib/clang-format $@
+
+clang-format: $(srcdir)/.clang-format
+
+.PHONY: clang-format
+
# Installation targets.
.PHONY: install uninstall
diff --git a/contrib/clang-format b/contrib/clang-format
new file mode 100644
index 000..2d358ec
--- /dev/null
+++ b/contrib/clang-format
@@ -0,0 +1,51 @@
+# Copyright (C) 2015 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see
Re: [0/7] Type promotion pass and elimination of zext/sext
On Sat, Nov 14, 2015 at 2:15 AM, Kugan
wrote:
>
> Attached is the latest version of the patch. With the patches
> 0001-Add-new-SEXT_EXPR-tree-code.patch,
> 0002-Add-type-promotion-pass.patch and
> 0003-Optimize-ZEXT_EXPR-with-tree-vrp.patch.
>
> I did bootstrap on ppc64-linux-gnu, aarch64-linux-gnu and
> x64-64-linux-gnu and regression testing on ppc64-linux-gnu,
> aarch64-linux-gnu arm64-linux-gnu and x64-64-linux-gnu. I ran into three
> issues in ppc64-linux-gnu regression testing. There are some other test
> cases which needs adjustment for scanning for some patterns that are not
> valid now.
>
> 1. rtl fwprop was going into infinite loop. Works with the following patch:
> diff --git a/gcc/fwprop.c b/gcc/fwprop.c
> index 16c7981..9cf4f43 100644
> --- a/gcc/fwprop.c
> +++ b/gcc/fwprop.c
> @@ -948,6 +948,10 @@ try_fwprop_subst (df_ref use, rtx *loc, rtx
> new_rtx, rtx_insn *def_insn,
>int old_cost = 0;
>bool ok;
>
> + /* Value to be substituted is the same, nothing to do. */
> + if (rtx_equal_p (*loc, new_rtx))
> +return false;
> +
>update_df_init (def_insn, insn);
>
>/* forward_propagate_subreg may be operating on an instruction with
Which testcase was this on?
> 2. gcc.dg/torture/ftrapv-1.c fails
> This is because we are checking for the SImode trapping. With the
> promotion of the operation to wider mode, this is i think expected. I
> think the testcase needs updating.
No, it is not expected. As said earlier you need to refrain from promoting
integer operations that trap. You can use ! operation_no_trapping_overflow
for this.
> 3. gcc.dg/sms-3.c fails
> It fails with -fmodulo-sched-allow-regmoves and OK when I remove it. I
> am looking into it.
>
>
> I also have the following issues based on the previous review (as posted
> in the previous patch). Copying again for the review purpose.
>
> 1.
>> you still call promote_ssa on both DEFs and USEs and promote_ssa looks
>> at SSA_NAME_DEF_STMT of the passed arg. Please call promote_ssa just
>> on DEFs and fixup_uses on USEs.
>
> I am doing this to promote SSA that are defined with GIMPLE_NOP. Is
> there anyway to iterate over this. I have added gcc_assert to make sure
> that promote_ssa is called only once.
gcc_assert (!ssa_name_info_map->get_or_insert (def));
with --disable-checking this will be compiled away so you need to do
the assert in a separate statement.
> 2.
>> Instead of this you should, in promote_all_stmts, walk over all uses
> doing what
>> fixup_uses does and then walk over all defs, doing what promote_ssa does.
>>
>> +case GIMPLE_NOP:
>> + {
>> + if (SSA_NAME_VAR (def) == NULL)
>> + {
>> + /* Promote def by fixing its type for anonymous def. */
>> + TREE_TYPE (def) = promoted_type;
>> + }
>> + else
>> + {
>> + /* Create a promoted copy of parameters. */
>> + bb = single_succ (ENTRY_BLOCK_PTR_FOR_FN (cfun));
>>
>> I think the uninitialized vars are somewhat tricky and it would be best
>> to create a new uninit anonymous SSA name for them. You can
>> have SSA_NAME_VAR != NULL and def _not_ being a parameter
>> btw.
>
> I experimented with get_or_create_default_def. Here we have to have a
> SSA_NAME_VAR (def) of promoted type.
>
> In the attached patch I am doing the following and seems to work. Does
> this looks OK?
>
> + }
> + else if (TREE_CODE (SSA_NAME_VAR (def)) != PARM_DECL)
> + {
> + tree var = copy_node (SSA_NAME_VAR (def));
> + TREE_TYPE (var) = promoted_type;
> + TREE_TYPE (def) = promoted_type;
> + SET_SSA_NAME_VAR_OR_IDENTIFIER (def, var);
> + }
I believe this will wreck the SSA default-def map so you should do
set_ssa_default_def (cfun, SSA_NAME_VAR (def), NULL_TREE);
tree var = create_tmp_reg (promoted_type);
TREE_TYPE (def) = promoted_type;
SET_SSA_NAME_VAR_OR_IDENTIFIER (def, var);
set_ssa_default_def (cfun, var, def);
instead.
> I prefer to promote def as otherwise iterating over the uses and
> promoting can look complicated (have to look at all the different types
> of stmts again and do the right thing as It was in the earlier version
> of this before we move to this approach)
>
> 3)
>> you can also transparently handle constants for the cases where promoting
>> is required. At the moment their handling is interwinded with the def
> promotion
>> code. That makes the whole thing hard to follow.
>
>
> I have updated the comments with:
>
> +/* Promote constants in STMT to TYPE. If PROMOTE_COND_EXPR is true,
> + promote only the constants in conditions part of the COND_EXPR.
> +
> + We promote the constants when the associated operands are promoted.
> + This usually means that we promote the constants when we promote the
> + defining stmnts (as part of promote_ssa). However for COND_EXPR, we
> + can promote only when we promote the other operand. Therefore, this
> + is done during fixup_u
[Ada] Crahs on indexing operation with named associations
This patch fixes a crash on a variable indexing operation appearing on the left-hand side of an assignment, when the index expressions are given by parameter associations. The following must compile quietly: gcc -c -gnatct test_indexing.adb --- with Ada.Text_IO; use Ada.Text_IO; with Project; use Project; with Matrix_3x3s; use Matrix_3x3s; with Vector_3s; use Vector_3s; procedure Test_Indexing is V : Vector_3 := Create (X => 12.34, Y => 123.4, Z => 1234.0); M : Matrix_3x3 := (Create (X => V, Y => V * 2.0, Z => V * 4.0)); Idx1 : Integer := 1; type Arr_Type is array (1 .. 10) of Integer; Arr : Arr_Type; begin M (X => Idx1, Y=>1) := 20.0; M (Idx1, 2) := 20.0; M (1, 1) := 20.0; Arr (1) := 10; end Test_Indexing; --- with Project; use Project; with Project.Real_Arrays; use Project.Real_Arrays; with Vector_3s; use Vector_3s; package Matrix_3x3s is pragma Pure (Matrix_3x3s); subtype An_Axis is Integer range 1 .. 3; type Matrix_3x3 is tagged private with Constant_Indexing => Matrix_3x3s.Constant_Reference, Variable_Indexing => Matrix_3x3s.Variable_Reference; function Create (X, Y, Z : Vector_3) return Matrix_3x3; type Constant_Reference_Type (Value : not null access constant Real) is private with Implicit_Dereference => Value; function Constant_Reference (This : Matrix_3x3; X, Y : An_Axis) return Constant_Reference_Type; type Reference_Type (Value : not null access Real) is private with Implicit_Dereference => Value; function Variable_Reference (This : Matrix_3x3; X, Y : An_Axis) return Reference_Type; private type Matrix_3x3 is tagged record M : Real_Matrix (An_Axis, An_Axis); end record; function Create (X, Y, Z : Vector_3) return Matrix_3x3 is (M => (1 => (X.Get_X, X.Get_Y, X.Get_Z), 2 => (Y.Get_X, Y.Get_Y, Y.Get_Z), 3 => (Z.Get_X, Z.Get_Y, Z.Get_Z))); type Constant_Reference_Type (Value : not null access constant Real) is null record; type Reference_Type (Value : not null access Real) is null record; function Constant_Reference (This : Matrix_3x3; X, Y : An_Axis) return Constant_Reference_Type is (Value => This.M (X, Y)'Unrestricted_Access); function Variable_Reference (This : Matrix_3x3; X, Y : An_Axis) return Reference_Type is (Value => This.M (X, Y)'Unrestricted_Access); end Matrix_3x3s; --- with Ada.Numerics.Long_Real_Arrays; package Project.Real_Arrays renames Ada.Numerics.Long_Real_Arrays; package Project is pragma Pure (Project); subtype Real is Long_Float; pragma Assert (Real'Size >= 64); subtype Non_Negative_Real is Real range 0.0 .. Real'Last; subtype Positive_Real is Real range Real'Succ (0.0) .. Real'Last; end Project; --- with Project; use Project; with Project.Real_Arrays; use Project.Real_Arrays; package Vector_3s is pragma Pure (Vector_3s); subtype An_Axis is Integer range 1 .. 3; type Vector_3 is tagged private with Constant_Indexing => Vector_3s.Constant_Reference, Variable_Indexing => Vector_3s.Variable_Reference; function Create (X, Y, Z : Real) return Vector_3; function Get_X (This : Vector_3) return Real; function Get_Y (This : Vector_3) return Real; function Get_Z (This : Vector_3) return Real; function "*" (Left : Vector_3; Right : Real'Base) return Vector_3; subtype Real_Vector_3 is Real_Vector (An_Axis); type Constant_Reference_Type (Value : not null access constant Real) is private with Implicit_Dereference => Value; function Constant_Reference (This : Vector_3; Axis : An_Axis) return Constant_Reference_Type; type Reference_Type (Value : not null access Real) is private with Implicit_Dereference => Value; function Variable_Reference (This : Vector_3; Axis : An_Axis) return Reference_Type; private type Vector_3 is tagged record V : Real_Vector (An_Axis); end record; function Create (X, Y, Z : Real) return Vector_3 is (V => (1 => X, 2 => Y, 3 => Z)); function Get_X (This : Vector_3) return Real is (This.V (1)); function Get_Y (This : Vector_3) return Real is (This.V (2)); function Get_Z (This : Vector_3) return Real is (This.V (3)); function "*" (Left : Vector_3; Right : Real'Base) return Vector_3 is (V => Left.V * Right); type Constant_Reference_Type (Value : not null access constant Real) is null record; type Reference_Type (Value : not null access Real) is null record; fun
[Ada] Make System.String_Hash available through GNAT.String_Hash
System.String_Hash provides a generic hash function that can be
generally useful outside standard containers. This change make it
available for users through a new renaming package: GNAT.String_Hash.
Tested on x86_64-pc-linux-gnu, committed on trunk
2015-11-18 Pierre-Marie de Rodat
* Makefile.rtl, impunit.adb: Add g-strhas.ads.
* g-strhas.ads: New file.
* s-strhas.ads: Add a comment to redirect users to g-strhas.ads.
Index: impunit.adb
===
--- impunit.adb (revision 230522)
+++ impunit.adb (working copy)
@@ -312,6 +312,7 @@
("g-sptabo", F), -- GNAT.Spitbol.Table_Boolean
("g-sptain", F), -- GNAT.Spitbol.Table_Integer
("g-sptavs", F), -- GNAT.Spitbol.Table_Vstring
+("g-strhas", F), -- GNAT.String_Hash
("g-string", F), -- GNAT.Strings
("g-strspl", F), -- GNAT.String_Split
("g-sse ", F), -- GNAT.SSE
Index: g-strhas.ads
===
--- g-strhas.ads(revision 0)
+++ g-strhas.ads(revision 0)
@@ -0,0 +1,43 @@
+--
+-- --
+-- GNAT COMPILER COMPONENTS --
+-- --
+-- G N A T . S T R I N G _ H A S H --
+-- --
+-- S p e c --
+-- --
+-- Copyright (C) 2015, Free Software Foundation, Inc. --
+-- --
+-- GNAT is free software; you can redistribute it and/or modify it under --
+-- terms of the GNU General Public License as published by the Free Soft- --
+-- ware Foundation; either version 3, or (at your option) any later ver- --
+-- sion. GNAT is distributed in the hope that it will be useful, but WITH- --
+-- OUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY --
+-- or FITNESS FOR A PARTICULAR PURPOSE. --
+-- --
+-- As a special exception under Section 7 of GPL version 3, you are granted --
+-- additional permissions described in the GCC Runtime Library Exception, --
+-- version 3.1, as published by the Free Software Foundation. --
+-- --
+-- You should have received a copy of the GNU General Public License and--
+-- a copy of the GCC Runtime Library Exception along with this program; --
+-- see the files COPYING3 and COPYING.RUNTIME respectively. If not, see--
+--
[Ada] Freezing of contracts, Part_Of and current instance of a concurrent type
This patch addresses several areas: The freezing of contracts has been enhanced. A body continues to freeze the contract of the nearest enclosing package and now freezes the contracts of all eligible constructs in the same declarative list that precede the body. A concurrent constituent is no longer considered a visible state of a package body because it is already part of a single protected/task type. The current instance of a concurrent type now correctly includes single protected/task types. As a result, the current instance of such a type will appear as an implicit formal parameter of a protected subprogram or a single task type as per SPARK RM 6.1.4. -- Source -- -- subprogram_freezing.ads package Subprogram_Freezing with SPARK_Mode is procedure Force_Freeze; end Subprogram_Freezing; -- subprogram_freezing.adb package body Subprogram_Freezing with SPARK_Mode is function Func (Formal : Integer) return Integer with Contract_Cases => (Var + Formal = 1 => Var + Func'Result = 0, -- Error others => Var - Func'Result = 1), -- Error Pre=> (Var > 2), -- Error Post => (Var + Func'Result = 100); -- Error procedure Force_Freeze is begin null; end Force_Freeze; Var : Integer := 1; function Func (Formal : Integer) return Integer is begin return Formal; end Func; end Subprogram_Freezing; -- variable_freezing.ads package Variable_Freezing with SPARK_Mode is procedure Force_Freeze; end Variable_Freezing; -- variable_freezing.adb package body Variable_Freezing with SPARK_Mode is Error_1 : Integer := 1 with Part_Of => Prot; -- Error procedure Force_Freeze is begin null; end Force_Freeze; protected Prot is end Prot; protected body Prot is end Prot; end Variable_Freezing; -- Compilation and output -- $ gcc -c subprogram_freezing.adb $ gcc -c variable_freezing.adb subprogram_freezing.adb:2:13: body "Force_Freeze" declared at line 9 freezes the contract of "Func" subprogram_freezing.adb:2:13: all contractual items must be declared before body at line 9 subprogram_freezing.adb:6:30: "Var" is undefined (more references follow) variable_freezing.adb:2:04: body "Force_Freeze" declared at line 4 freezes the contract of "Error_1" variable_freezing.adb:2:04: all contractual items must be declared before body at line 4 variable_freezing.adb:2:43: "Prot" is undefined Tested on x86_64-pc-linux-gnu, committed on trunk 2015-11-18 Hristian Kirtchev * contracts.adb (Analyze_Contracts): New routine. (Analyze_Enclosing_Package_Body_Contract): Removed. (Analyze_Entry_Or_Subprogram_Contract): Add formal parameter Freeze_Id. Propagate the entity of the freezing body to vaious analysis routines. (Analyze_Initial_Declaration_Contract): Removed. (Analyze_Object_Contract): Add formal parameter Freeze_Id. Propagate the entity of the freezing body to vaious analysis routines. (Analyze_Previous_Contracts): New routine. * contracts.ads (Analyze_Enclosing_Package_Body_Contract): Removed. (Analyze_Contracts): New routine. (Analyze_Entry_Or_Subprogram_Contract): Add formal parameter Freeze_Id and update the comment on usage. (Analyze_Initial_Declaration_Contract): Removed. (Analyze_Object_Contract): Add formal parameter Freeze_Id and update the comment on usage. (Analyze_Previous_Contracts): New routine. * sem_ch3.adb (Analyze_Declarations): Use Analyze_Contracts to analyze all contracts of eligible constructs. * sem_ch6.adb (Analyze_Generic_Subprogram_Body): A body no longer freezes the contract of its initial declaration. This effect is achieved through different means. (Analyze_Subprogram_Body_Helper): A body now freezes the contracts of all eligible constructs that precede it. A body no longer freezes the contract of its initial declaration. This effect is achieved through different means. * sem_ch7.adb (Analyze_Package_Body_Helper): A body now freezes the contracts of all eligible constructs that precede it. A body no longer freezes the contract of its initial declaration. This effect is achieved through different means. * sem_ch9.adb (Analyze_Entry_Body): A body now freezes the contracts of all eligible constructs that precede it. A body no longer freezes the contract of its initial declaration. This effect is achieved through different means. (Analyze_Protected_Body): A body now freezes the contracts of all eligible constructs that precede it. A body no longer freezes the contract of its initial declaration. This ef
[PTX] OpenACC complex double reductions
Here's the version of the complex double reduction patch I've committed to
trunk.
There's no atomic cmp&swap larger than 64 bits, so we have to do something else.
I started with a patch to synthesize such an operation using a global lock,
and fitted it into the current scheme. But that (a) ended up looking
complicated and (b) had a lock. As we have to use a lock, one might as well go
for a mutex scheme.
The lock variable has to be in global memory, even if it's protecting .shared
state. Locking in .shared memory can introduce resource starvation as there's
then no descheduling of the thread attempting to get the lock. Nvidia have
confirmed that global locks do not suffer this problem.
nathan
2015-11-18 Nathan Sidwell
gcc/
* config/nvptx/nvptx.c (global_lock_var): New.
(nvptx_global_lock_addr): New.
(nvptx_lockless_update): Recomment and adjust for clarity.
(nvptx_lockfull_update): New.
(nvptx_reduction_update): New.
(nvptx_goacc_reduction_fini): Call it.
libgcc/
* config/nvptx/reduction.c: New.
* config/nvptx/t-nvptx (LIB2ADD): Add it.
libgomp/
* testsuite/libgomp.oacc-c-c++-common/reduction-cplx-flt.c: Add
worker & gang cases.
* testsuite/libgomp.oacc-c-c++-common/reduction-cplx-dbl.c: Likewise.
Index: gcc/config/nvptx/nvptx.c
===
--- gcc/config/nvptx/nvptx.c (revision 230544)
+++ gcc/config/nvptx/nvptx.c (working copy)
@@ -114,6 +114,9 @@ static unsigned worker_red_align;
#define worker_red_name "__worker_red"
static GTY(()) rtx worker_red_sym;
+/* Global lock variable, needed for 128bit worker & gang reductions. */
+static GTY(()) tree global_lock_var;
+
/* Allocate a new, cleared machine_function structure. */
static struct machine_function *
@@ -3681,8 +3684,45 @@ nvptx_generate_vector_shuffle (location_
gimplify_assign (dest_var, expr, seq);
}
-/* Insert code to locklessly update *PTR with *PTR OP VAR just before
- GSI. */
+/* Lazily generate the global lock var decl and return its address. */
+
+static tree
+nvptx_global_lock_addr ()
+{
+ tree v = global_lock_var;
+
+ if (!v)
+{
+ tree name = get_identifier ("__reduction_lock");
+ tree type = build_qualified_type (unsigned_type_node,
+ TYPE_QUAL_VOLATILE);
+ v = build_decl (BUILTINS_LOCATION, VAR_DECL, name, type);
+ global_lock_var = v;
+ DECL_ARTIFICIAL (v) = 1;
+ DECL_EXTERNAL (v) = 1;
+ TREE_STATIC (v) = 1;
+ TREE_PUBLIC (v) = 1;
+ TREE_USED (v) = 1;
+ mark_addressable (v);
+ mark_decl_referenced (v);
+}
+
+ return build_fold_addr_expr (v);
+}
+
+/* Insert code to locklessly update *PTR with *PTR OP VAR just before
+ GSI. We use a lockless scheme for nearly all case, which looks
+ like:
+ actual = initval(OP);
+ do {
+ guess = actual;
+ write = guess OP myval;
+ actual = cmp&swap (ptr, guess, write)
+ } while (actual bit-different-to guess);
+ return write;
+
+ This relies on a cmp&swap instruction, which is available for 32-
+ and 64-bit types. Larger types must use a locking scheme. */
static tree
nvptx_lockless_update (location_t loc, gimple_stmt_iterator *gsi,
@@ -3690,46 +3730,30 @@ nvptx_lockless_update (location_t loc, g
{
unsigned fn = NVPTX_BUILTIN_CMP_SWAP;
tree_code code = NOP_EXPR;
- tree type = unsigned_type_node;
-
- enum machine_mode mode = TYPE_MODE (TREE_TYPE (var));
+ tree arg_type = unsigned_type_node;
+ tree var_type = TREE_TYPE (var);
- if (!INTEGRAL_MODE_P (mode))
+ if (TREE_CODE (var_type) == COMPLEX_TYPE
+ || TREE_CODE (var_type) == REAL_TYPE)
code = VIEW_CONVERT_EXPR;
- if (GET_MODE_SIZE (mode) == GET_MODE_SIZE (DImode))
+
+ if (TYPE_SIZE (var_type) == TYPE_SIZE (long_long_unsigned_type_node))
{
+ arg_type = long_long_unsigned_type_node;
fn = NVPTX_BUILTIN_CMP_SWAPLL;
- type = long_long_unsigned_type_node;
}
+ tree swap_fn = nvptx_builtin_decl (fn, true);
+
gimple_seq init_seq = NULL;
- tree init_var = make_ssa_name (type);
- tree init_expr = omp_reduction_init_op (loc, op, TREE_TYPE (var));
- init_expr = fold_build1 (code, type, init_expr);
+ tree init_var = make_ssa_name (arg_type);
+ tree init_expr = omp_reduction_init_op (loc, op, var_type);
+ init_expr = fold_build1 (code, arg_type, init_expr);
gimplify_assign (init_var, init_expr, &init_seq);
gimple *init_end = gimple_seq_last (init_seq);
gsi_insert_seq_before (gsi, init_seq, GSI_SAME_STMT);
- gimple_seq loop_seq = NULL;
- tree expect_var = make_ssa_name (type);
- tree actual_var = make_ssa_name (type);
- tree write_var = make_ssa_name (type);
-
- tree write_expr = fold_build1 (code, TREE_TYPE (var), expect_var);
- write_expr = fold_build2 (op, TREE_TYPE (var), write_expr, var);
- write_expr = fold_build1 (code, type, write_expr);
- gimplify_assign (write_var, write_expr, &loop_seq);
-
- tree swap_expr = nvptx_builtin_decl (fn, tru
Re: [PATCH, PR tree-optimization/68327] Compute vectype for live phi nodes when copmputing VF
2015-11-18 16:44 GMT+03:00 Richard Biener : > On Wed, Nov 18, 2015 at 12:34 PM, Ilya Enkovich > wrote: >> Hi, >> >> When we compute vectypes we skip non-relevant phi nodes. But we process >> non-relevant alive statements and thus may need vectype of non-relevant live >> phi node to compute mask vectype. This patch enables vectype computation >> for live phi nodes. Botostrapped and regtested on x86_64-unknown-linux-gnu. >> OK for trunk? > > Hmm. What breaks if you instead skip all !relevant stmts and not > compute vectype for life but not relevant ones? We won't ever > "vectorize" !relevant ones, that is, we don't need their vector type. I tried it and got regression in SLP. It expected non-null vectype for non-releveant but live statement. Regression was in gcc/gcc/testsuite/gfortran.fortran-torture/execute/pr43390.f90 Ilya > > Richard. > >> Thanks, >> Ilya
