Refactor gimple_expr_type
Hi,
I have tried to refactor gimple_expr_type to make it more readable. Removed the
switch block and redundant if.
Please review this patch.
Thanks,
-Aditya
gcc/ChangeLog:
2015-05-15 hiraditya
* gimple.h (gimple_expr_type): Refactor to make it concise. Remove
redundant if.
diff --git a/gcc/gimple.h b/gcc/gimple.h
index 95e4fc8..168d3ba 100644
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -5717,35 +5717,28 @@ static inline tree
gimple_expr_type (const_gimple stmt)
{
enum gimple_code code = gimple_code (stmt);
-
- if (code == GIMPLE_ASSIGN || code == GIMPLE_CALL)
+ tree type;
+ /* In general we want to pass out a type that can be substituted
+ for both the RHS and the LHS types if there is a possibly
+ useless conversion involved. That means returning the
+ original RHS type as far as we can reconstruct it. */
+ if (code == GIMPLE_CALL)
{
- tree type;
- /* In general we want to pass out a type that can be substituted
- for both the RHS and the LHS types if there is a possibly
- useless conversion involved. That means returning the
- original RHS type as far as we can reconstruct it. */
- if (code == GIMPLE_CALL)
- {
- const gcall *call_stmt = as_a (stmt);
- if (gimple_call_internal_p (call_stmt)
- && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
- type = TREE_TYPE (gimple_call_arg (call_stmt, 3));
- else
- type = gimple_call_return_type (call_stmt);
- }
+ const gcall *call_stmt = as_a (stmt);
+ if (gimple_call_internal_p (call_stmt)
+ && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
+ type = TREE_TYPE (gimple_call_arg (call_stmt, 3));
+ else
+ type = gimple_call_return_type (call_stmt);
+ return type;
+ }
+ else if (code == GIMPLE_ASSIGN)
+ {
+ if (gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR)
+ type = TREE_TYPE (gimple_assign_rhs1 (stmt));
else
- switch (gimple_assign_rhs_code (stmt))
- {
- case POINTER_PLUS_EXPR:
- type = TREE_TYPE (gimple_assign_rhs1 (stmt));
- break;
-
- default:
- /* As fallback use the type of the LHS. */
- type = TREE_TYPE (gimple_get_lhs (stmt));
- break;
- }
+ /* As fallback use the type of the LHS. */
+ type = TREE_TYPE (gimple_get_lhs (stmt));
return type;
}
else if (code == GIMPLE_COND)
Re: [ARM] fix arm bootstrap
On 15/05/15 07:46, Kugan wrote: Sorry, this is not the right fix. Looking at it. Hi Kugan, FWIW, I'm seeing this bootstrap failure as well. If you have a patch for this, it would be appreciated. Kyrill Thanks, Kugan On 15/05/15 16:41, Kugan wrote: Hi Richard, r223090 seem to miss a parenthesis and seem to be causing: from ../../widen/gcc/fold-const.c:46: ../../widen/gcc/fold-const.c: In function 'tree_node* fold_range_test(location_t, tree_code, tree, tree, tree)': ../../widen/gcc/config/arm/arm.h:2068:4: error: enumeral and non-enumeral type in conditional expression [-Werror=extra] ? (TARGET_THUMB ? false : true) \ ^ ../../widen/gcc/fold-const.c:5234:12: note: in expansion of macro 'LOGICAL_OP_NON_SHORT_CIRCUIT' else if (LOGICAL_OP_NON_SHORT_CIRCUIT ^ ../../widen/gcc/fold-const.c: In function 'tree_node* fold_truth_andor(location_t, tree_code, tree, tree, tree, tree, tree)': ../../widen/gcc/config/arm/arm.h:2068:4: error: enumeral and non-enumeral type in conditional expression [-Werror=extra] ? (TARGET_THUMB ? false : true) \ ^ ../../widen/gcc/fold-const.c:8431:7: note: in expansion of macro 'LOGICAL_OP_NON_SHORT_CIRCUIT' if (LOGICAL_OP_NON_SHORT_CIRCUIT Regression testing for arm-none-linux-gnueab is ongoing. Is this OK for trunk if no regression? Thanks, Kugan gcc/ChangeLog: 2015-05-15 Kugan Vivekanandarajah * config/arm/arm.h (enum arm_auto_incmodes): Add parenthesis.
[PATCH][AArch64] Removed unused SLOWMUL target flags
Hi all, It seems that AARCH64_TUNE_SLOWMUL somehow slipped from the arm backend into the aarch64 one. This macro is not used anywhere and I don't expect it will be used. It has been there since October 2012 i.e. when the port was first contributed. This patch removes that and the relevant AARCH64_FL_SLOWMUL flag definition as well. Bootstrapped and tested on aarch64. Ok for trunk? Thanks, Kyrill 2015-05-15 Kyrylo Tkachov * config/aarch64/aarch64.h (AARCH64_TUNE_SLOWMUL): Delete. (AARCH64_FL_SLOWMUL): Delete. (AARCH64_FL_CRC): Redefine to 1<<3. (AARCH64_FL_USE_FMA_STEERING_PASS): Redefine to 1<<4. diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h index c85d279..25b9927 100644 --- a/gcc/config/aarch64/aarch64.h +++ b/gcc/config/aarch64/aarch64.h @@ -198,10 +198,9 @@ extern unsigned aarch64_architecture_version; #define AARCH64_FL_SIMD (1 << 0) /* Has SIMD instructions. */ #define AARCH64_FL_FP (1 << 1) /* Has FP. */ #define AARCH64_FL_CRYPTO (1 << 2) /* Has crypto. */ -#define AARCH64_FL_SLOWMUL(1 << 3) /* A slow multiply core. */ -#define AARCH64_FL_CRC(1 << 4) /* Has CRC. */ +#define AARCH64_FL_CRC(1 << 3) /* Has CRC. */ /* Has static dispatch of FMA. */ -#define AARCH64_FL_USE_FMA_STEERING_PASS (1 << 5) +#define AARCH64_FL_USE_FMA_STEERING_PASS (1 << 4) /* Has FP and SIMD. */ #define AARCH64_FL_FPSIMD (AARCH64_FL_FP | AARCH64_FL_SIMD) @@ -221,7 +220,6 @@ extern unsigned long aarch64_isa_flags; /* Macros to test tuning flags. */ extern unsigned long aarch64_tune_flags; -#define AARCH64_TUNE_SLOWMUL (aarch64_tune_flags & AARCH64_FL_SLOWMUL) #define AARCH64_TUNE_FMA_STEERING \ (aarch64_tune_flags & AARCH64_FL_USE_FMA_STEERING_PASS)
Re: [ARM] fix arm bootstrap
On 15/05/15 07:41, Kugan wrote: Hi Richard, r223090 seem to miss a parenthesis and seem to be causing: from ../../widen/gcc/fold-const.c:46: ../../widen/gcc/fold-const.c: In function 'tree_node* fold_range_test(location_t, tree_code, tree, tree, tree)': ../../widen/gcc/config/arm/arm.h:2068:4: error: enumeral and non-enumeral type in conditional expression [-Werror=extra] ? (TARGET_THUMB ? false : true) \ ^ Looks like a missing cast - A cast appears to let me continue with the build (fold-const.o builds without any warnings). Will apply it if the bootstrap finishes ok with an appropriate changelog. regards Ramana * config/arm/arm.h (LOGICAL_OP_SHORT_CIRCUIT): Add static casts. ../../widen/gcc/fold-const.c:5234:12: note: in expansion of macro 'LOGICAL_OP_NON_SHORT_CIRCUIT' else if (LOGICAL_OP_NON_SHORT_CIRCUIT ^ ../../widen/gcc/fold-const.c: In function 'tree_node* fold_truth_andor(location_t, tree_code, tree, tree, tree, tree, tree)': ../../widen/gcc/config/arm/arm.h:2068:4: error: enumeral and non-enumeral type in conditional expression [-Werror=extra] ? (TARGET_THUMB ? false : true) \ ^ ../../widen/gcc/fold-const.c:8431:7: note: in expansion of macro 'LOGICAL_OP_NON_SHORT_CIRCUIT' if (LOGICAL_OP_NON_SHORT_CIRCUIT Regression testing for arm-none-linux-gnueab is ongoing. Is this OK for trunk if no regression? Thanks, Kugan gcc/ChangeLog: 2015-05-15 Kugan Vivekanandarajah * config/arm/arm.h (enum arm_auto_incmodes): Add parenthesis. diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h index 8574388..a58a7b1 100644 --- a/gcc/config/arm/arm.h +++ b/gcc/config/arm/arm.h @@ -2060,8 +2060,8 @@ enum arm_auto_incmodes #define LOGICAL_OP_NON_SHORT_CIRCUIT \ ((optimize_size) \ ? (TARGET_THUMB ? false : true) \ - : TARGET_THUMB ? current_tune->logical_op_non_short_circuit_thumb \ - : current_tune->logical_op_non_short_circuit_arm) + : TARGET_THUMB ? static_cast (current_tune->logical_op_non_short_circuit_thumb) \ + : static_cast (current_tune->logical_op_non_short_circuit_arm)) /* Position Independent Code. */
[PATCH] Fix PR ipa/65908.
Hello.
Following patch is fix for GCC-5 branch for PR ipa/65908, was tested on
x86_64-linux-pc, as well as bootstrapped.
As soon as the patch is applied, I'm going to send the similar patch for trunk.
Ready for 5 branch?
Thanks,
Martin
gcc/testsuite/ChangeLog:
2015-05-12 Martin Liska
* g++.dg/ipa/pr65908.C: New test.
gcc/ChangeLog:
2015-05-12 Martin Liska
PR ipa/65908
* ipa-icf.c (sem_function::equals_private): Always compare argument
type got from TYPE_ARG_TYPES.
(sem_function::compatible_parm_types_p): New function.
(sem_function::equals_wpa): Use the function.
(sem_function::equals_private): Likewise.
(sem_function::parse_tree_args): Handle case where we have a different
number of arguments.
* ipa-icf.h (sem_function::compatible_parm_types_p): Declare new
function.
---
gcc/ipa-icf.c | 63 +++---
gcc/ipa-icf.h | 3 ++
gcc/testsuite/g++.dg/ipa/pr65908.C | 26
3 files changed, 68 insertions(+), 24 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/ipa/pr65908.C
diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
index b902373..e2da19e 100644
--- a/gcc/ipa-icf.c
+++ b/gcc/ipa-icf.c
@@ -417,6 +417,33 @@ bool sem_function::compare_edge_flags (cgraph_edge *e1,
cgraph_edge *e2)
return true;
}
+/* Return true if DECL_ARGUMENT types are valid to be merged. */
+
+bool
+sem_function::compatible_parm_types_p ()
+{
+ tree parm1, parm2;
+
+ for (parm1 = DECL_ARGUMENTS (decl),
+ parm2 = DECL_ARGUMENTS (m_compared_func->decl);
+ parm1 && parm2; parm1 = DECL_CHAIN (parm1), parm2 = DECL_CHAIN (parm2))
+ {
+if (POINTER_TYPE_P (parm1)
+ && (TYPE_RESTRICT (parm1) != TYPE_RESTRICT (parm2)))
+ return return_false_with_msg ("argument restrict flag mismatch");
+/* nonnull_arg_p implies non-zero range to REFERENCE types. */
+if (POINTER_TYPE_P (parm1)
+ && TREE_CODE (parm1) != TREE_CODE (parm2)
+ && opt_for_fn (decl, flag_delete_null_pointer_checks))
+ return return_false_with_msg ("pointer wrt reference mismatch");
+ }
+
+ if (parm1 || parm2)
+return return_false ();
+
+ return true;
+}
+
/* Fast equality function based on knowledge known in WPA. */
bool
@@ -520,12 +547,12 @@ sem_function::equals_wpa (sem_item *item,
if (!func_checker::compatible_types_p (arg_types[i],
m_compared_func->arg_types[i]))
return return_false_with_msg ("argument type is different");
- if (POINTER_TYPE_P (arg_types[i])
- && (TYPE_RESTRICT (arg_types[i])
- != TYPE_RESTRICT (m_compared_func->arg_types[i])))
- return return_false_with_msg ("argument restrict flag mismatch");
}
+ /* For functions with !prototype_p, we have to compare DECL_ARGUMENTS. */
+ if (!compatible_parm_types_p ())
+return return_false ();
+
if (node->num_references () != item->node->num_references ())
return return_false_with_msg ("different number of references");
@@ -743,9 +770,12 @@ sem_function::equals_private (sem_item *item,
for (arg1 = DECL_ARGUMENTS (decl),
arg2 = DECL_ARGUMENTS (m_compared_func->decl);
arg1; arg1 = DECL_CHAIN (arg1), arg2 = DECL_CHAIN (arg2))
-if (!m_checker->compare_decl (arg1, arg2))
+if (!arg2 || !m_checker->compare_decl (arg1, arg2))
return return_false ();
+ if (!compatible_parm_types_p ())
+return return_false ();
+
/* Fill-up label dictionary. */
for (unsigned i = 0; i < bb_sorted.length (); ++i)
{
@@ -1481,30 +1511,15 @@ sem_function::parse (cgraph_node *node, bitmap_obstack
*stack)
void
sem_function::parse_tree_args (void)
{
- tree result;
-
if (arg_types.exists ())
arg_types.release ();
arg_types.create (4);
- tree fnargs = DECL_ARGUMENTS (decl);
+ tree type = TYPE_ARG_TYPES (TREE_TYPE (decl));
+ for (tree parm = type; parm; parm = TREE_CHAIN (parm))
+arg_types.safe_push (TREE_VALUE (parm));
- for (tree parm = fnargs; parm; parm = DECL_CHAIN (parm))
-arg_types.safe_push (DECL_ARG_TYPE (parm));
-
- /* Function result type. */
- result = DECL_RESULT (decl);
- result_type = result ? TREE_TYPE (result) : NULL;
-
- /* During WPA, we can get arguments by following method. */
- if (!fnargs)
-{
- tree type = TYPE_ARG_TYPES (TREE_TYPE (decl));
- for (tree parm = type; parm; parm = TREE_CHAIN (parm))
- arg_types.safe_push (TYPE_CANONICAL (TREE_VALUE (parm)));
-
- result_type = TREE_TYPE (TREE_TYPE (decl));
-}
+ result_type = TREE_TYPE (TREE_TYPE (decl));
}
/* For given basic blocks BB1 and BB2 (from functions FUNC1 and FUNC),
diff --git a/gcc/ipa-icf.h b/gcc/ipa-icf.h
index 7eb9f27..d6dabfe 100644
--- a/gcc/ipa-icf.h
+++ b/gcc/ipa-icf.h
@@ -364,6 +364,9 @@ private:
bool equals_private (sem_item *item,
hash_map &ignored_node
Re: [PATCH diagnostics/fortran] Handle two locations for the same diagnostic. Convert all gfc_warning_1 and gfc_notify_std_1 calls
Manuel López-Ibáñez writes:
> Thanks for the review. I followed all your suggestions. For the
> accessor functions, I was not sure what type you would prefer, so I
> implemented them as C++ methods and made use of 'private' to be sure
> they are the only way to access the locations array. If you want me to
> change it, just tell me what you prefer. I also replaced
> diagnostic_same_locus with diagnostic_same_line.
Great, thank you.
>
> Bootstrapped & regression tested on x86_64-linux-gnu.
>
> OK?
Yes, but with the slight changes below. Sorry I spotted them just now,
but they are quick to fix, I believe.
>
> Manuel.
>
[...]
> -/* Expand the location of this diagnostic. Use this function for
> consistency. */
> +/* Return the location associated to this diagnostic. WHICH specifies
Here, I think only the 'W' (in WHICH) should be uppercase.
[...]
> /* The type of a text to be formatted according a format specification
> along with a list of things. */
> struct text_info
> {
> +public:
As this is a struct, the 'public' here is not necessary, as the members
are public by default.
>const char *format_spec;
>va_list *args_ptr;
>int err_no; /* for %m */
> - location_t *locus;
>void **x_data;
> +
> + inline location_t & set_location (unsigned int index_of_location)
I think it's less surprising to have the this function take two
parameters: The index_of_location and the new new location.
So that it's used by doing: set_location (0, the_new_location);
> + {
> +gcc_checking_assert (index_of_location < MAX_LOCATIONS_PER_MESSAGE);
> +return this->locations[index_of_location];
> + }
> +
> + inline location_t get_location (unsigned int index_of_location) const
> + {
> +gcc_checking_assert (index_of_location < MAX_LOCATIONS_PER_MESSAGE);
> +return this->locations[index_of_location];
> + }
> +
> +private:
> + location_t locations[MAX_LOCATIONS_PER_MESSAGE];
> };
>
[...]
OK to commit with the changes above.
Thanks a lot!
--
Dodji
Re: [PATCH][AArch64] Removed unused SLOWMUL target flags
On Fri, May 15, 2015 at 09:04:52AM +0100, Kyrill Tkachov wrote: > Hi all, > > It seems that AARCH64_TUNE_SLOWMUL somehow slipped from the arm backend into > the aarch64 one. > This macro is not used anywhere and I don't expect it will be used. It has > been there since > October 2012 i.e. when the port was first contributed. > This patch removes that and the relevant AARCH64_FL_SLOWMUL flag definition > as well. > > Bootstrapped and tested on aarch64. Looks obvious to me, please apply. Note that, as I've mentioned in the past, you don't need to ask for approval for such obvious patches, this is cleaning up obviously dead code. See https://gcc.gnu.org/svnwrite.html#policies for details of the policy for obvious fixes (under the section "Free for all"). Thanks, James > 2015-05-15 Kyrylo Tkachov > > * config/aarch64/aarch64.h (AARCH64_TUNE_SLOWMUL): Delete. > (AARCH64_FL_SLOWMUL): Delete. > (AARCH64_FL_CRC): Redefine to 1<<3. > (AARCH64_FL_USE_FMA_STEERING_PASS): Redefine to 1<<4. > diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h > index c85d279..25b9927 100644 > --- a/gcc/config/aarch64/aarch64.h > +++ b/gcc/config/aarch64/aarch64.h > @@ -198,10 +198,9 @@ extern unsigned aarch64_architecture_version; > #define AARCH64_FL_SIMD (1 << 0) /* Has SIMD instructions. */ > #define AARCH64_FL_FP (1 << 1) /* Has FP. */ > #define AARCH64_FL_CRYPTO (1 << 2) /* Has crypto. */ > -#define AARCH64_FL_SLOWMUL(1 << 3) /* A slow multiply core. */ > -#define AARCH64_FL_CRC(1 << 4) /* Has CRC. */ > +#define AARCH64_FL_CRC(1 << 3) /* Has CRC. */ > /* Has static dispatch of FMA. */ > -#define AARCH64_FL_USE_FMA_STEERING_PASS (1 << 5) > +#define AARCH64_FL_USE_FMA_STEERING_PASS (1 << 4) > > /* Has FP and SIMD. */ > #define AARCH64_FL_FPSIMD (AARCH64_FL_FP | AARCH64_FL_SIMD) > @@ -221,7 +220,6 @@ extern unsigned long aarch64_isa_flags; > > /* Macros to test tuning flags. */ > extern unsigned long aarch64_tune_flags; > -#define AARCH64_TUNE_SLOWMUL (aarch64_tune_flags & AARCH64_FL_SLOWMUL) > #define AARCH64_TUNE_FMA_STEERING \ >(aarch64_tune_flags & AARCH64_FL_USE_FMA_STEERING_PASS) >
[PATCH, PR middle-end/66134] Fix coalescing for bounds used in abnormal phi
Hi,
This patch fixes misuse of abnormal bounds copy to avoid coalescing issue.
Bootstrapped and regtested for x86_64-unknown-linux-gnu. Applied to trunk. Is
it OK for gcc-5?
Thanks,
Ilya
--
gcc/
2015-05-15 Ilya Enkovich
PR middle-end/66134
* tree-chkp.c (chkp_get_orginal_bounds_for_abnormal_copy): New.
(chkp_maybe_copy_and_register_bounds): Don't copy abnormal copy.
gcc/testsuite/
2015-05-15 Ilya Enkovich
PR middle-end/66134
* gcc.target/i386/mpx/pr66134.c: New test.
diff --git a/gcc/testsuite/gcc.target/i386/mpx/pr66134.c
b/gcc/testsuite/gcc.target/i386/mpx/pr66134.c
new file mode 100755
index 000..3889674
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/mpx/pr66134.c
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fcheck-pointer-bounds -mmpx -fno-tree-ccp" } */
+
+extern int vfork (void) __attribute__ ((__nothrow__ , __leaf__));
+void test1 (void);
+void test2 (void);
+void test3 (int *);
+
+void test (int *p)
+{
+ test1 ();
+ p++;
+ test2 ();
+ p++;
+ vfork ();
+ test3 (p);
+}
diff --git a/gcc/tree-chkp.c b/gcc/tree-chkp.c
index 288470b..17a52bc 100644
--- a/gcc/tree-chkp.c
+++ b/gcc/tree-chkp.c
@@ -1097,7 +1097,20 @@ chkp_get_bounds_var (tree ptr_var)
return bnd_var;
}
+/* If BND is an abnormal bounds copy, return a copied value.
+ Otherwise return BND. */
+static tree
+chkp_get_orginal_bounds_for_abnormal_copy (tree bnd)
+{
+ if (bitmap_bit_p (chkp_abnormal_copies, SSA_NAME_VERSION (bnd)))
+{
+ gimple bnd_def = SSA_NAME_DEF_STMT (bnd);
+ gcc_checking_assert (gimple_code (bnd_def) == GIMPLE_ASSIGN);
+ bnd = gimple_assign_rhs1 (bnd_def);
+}
+ return bnd;
+}
/* Register bounds BND for object PTR in global bounds table.
A copy of bounds may be created for abnormal ssa names.
@@ -1141,11 +1154,7 @@ chkp_maybe_copy_and_register_bounds (tree ptr, tree bnd)
/* For abnormal copies we may just find original
bounds and use them. */
if (!abnormal_ptr && !SSA_NAME_IS_DEFAULT_DEF (bnd))
- {
- gimple bnd_def = SSA_NAME_DEF_STMT (bnd);
- gcc_checking_assert (gimple_code (bnd_def) == GIMPLE_ASSIGN);
- bnd = gimple_assign_rhs1 (bnd_def);
- }
+ bnd = chkp_get_orginal_bounds_for_abnormal_copy (bnd);
/* For undefined values we usually use none bounds
value but in case of abnormal edge it may cause
coalescing failures. Use default definition of
@@ -1177,6 +1186,7 @@ chkp_maybe_copy_and_register_bounds (tree ptr, tree bnd)
copy = make_temp_ssa_name (pointer_bounds_type_node,
gimple_build_nop (),
CHKP_BOUND_TMP_NAME);
+ bnd = chkp_get_orginal_bounds_for_abnormal_copy (bnd);
assign = gimple_build_assign (copy, bnd);
if (dump_file && (dump_flags & TDF_DETAILS))
Improve error message for vector conversion
Hello,
this patch was regtested on ppc64le-redhat-linux.
2015-05-15 Marc Glisse
gcc/
* convert.c (convert_to_integer, convert_to_vector): Include the
types in the error message.
gcc/testsuite/
* gcc.dg/simd-1.c: Update to the new message.
--
Marc GlisseIndex: gcc/convert.c
===
--- gcc/convert.c (revision 223214)
+++ gcc/convert.c (working copy)
@@ -913,21 +913,23 @@ convert_to_integer (tree type, tree expr
return build1 (FIXED_CONVERT_EXPR, type, expr);
case COMPLEX_TYPE:
return convert (type,
fold_build1 (REALPART_EXPR,
TREE_TYPE (TREE_TYPE (expr)), expr));
case VECTOR_TYPE:
if (!tree_int_cst_equal (TYPE_SIZE (type), TYPE_SIZE (TREE_TYPE (expr
{
- error ("can%'t convert between vector values of different size");
+ error ("can%'t convert a vector of type %qT"
+" to type %qT which has different size",
+TREE_TYPE (expr), type);
return error_mark_node;
}
return build1 (VIEW_CONVERT_EXPR, type, expr);
default:
error ("aggregate value used where an integer was expected");
return convert (type, integer_zero_node);
}
}
@@ -997,21 +999,23 @@ convert_to_complex (tree type, tree expr
tree
convert_to_vector (tree type, tree expr)
{
switch (TREE_CODE (TREE_TYPE (expr)))
{
case INTEGER_TYPE:
case VECTOR_TYPE:
if (!tree_int_cst_equal (TYPE_SIZE (type), TYPE_SIZE (TREE_TYPE (expr
{
- error ("can%'t convert between vector values of different size");
+ error ("can%'t convert a value of type %qT"
+" to vector type %qT which has different size",
+TREE_TYPE (expr), type);
return error_mark_node;
}
return build1 (VIEW_CONVERT_EXPR, type, expr);
default:
error ("can%'t convert value to a vector");
return error_mark_node;
}
}
Index: gcc/testsuite/gcc.dg/simd-1.c
===
--- gcc/testsuite/gcc.dg/simd-1.c (revision 223214)
+++ gcc/testsuite/gcc.dg/simd-1.c (working copy)
@@ -38,21 +38,21 @@ hanneke ()
/* Casted different signed SIMD assignment. */
f = (uv4si) a;
/* Assignment between scalar and SIMD of different size. */
foo = a; /* { dg-error "incompatible types when assigning" } */
/* Casted assignment between scalar and SIMD of same size. */
foo = (typeof (foo)) foo2;
/* Casted assignment between scalar and SIMD of different size. */
- foo1 = (typeof (foo1)) foo2; /* { dg-error "can't convert between vector
values of different size" } */
+ foo1 = (typeof (foo1)) foo2; /* { dg-error "can't convert a vector of type"
} */
/* Operators on compatible SIMD types. */
a += b + b;
a -= b;
a *= b;
a /= b;
a = -b;
/* Operators on incompatible SIMD types. */
a = b + c; /* { dg-error "invalid operands to binary +" } */
RE: [RFC: Patch, PR 60158] gcc/varasm.c : Pass actual alignment value to output_constant_pool_2
> -Original Message- > From: Jeff Law [mailto:l...@redhat.com] > Sent: Friday, May 15, 2015 10:30 AM > > Just to summarize: By default in GCC v4.7.x, all the constants are put > > into '.rodata.str1.4' section. In GCC v4.8.x from r192719 onwards, one > > of the move instruction of the string constant ".LC0" is getting > > spilled. The reload pass, for any constants that aren't allowed and > > can't be reloaded in to registers tries to change them into memory > > references. Then while emitting that string constant to asm code > > (A:varasm.c: output_constant_pool_1), it explicitly passes the > > alignment as 1 which prevents the generation of fix-up table entries > > in 'B: rs6000.c:rs6000_assemble_integer' because the data is > > considered unaligned now. > > > > The bug seems to have gone latent with an unrelated trunk commit > > r204695 [* tree-ssa-loop-ivopts.c (force_expr_to_var_cost): Refactor > > the code. Handle type conversion.]. This commit chooses different > > spill candidates hence all the string constants are being put in to > > '.rodata.str1.4´section. > > > > The check I had in the test case is that if there is a > > '.data.rel.ro.local', then there should be '.fixup' section generated. > > > > Please let me know if you need any other details. > Thanks. Even though I wasn't able to trigger the bug with the testcase from > 65018, I went ahead and committed this patch to the trunk. It can't hurt and > it's the right thing to do. > > Thanks for your patience, > Jeff, Thanks for checking-in the changes. Can we apply the patch to GCC v4.8 and GCC v4.9 branch as well? As mentioned earlier, the issue is still reproducible in GCC v4.8 branch [r223205] but not in GCC v4.9 branch. Regards, Rohit
Re: [PATCH] Fix PR ipa/65908.
On Fri, May 15, 2015 at 10:38:57AM +0200, Martin Liška wrote:
> Following patch is fix for GCC-5 branch for PR ipa/65908, was tested on
> x86_64-linux-pc, as well as bootstrapped.
> As soon as the patch is applied, I'm going to send the similar patch for
> trunk.
I'll leave the review to Honza or Richi, just a few nits:
1) it should go to the trunk first, then to GCC-5 branch
> --- a/gcc/ipa-icf.c
> +++ b/gcc/ipa-icf.c
> @@ -417,6 +417,33 @@ bool sem_function::compare_edge_flags (cgraph_edge *e1,
> cgraph_edge *e2)
>return true;
> }
> +/* Return true if DECL_ARGUMENT types are valid to be merged. */
Please add another newline between } and the comment.
> --- a/gcc/ipa-icf.h
> +++ b/gcc/ipa-icf.h
> @@ -364,6 +364,9 @@ private:
>bool equals_private (sem_item *item,
> hash_map &ignored_nodes);
> + /* Return true if DECL_ARGUMENT types are valid to be merged. */
> + bool compatible_parm_types_p ();
> +
>/* Returns true if tree T can be compared as a handled component. */
>static bool icf_handled_component_p (tree t);
And here should be an empty line above the Return true if ... comment
too. Though, I wonder how this can apply cleanly to the current 5 branch,
because there is an empty newline in between equals_private and the comment
above icf_handled_component_p.
> diff --git a/gcc/testsuite/g++.dg/ipa/pr65908.C
> b/gcc/testsuite/g++.dg/ipa/pr65908.C
> new file mode 100644
> index 000..c1884ab
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/ipa/pr65908.C
> @@ -0,0 +1,26 @@
> +// PR ipa/65908
> +// { dg-options "-O2 -fPIC" }
> +// { dg-do compile { target { { fpic } } } }
Perhaps
// PR ipa/65908
// { dg-do compile }
// { dg-options "-O2" }
// { dg-additional-options "-fPIC" { target fpic } }
instead?
Jakub
Small fold-const.c / match.pd tweaks
Hello,
these are just a few tweaks / clean-ups. The 'ord' pattern is the dual of
the 'unord' one I added last month. The tree_unary_nonnegative_warnv_p
change should have no effect (it is only used from VRP which does not
handle vectors (yet?)) but it seems right to handle vectors like scalars
here.
Regtested on ppc64le-redhat-linux.
2015-05-15 Marc Glisse
PR tree-optimization/63387
gcc/
* match.pd ((X /[ex] A) * A -> X): Remove unnecessary condition.
((x ord x) & (y ord y) -> (x ord y),
(x ord x) & (x ord y) -> (x ord y)): New simplifications.
* fold-const.c (tree_unary_nonnegative_warnv_p) : Handle
vectors like scalars.
gcc/testsuite/
* gcc.dg/pr63387-2.c: New testcase.
--
Marc GlisseIndex: gcc/fold-const.c
===
--- gcc/fold-const.c(revision 223214)
+++ gcc/fold-const.c(working copy)
@@ -14685,21 +14685,21 @@ tree_unary_nonnegative_warnv_p (enum tre
bool *strict_overflow_p)
{
if (TYPE_UNSIGNED (type))
return true;
switch (code)
{
case ABS_EXPR:
/* We can't return 1 if flag_wrapv is set because
ABS_EXPR = INT_MIN. */
- if (!INTEGRAL_TYPE_P (type))
+ if (!ANY_INTEGRAL_TYPE_P (type))
return true;
if (TYPE_OVERFLOW_UNDEFINED (type))
{
*strict_overflow_p = true;
return true;
}
break;
case NON_LVALUE_EXPR:
case FLOAT_EXPR:
Index: gcc/match.pd
===
--- gcc/match.pd(revision 223214)
+++ gcc/match.pd(working copy)
@@ -791,22 +791,21 @@ along with GCC; see the file COPYING3.
&& TYPE_PRECISION (type) <= TYPE_PRECISION (TREE_TYPE (@0))
&& operand_equal_p (@1, build_low_bits_mask (TREE_TYPE (@1),
TYPE_PRECISION (type)), 0))
(convert @0)))
/* (X /[ex] A) * A -> X. */
(simplify
(mult (convert? (exact_div @0 @1)) @1)
/* Look through a sign-changing conversion. */
- (if (TYPE_PRECISION (TREE_TYPE (@0)) == TYPE_PRECISION (type))
- (convert @0)))
+ (convert @0))
/* Canonicalization of binary operations. */
/* Convert X + -C into X - C. */
(simplify
(plus @0 REAL_CST@1)
(if (REAL_VALUE_NEGATIVE (TREE_REAL_CST (@1)))
(with { tree tem = fold_unary (NEGATE_EXPR, type, @1); }
(if (!TREE_OVERFLOW (tem) || !flag_trapping_math)
(minus @0 { tem; })
@@ -944,22 +943,29 @@ along with GCC; see the file COPYING3.
(icmp @0 @1))
(if (ic == ncmp)
(ncmp @0 @1)
/* Unordered tests if either argument is a NaN. */
(simplify
(bit_ior (unordered @0 @0) (unordered @1 @1))
(if (types_match (@0, @1))
(unordered @0 @1)))
(simplify
+ (bit_and (ordered @0 @0) (ordered @1 @1))
+ (if (types_match (@0, @1))
+ (ordered @0 @1)))
+(simplify
(bit_ior:c (unordered @0 @0) (unordered:c@2 @0 @1))
@2)
+(simplify
+ (bit_and:c (ordered @0 @0) (ordered:c@2 @0 @1))
+ @2)
/* Simplification of math builtins. */
(define_operator_list LOG BUILT_IN_LOGF BUILT_IN_LOG BUILT_IN_LOGL)
(define_operator_list EXP BUILT_IN_EXPF BUILT_IN_EXP BUILT_IN_EXPL)
(define_operator_list LOG2 BUILT_IN_LOG2F BUILT_IN_LOG2 BUILT_IN_LOG2L)
(define_operator_list EXP2 BUILT_IN_EXP2F BUILT_IN_EXP2 BUILT_IN_EXP2L)
(define_operator_list LOG10 BUILT_IN_LOG10F BUILT_IN_LOG10 BUILT_IN_LOG10L)
(define_operator_list EXP10 BUILT_IN_EXP10F BUILT_IN_EXP10 BUILT_IN_EXP10L)
(define_operator_list POW BUILT_IN_POWF BUILT_IN_POW BUILT_IN_POWL)
Index: gcc/testsuite/gcc.dg/pr63387-2.c
===
--- gcc/testsuite/gcc.dg/pr63387-2.c(revision 0)
+++ gcc/testsuite/gcc.dg/pr63387-2.c(working copy)
@@ -0,0 +1,26 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fdump-tree-optimized" } */
+
+int f(double aaa, double bbb){
+ int xa = !__builtin_isunordered(aaa, aaa);
+ int xb = !__builtin_isunordered(bbb, bbb);
+ return xa & xb;
+}
+
+int g(double aaa, double bbb){
+ int xa = !__builtin_isunordered(aaa, bbb);
+ int xb = !__builtin_isunordered(bbb, bbb);
+ return xa & xb;
+}
+
+int h(double ccc, float ddd){
+ int xc = !__builtin_isunordered(ccc, ccc);
+ int xd = !__builtin_isunordered(ddd, ddd);
+ return xc & xd;
+}
+
+/* { dg-final { scan-tree-dump-not "aaa\[^\n\r\]* ord aaa" "optimized" } } */
+/* { dg-final { scan-tree-dump-not "bbb\[^\n\r\]* ord bbb" "optimized" } } */
+/* { dg-final { scan-tree-dump-times "aaa\[^\n\r\]* ord bbb" 2 "optimized" } }
*/
+/* { dg-final { scan-tree-dump-not "ccc\[^\n\r\]* ord ddd" "optimized" } } */
+/* { dg-final { cleanup-tree-dump "optimized" } } */
Re: [patch, fortran] fix for PR 60780, PR 40958
> The change may be small enough that an assignment isn't needed. > We (ie, the gfortran developers) will need to check. I think that’s small enough, compared to what we’ve accepted as such in the past. If not, a disclaimer by Russell putting his change in the public domain would also be a quick way: https://gcc.gnu.org/contribute.html#legal > Having an assignment will help when you submit additional patches. :-) Indeed! Regarding the patch itself, it seems OK. (I first wondered if the strcmp() is necessary, but it appears module strings at this point are not GCC identifiers, but normal strings.) Russell, you said “tested on x86_64-linux”. Could you explicitly confirm that you have bootstrapped it and regression-tested the full gfortran testsuite ? Cheers, FX
[PATCH 2/2] IPA ICF: add support for debug counter.
Hi. Following patch add support of dbg-cnt for IPA ICF pass. Ready for trunk? Thanks, Martin gcc/ChangeLog: 2015-05-15 Martin Liska * dbgcnt.def: Add new counter. * ipa-icf.c (sem_item_optimizer::merge_classes): Use the counter. --- gcc/dbgcnt.def | 1 + gcc/ipa-icf.c | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/gcc/dbgcnt.def b/gcc/dbgcnt.def index 95f6b06..24bf24b 100644 --- a/gcc/dbgcnt.def +++ b/gcc/dbgcnt.def @@ -168,6 +168,7 @@ DEBUG_COUNTER (if_after_reload) DEBUG_COUNTER (if_conversion) DEBUG_COUNTER (if_conversion_tree) DEBUG_COUNTER (ira_move) +DEBUG_COUNTER (merged_ipa_icf) DEBUG_COUNTER (local_alloc_for_sched) DEBUG_COUNTER (merged_ipa_icf) DEBUG_COUNTER (postreload_cse) diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c index 1fbdf6d..1237157 100644 --- a/gcc/ipa-icf.c +++ b/gcc/ipa-icf.c @@ -123,6 +123,7 @@ along with GCC; see the file COPYING3. If not see #include "ipa-icf-gimple.h" #include "ipa-icf.h" #include "stor-layout.h" +#include "dbgcnt.h" using namespace ipa_icf_gimple; @@ -3453,7 +3454,8 @@ sem_item_optimizer::merge_classes (unsigned int prev_class_count) alias->dump_to_file (dump_file); } - merged_p |= source->merge (alias); + if (dbg_cnt (merged_ipa_icf)) + merged_p |= source->merge (alias); } } -- 2.1.4
[PATCH 1/2] dbgcnt: enhance behavior.
Hi. Following patch changes GCC so that it exits after -fdbg-cnt-list and counters are sorted in dbgcnt.def file. Ready for trunk? Thanks, Martin gcc/ChangeLog: 2015-05-15 Martin Liska * dbgcnt.def: Sort counters. * opts.c (common_handle_option): Do not compile if -fdbg-cnt-list is enabled. --- gcc/dbgcnt.def | 27 ++- gcc/opts.c | 4 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/gcc/dbgcnt.def b/gcc/dbgcnt.def index 4b26e77..95f6b06 100644 --- a/gcc/dbgcnt.def +++ b/gcc/dbgcnt.def @@ -144,8 +144,8 @@ echo ubound: $ub DEBUG_COUNTER (auto_inc_dec) DEBUG_COUNTER (ccp) DEBUG_COUNTER (cfg_cleanup) -DEBUG_COUNTER (cse2_move2add) DEBUG_COUNTER (cprop) +DEBUG_COUNTER (cse2_move2add) DEBUG_COUNTER (dce) DEBUG_COUNTER (dce_fast) DEBUG_COUNTER (dce_ud) @@ -155,6 +155,7 @@ DEBUG_COUNTER (df_byte_scan) DEBUG_COUNTER (dse) DEBUG_COUNTER (dse1) DEBUG_COUNTER (dse2) +DEBUG_COUNTER (eipa_sra) DEBUG_COUNTER (gcse2_delete) DEBUG_COUNTER (global_alloc_at_func) DEBUG_COUNTER (global_alloc_at_reg) @@ -162,31 +163,31 @@ DEBUG_COUNTER (graphite_scop) DEBUG_COUNTER (hoist) DEBUG_COUNTER (hoist_insn) DEBUG_COUNTER (ia64_sched2) -DEBUG_COUNTER (if_conversion) -DEBUG_COUNTER (if_conversion_tree) DEBUG_COUNTER (if_after_combine) DEBUG_COUNTER (if_after_reload) +DEBUG_COUNTER (if_conversion) +DEBUG_COUNTER (if_conversion_tree) +DEBUG_COUNTER (ira_move) DEBUG_COUNTER (local_alloc_for_sched) +DEBUG_COUNTER (merged_ipa_icf) DEBUG_COUNTER (postreload_cse) DEBUG_COUNTER (pre) DEBUG_COUNTER (pre_insn) -DEBUG_COUNTER (treepre_insert) -DEBUG_COUNTER (tree_sra) -DEBUG_COUNTER (eipa_sra) -DEBUG_COUNTER (vect_loop) -DEBUG_COUNTER (vect_slp) +DEBUG_COUNTER (registered_jump_thread) DEBUG_COUNTER (sched2_func) DEBUG_COUNTER (sched_block) +DEBUG_COUNTER (sched_breakdep) DEBUG_COUNTER (sched_func) DEBUG_COUNTER (sched_insn) -DEBUG_COUNTER (sched_breakdep) DEBUG_COUNTER (sched_region) DEBUG_COUNTER (sel_sched_cnt) -DEBUG_COUNTER (sel_sched_region_cnt) DEBUG_COUNTER (sel_sched_insn_cnt) +DEBUG_COUNTER (sel_sched_region_cnt) DEBUG_COUNTER (sms_sched_loop) -DEBUG_COUNTER (store_motion) DEBUG_COUNTER (split_for_sched2) +DEBUG_COUNTER (store_motion) DEBUG_COUNTER (tail_call) -DEBUG_COUNTER (ira_move) -DEBUG_COUNTER (registered_jump_thread) +DEBUG_COUNTER (treepre_insert) +DEBUG_COUNTER (tree_sra) +DEBUG_COUNTER (vect_loop) +DEBUG_COUNTER (vect_slp) diff --git a/gcc/opts.c b/gcc/opts.c index fed5d12..a5461c5 100644 --- a/gcc/opts.c +++ b/gcc/opts.c @@ -1757,8 +1757,12 @@ common_handle_option (struct gcc_options *opts, break; case OPT_fdbg_cnt_: + /* Deferred. */ + break; + case OPT_fdbg_cnt_list: /* Deferred. */ + opts->x_exit_after_options = true; break; case OPT_fdebug_prefix_map_: -- 2.1.4
Re: [PATCH diagnostics/fortran] Handle two locations for the same diagnostic. Convert all gfc_warning_1 and gfc_notify_std_1 calls
On 15 May 2015 at 10:39, Dodji Seketeli wrote:
> Manuel López-Ibáñez writes:
>> -/* Expand the location of this diagnostic. Use this function for
>> consistency. */
>> +/* Return the location associated to this diagnostic. WHICH specifies
>
> Here, I think only the 'W' (in WHICH) should be uppercase.
I'm following the convention that parameter names are uppercase in the
description of functions. I can also say "Parameter WHICH specifies"
to be more explicit.
>> /* The type of a text to be formatted according a format specification
>> along with a list of things. */
>> struct text_info
>> {
>> +public:
>
> As this is a struct, the 'public' here is not necessary, as the members
> are public by default.
I have a very poor memory for such details ;), since we are using
'private:' already, does it really hurt to be explicit and use
'public:' here?
>>const char *format_spec;
>>va_list *args_ptr;
>>int err_no; /* for %m */
>> - location_t *locus;
>>void **x_data;
>> +
>> + inline location_t & set_location (unsigned int index_of_location)
>
> I think it's less surprising to have the this function take two
> parameters: The index_of_location and the new new location.
OK.
> OK to commit with the changes above.
Thanks. If you insist on the two first changes, I'll do them, I just
want to confirm those.
Cheers,
Manuel.
Re: [PATCH] rs6000: Improve rtx_costs for EQ a bit (PR30967)
On Thu, May 14, 2015 at 09:51:18AM -0400, David Edelsohn wrote: > On Wed, May 13, 2015 at 1:29 PM, Segher Boessenkool > wrote: > > On Wed, May 13, 2015 at 01:25:16PM -0400, David Edelsohn wrote: > >> > PR rtl-optimization/30967 > >> > * config/rs6000/rs6000.c (rs6000_rtx_costs): Don't consider > >> > destination mode for the cost of scc patterns. > >> > >> Okay. > >> > >> Should this be applied to GCC 5 branch? > > > > [ Testing is still in progress. ] > > > > It should be prefectly safe to backport; OTOH, it is just one of very > > many cases where we generate an extra sign-extend, so I don't think it > > is particularly useful. Your choice :-) > > The other prereqs are backported, so probably should backport this also. The only related patch that could be backported (the compare imm thing) is not backported yet AFAIK (I haven't checked); should it be? This cost patch improves things with or without it. Segher
[PATCH, alpha]: Fix PR66140, ICE at extract_insn, at recog.c
Hello!
As shown in the PR [1], reload can also schedule a replacement inside
memory address. Attached patch removes invalid check.
2015-05-15 Uros Bizjak
PR target/66140
* config/alpha/alpha.c (get_aligned_mem): Also look for reload
replacements in memory addresses.
(get_unaligned_address): Ditto.
testsuite/ChangeLog:
2015-05-15 Uros Bizjak
PR target/66140
* gcc.target/alpha/pr66140.c: New test.
Patch was bootstrapped and regression tested on alpha-linux-gnu.
OK for mainline and release branches?
[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66140
Uros.
Index: testsuite/gcc.target/alpha/pr66140.c
===
--- testsuite/gcc.target/alpha/pr66140.c(revision 0)
+++ testsuite/gcc.target/alpha/pr66140.c(revision 0)
@@ -0,0 +1,53 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -mcpu=ev4" } */
+
+struct scsi_cmnd {
+ int sc_data_direction;
+};
+struct lpfc_hba {
+ unsigned cfg_total_seg_cnt;
+};
+struct lpfc_scsi_buf {
+ struct scsi_cmnd *pCmd;
+ unsigned seg_cnt;
+ unsigned *fcp_bpl;
+};
+
+extern void *sg_next(void *sg);
+extern void *scsi_sglist(struct scsi_cmnd *cmd);
+extern unsigned scsi_sg_count(struct scsi_cmnd *cmd);
+
+static inline void dma_map_sg_attrs(void *sg, int nents, int dir)
+{
+ int i;
+
+ for (i = 0; i < nents; i++, sg = sg_next(sg))
+ ;
+
+ if (!dir)
+ asm volatile( "call_pal %0" : : "i"(129));
+}
+
+static inline void lpfc_bg_setup_bpl(struct lpfc_hba *phba, struct scsi_cmnd
*sc,
+unsigned *pde5)
+{
+ void *sgde;
+ int i;
+
+ *pde5 = (((0x85 & 0x00ff) << 24) | (*pde5 & ~(0x00ff << 24)));
+ for (i = 0, sgde = scsi_sglist(sc); i < 2; i++, sgde = sg_next(sgde))
+ ;
+}
+
+void lpfc_bg_scsi_prep_dma_buf_s3(struct lpfc_hba *phba,
+ struct lpfc_scsi_buf *lpfc_cmd)
+{
+ struct scsi_cmnd *scsi_cmnd = lpfc_cmd->pCmd;
+ unsigned *bpl = lpfc_cmd->fcp_bpl;
+
+ dma_map_sg_attrs(scsi_sglist(scsi_cmnd),
+scsi_sg_count(scsi_cmnd),
+scsi_cmnd->sc_data_direction);
+ if (lpfc_cmd->seg_cnt > phba->cfg_total_seg_cnt)
+ lpfc_bg_setup_bpl(phba, scsi_cmnd, bpl);
+}
Index: config/alpha/alpha.c
===
--- config/alpha/alpha.c(revision 223211)
+++ config/alpha/alpha.c(working copy)
@@ -1556,8 +1556,7 @@ get_aligned_mem (rtx ref, rtx *paligned_mem, rtx *
gcc_assert (MEM_P (ref));
- if (reload_in_progress
- && ! memory_address_p (GET_MODE (ref), XEXP (ref, 0)))
+ if (reload_in_progress)
{
base = find_replacement (&XEXP (ref, 0));
gcc_assert (memory_address_p (GET_MODE (ref), base));
@@ -1602,11 +1601,9 @@ get_unaligned_address (rtx ref)
gcc_assert (MEM_P (ref));
- if (reload_in_progress
- && ! memory_address_p (GET_MODE (ref), XEXP (ref, 0)))
+ if (reload_in_progress)
{
base = find_replacement (&XEXP (ref, 0));
-
gcc_assert (memory_address_p (GET_MODE (ref), base));
}
else
Re: [PATCH 3/13] aarch64 musl support
On 27/04/15 15:33, Szabolcs Nagy wrote: > > On 21/04/15 15:16, pins...@gmail.com wrote: >> >> I don't think you need to check if defaulting to little or big-endian here >> are the specs always have one or the other passing through. >> >> Also if musl does not support ilp32, you might want to error out. Or even >> define the dynamic linker name even before support goes into musl. >> > > Patch v2. > > (now _ilp32 abi variant is included too) > ping (now with maintainer in cc) > gcc/Changelog: > > 2015-04-27 Gregor Richards > Szabolcs Nagy > > * config/aarch64/aarch64-linux.h (MUSL_DYNAMIC_LINKER): Define. >
Re: [PATCH 9/13] x86 musl support
On 27/04/15 15:52, Szabolcs Nagy wrote: > On 20/04/15 19:57, Szabolcs Nagy wrote: >> Set up dynamic linker name for x86. >> > Patch v2. > > (undef MUSL_DYNAMIC_LINKER* before defining it). ping (now with maintainers in cc) > gcc/Changelog: > > 2015-04-27 Gregor Richards > > * config/i386/linux.h (MUSL_DYNAMIC_LINKER): Define. > * config/i386/linux64.h (MUSL_DYNAMIC_LINKER32): Define. > (MUSL_DYNAMIC_LINKER64, MUSL_DYNAMIC_LINKERX32): Define. >
Re: [PATCH 13/13] fix incompatible posix_memalign declaration on x86
On 20/04/15 20:00, Szabolcs Nagy wrote: > The posix_memalign declaration is incompatible with musl for C++, > because of the exception specification. It also pollutes the > namespace and lacks protection against a potential macro definition > that is allowed by POSIX. The fix avoids source level namespace > pollution but retains the dependency on the posix_memalign extern > libc symbol. > > The fix is ugly, but it is not possible to correctly redeclare a > libc function in a public gcc header for C++. > ping (now with maintainers in cc) > > gcc/Changelog: > > 2015-04-16 Szabolcs Nagy > > * config/i386/pmm_malloc.h (posix_memalign): Renamed to ... > (__gcc_posix_memalign): This. Use posix_memalign as extern > symbol only. >
Re: [PATCH 4/13] arm musl support
On 20/04/15 19:53, Szabolcs Nagy wrote: > Set up dynamic linker name for arm. > ping (now with maintainer in cc) > gcc/Changelog: > > 2015-04-16 Gregor Richards > > * config/arm/linux-eabi.h (MUSL_DYNAMIC_LINKER): Define. >
Re: [PATCH 9/13] x86 musl support
On Fri, May 15, 2015 at 2:33 PM, Szabolcs Nagy wrote: > On 27/04/15 15:52, Szabolcs Nagy wrote: >> On 20/04/15 19:57, Szabolcs Nagy wrote: >>> Set up dynamic linker name for x86. >>> >> Patch v2. >> >> (undef MUSL_DYNAMIC_LINKER* before defining it). > > ping > > (now with maintainers in cc) > >> gcc/Changelog: >> >> 2015-04-27 Gregor Richards >> >> * config/i386/linux.h (MUSL_DYNAMIC_LINKER): Define. >> * config/i386/linux64.h (MUSL_DYNAMIC_LINKER32): Define. >> (MUSL_DYNAMIC_LINKER64, MUSL_DYNAMIC_LINKERX32): Define. OK with a small nit below. diff --git a/gcc/config/i386/linux.h b/gcc/config/i386/linux.h index a100963..9c47fcb 100644 --- a/gcc/config/i386/linux.h +++ b/gcc/config/i386/linux.h @@ -21,3 +21,5 @@ along with GCC; see the file COPYING3. If not see #define GNU_USER_LINK_EMULATION "elf_i386" #define GLIBC_DYNAMIC_LINKER "/lib/ld-linux.so.2" Please add vertical space here. +#undef MUSL_DYNAMIC_LINKER +#define MUSL_DYNAMIC_LINKER "/lib/ld-musl-i386.so.1" Thanks, Uros.
Re: [PATCH 13/13] fix incompatible posix_memalign declaration on x86
On Fri, May 15, 2015 at 2:34 PM, Szabolcs Nagy wrote: > On 20/04/15 20:00, Szabolcs Nagy wrote: >> The posix_memalign declaration is incompatible with musl for C++, >> because of the exception specification. It also pollutes the >> namespace and lacks protection against a potential macro definition >> that is allowed by POSIX. The fix avoids source level namespace >> pollution but retains the dependency on the posix_memalign extern >> libc symbol. >> >> The fix is ugly, but it is not possible to correctly redeclare a >> libc function in a public gcc header for C++. >> > > ping > > (now with maintainers in cc) > >> >> gcc/Changelog: >> >> 2015-04-16 Szabolcs Nagy >> >> * config/i386/pmm_malloc.h (posix_memalign): Renamed to ... >> (__gcc_posix_memalign): This. Use posix_memalign as extern >> symbol only. This changes Intel's header - adding relevant people to OK the change. Uros.
Re: [PATCH] [PR libitm/61164] Remove redefinition of glibc internal macro __always_inline
On Wed, 2015-05-06 at 17:54 +0300, Gleb Fotengauer-Malinovskiy wrote: > 2015-05-06 Gleb Fotengauer-Malinovskiy > > PR libitm/61164 > * local_atomic (__always_inline): Rename to... > (__libitm_always_inline): ... this. OK. Thanks.
Re: [PATCH] Fix PR ipa/65908.
On 05/15/2015 12:38 PM, Jakub Jelinek wrote:
On Fri, May 15, 2015 at 10:38:57AM +0200, Martin Liška wrote:
Following patch is fix for GCC-5 branch for PR ipa/65908, was tested on
x86_64-linux-pc, as well as bootstrapped.
As soon as the patch is applied, I'm going to send the similar patch for trunk.
I'll leave the review to Honza or Richi, just a few nits:
1) it should go to the trunk first, then to GCC-5 branch
--- a/gcc/ipa-icf.c
+++ b/gcc/ipa-icf.c
@@ -417,6 +417,33 @@ bool sem_function::compare_edge_flags (cgraph_edge *e1,
cgraph_edge *e2)
return true;
}
+/* Return true if DECL_ARGUMENT types are valid to be merged. */
Please add another newline between } and the comment.
--- a/gcc/ipa-icf.h
+++ b/gcc/ipa-icf.h
@@ -364,6 +364,9 @@ private:
bool equals_private (sem_item *item,
hash_map &ignored_nodes);
+ /* Return true if DECL_ARGUMENT types are valid to be merged. */
+ bool compatible_parm_types_p ();
+
/* Returns true if tree T can be compared as a handled component. */
static bool icf_handled_component_p (tree t);
And here should be an empty line above the Return true if ... comment
too. Though, I wonder how this can apply cleanly to the current 5 branch,
because there is an empty newline in between equals_private and the comment
above icf_handled_component_p.
diff --git a/gcc/testsuite/g++.dg/ipa/pr65908.C
b/gcc/testsuite/g++.dg/ipa/pr65908.C
new file mode 100644
index 000..c1884ab
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ipa/pr65908.C
@@ -0,0 +1,26 @@
+// PR ipa/65908
+// { dg-options "-O2 -fPIC" }
+// { dg-do compile { target { { fpic } } } }
Perhaps
// PR ipa/65908
// { dg-do compile }
// { dg-options "-O2" }
// { dg-additional-options "-fPIC" { target fpic } }
instead?
Jakub
Hi.
Thanks for all nits. I've just bootstrapped reapplied version of the patch,
based on
current trunk with respect to all changes observed by Jakub.
Patch can also survive regression tests.
What do you think about it Honza?
Thanks,
Martin
>From 22ee6b7aaae73db83663e8b5c12054b2f126a53e Mon Sep 17 00:00:00 2001
From: mliska
Date: Fri, 15 May 2015 13:23:33 +0200
Subject: [PATCH] Fix PR ipa/65908.
gcc/testsuite/ChangeLog:
2015-05-12 Martin Liska
* g++.dg/ipa/pr65908.C: New test.
gcc/ChangeLog:
2015-05-12 Martin Liska
PR ipa/65908
* ipa-icf.c (sem_function::equals_private): Always compare argument
type got from TYPE_ARG_TYPES.
(sem_function::compatible_parm_types_p): New function.
(sem_function::equals_wpa): Use the function.
(sem_function::equals_private): Likewise.
(sem_function::parse_tree_args): Handle case where we have a different
number of arguments.
* ipa-icf.h (sem_function::compatible_parm_types_p): Declare new
function.
---
gcc/ipa-icf.c | 78 +-
gcc/ipa-icf.h | 3 ++
gcc/testsuite/g++.dg/ipa/pr65908.C | 27 +
3 files changed, 74 insertions(+), 34 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/ipa/pr65908.C
diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
index 3c4ac05..5d1881e 100644
--- a/gcc/ipa-icf.c
+++ b/gcc/ipa-icf.c
@@ -594,6 +594,38 @@ sem_function::param_used_p (unsigned int i)
return ipa_is_param_used (IPA_NODE_REF (get_node ()), i);
}
+/* Return true if DECL_ARGUMENT types are valid to be merged. */
+
+bool
+sem_function::compatible_parm_types_p ()
+{
+ tree parm1, parm2;
+ unsigned i = 0;
+
+ for (parm1 = DECL_ARGUMENTS (decl),
+ parm2 = DECL_ARGUMENTS (m_compared_func->decl);
+ parm1 && parm2;
+ parm1 = DECL_CHAIN (parm1), parm2 = DECL_CHAIN (parm2), i++)
+ {
+if (!param_used_p (i))
+ continue;
+
+if (POINTER_TYPE_P (parm1)
+ && (TYPE_RESTRICT (parm1) != TYPE_RESTRICT (parm2)))
+ return return_false_with_msg ("argument restrict flag mismatch");
+/* nonnull_arg_p implies non-zero range to REFERENCE types. */
+if (POINTER_TYPE_P (parm1)
+ && TREE_CODE (parm1) != TREE_CODE (parm2)
+ && opt_for_fn (decl, flag_delete_null_pointer_checks))
+ return return_false_with_msg ("pointer wrt reference mismatch");
+ }
+
+ if (parm1 || parm2)
+return return_false ();
+
+ return true;
+}
+
/* Fast equality function based on knowledge known in WPA. */
bool
@@ -713,22 +745,12 @@ sem_function::equals_wpa (sem_item *item,
if (!func_checker::compatible_types_p (arg_types[i],
m_compared_func->arg_types[i]))
return return_false_with_msg ("argument type is different");
-
- /* On used arguments we need to do a bit more of work. */
- if (!param_used_p (i))
- continue;
- if (POINTER_TYPE_P (arg_types[i])
- && (TYPE_RESTRICT (arg_types[i])
- != TYPE_RESTRICT (m_compared_func->arg_types[i])))
- return return_false_with_msg ("argument restrict flag mismatch");
- /* nonnull_arg_p implies non-zero range to REFERENCE types. */
- if (POINTER_TYPE_P (arg_types[i])
- && TREE_CODE (arg_types[i])
- != TREE_C
Re: [PATCH 9/13] x86 musl support
On 15/05/15 13:43, Uros Bizjak wrote: On Fri, May 15, 2015 at 2:33 PM, Szabolcs Nagy wrote: On 27/04/15 15:52, Szabolcs Nagy wrote: On 20/04/15 19:57, Szabolcs Nagy wrote: Set up dynamic linker name for x86. Patch v2. (undef MUSL_DYNAMIC_LINKER* before defining it). ping (now with maintainers in cc) gcc/Changelog: 2015-04-27 Gregor Richards * config/i386/linux.h (MUSL_DYNAMIC_LINKER): Define. * config/i386/linux64.h (MUSL_DYNAMIC_LINKER32): Define. (MUSL_DYNAMIC_LINKER64, MUSL_DYNAMIC_LINKERX32): Define. OK with a small nit below. diff --git a/gcc/config/i386/linux.h b/gcc/config/i386/linux.h index a100963..9c47fcb 100644 --- a/gcc/config/i386/linux.h +++ b/gcc/config/i386/linux.h @@ -21,3 +21,5 @@ along with GCC; see the file COPYING3. If not see #define GNU_USER_LINK_EMULATION "elf_i386" #define GLIBC_DYNAMIC_LINKER "/lib/ld-linux.so.2" Please add vertical space here. +#undef MUSL_DYNAMIC_LINKER +#define MUSL_DYNAMIC_LINKER "/lib/ld-musl-i386.so.1" Hi all, I've committed the patch for Szabolcs with r223218. I'm attaching the committed patch with the whitespace change that he provided. Cheers, Kyrill Thanks, Uros. diff --git a/gcc/config/i386/linux.h b/gcc/config/i386/linux.h index a100963..385aefd 100644 --- a/gcc/config/i386/linux.h +++ b/gcc/config/i386/linux.h @@ -21,3 +21,6 @@ along with GCC; see the file COPYING3. If not see #define GNU_USER_LINK_EMULATION "elf_i386" #define GLIBC_DYNAMIC_LINKER "/lib/ld-linux.so.2" + +#undef MUSL_DYNAMIC_LINKER +#define MUSL_DYNAMIC_LINKER "/lib/ld-musl-i386.so.1" diff --git a/gcc/config/i386/linux64.h b/gcc/config/i386/linux64.h index a27d3be..e300480 100644 --- a/gcc/config/i386/linux64.h +++ b/gcc/config/i386/linux64.h @@ -30,3 +30,10 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see #define GLIBC_DYNAMIC_LINKER32 "/lib/ld-linux.so.2" #define GLIBC_DYNAMIC_LINKER64 "/lib64/ld-linux-x86-64.so.2" #define GLIBC_DYNAMIC_LINKERX32 "/libx32/ld-linux-x32.so.2" + +#undef MUSL_DYNAMIC_LINKER32 +#define MUSL_DYNAMIC_LINKER32 "/lib/ld-musl-i386.so.1" +#undef MUSL_DYNAMIC_LINKER64 +#define MUSL_DYNAMIC_LINKER64 "/lib/ld-musl-x86_64.so.1" +#undef MUSL_DYNAMIC_LINKERX32 +#define MUSL_DYNAMIC_LINKERX32 "/lib/ld-musl-x32.so.1"
Re: [RFC, PATCH] nonzero attribute, static array parameter
On Sat, May 09, 2015 at 09:42:23AM -0700, Martin Uecker wrote:
> here is a tentative patch to implement a new attribute nonzero,
> which is similar to nonnull, but is not a function attribute
> but a type attribute.
>
> One reason is that nonnull is awkward to use. For this reason,
> clang allows the use of nonnull in function parameters, but this
> is incompatible with old and current use of this attribute in gcc
> (though in a rather obscure use case).
> See: https://gcc.gnu.org/ml/gcc/2015-05/msg00077.html
Sorry, I quite fail to see how such an attribute would be useful.
It seems that the nonzero warning can only ever trigger when used
on function parameters or on a return value, much as the nonnull /
returns_nonnull attributes. The difference is that you can use
the nonzero attribute on a particular function parameter, but the
nonnull attribute has this ability as well:
__attribute__ ((nonnull (1))) void foo (int *, int *);
void
bar (void)
{
foo (0, 0);
}
Unlike nonnull, nonzero attribute can be attached to a typedef, but
it doesn't seem to buy you anything you couldn't do with the nonnull /
returns_nonnull attributes.
The nonzero attribute can appertain even to integer types, not only
pointer types. Can you give an example where this would come in handy?
It doesn't seem too useful to me.
+void foo1(int x[__attribute__((nonzero))]);
This looks weird, the placement of the nonzero attribute here suggests
that the array should have at least zero elements (the same that
int x[static 0] does), but in fact it checks that the pointer passed
to foo1 is non-NULL, i.e. something that could be easily achieved
with the nonnull attribute.
> The other reason is that a nonzero type attribute is conceptually
> much simpler and at the same time more general than the existing
> nonnull and nonnull_return function attributes (and could replace
> both), e.g. we can define non-zero types and diagnose all stores
> of known constant 0 to variables of this type, use this for
> computing better value ranges, etc.
Why would that be useful? What makes integer 0 special?
Marek
Re: [PATCH, alpha]: Fix PR66140, ICE at extract_insn, at recog.c
On 05/15/2015 05:26 AM, Uros Bizjak wrote: > 2015-05-15 Uros Bizjak > > PR target/66140 > * config/alpha/alpha.c (get_aligned_mem): Also look for reload > replacements in memory addresses. > (get_unaligned_address): Ditto. Ok. r~
[PATCH] fortran/64925 -- fix conflict between names
I've had the attached patch in my tree for a
long and have had no issues. Regression tested
numerous times on trunk. OK to commit?
2015-05-XX Steven G. Kargl
PR fortran/64925
* symbol.c(check_conflict): Check for a conflict between a dummy
argument and an internal procedure name.
2015-05-XX Steven G. Kargl
PR fortran/64925
* gfortran.dg/pr64925.f90: New test.
--
Steve
Index: gcc/fortran/symbol.c
===
--- gcc/fortran/symbol.c (revision 223094)
+++ gcc/fortran/symbol.c (working copy)
@@ -458,6 +458,11 @@ check_conflict (symbol_attribute *attr,
}
}
+ if (attr->dummy && ((attr->function || attr->subroutine) &&
+ gfc_current_state () == COMP_CONTAINS))
+gfc_error_now ("internal procedure '%s' at %L conflicts with "
+ "DUMMY argument", name, where);
+
conf (dummy, entry);
conf (dummy, intrinsic);
conf (dummy, threadprivate);
Index: gcc/testsuite/gfortran.dg/pr64925.f90
===
--- gcc/testsuite/gfortran.dg/pr64925.f90 (revision 0)
+++ gcc/testsuite/gfortran.dg/pr64925.f90 (working copy)
@@ -0,0 +1,22 @@
+! { dg-do compile }
+! PR fortran/64925
+! Original test case provided by Bill Long
+!
+subroutine foo(nnn, aaa, bbb, ccc, ddd)
+ implicit none
+ integer :: nnn, aaa, bbb(nnn)
+ integer :: i
+ do i=1,nnn
+ aaa = aaa + bbb(ccc(i))
+ end do
+ call ddd(aaa)
+contains
+ integer function ccc(i) ! { dg-error "conflicts with DUMMY" }
+integer :: i
+ccc = i
+ end function ccc
+ subroutine ddd(j)! { dg-error "conflicts with DUMMY" }
+integer j
+j = j + 1
+ end subroutine ddd
+end subroutine foo
[PATCH] fortran/66039 -- Check for mangled file positioning
The attached patch checks for mangled/incomplete
file positioning statements. Regression tested
on trunk with no new failures? OK to commit
2015-05-XX Steven G. Kargl
PR fortran/66039
* io.c (match_filepos): Check for incomplete/mangled REWIND, FLUSH,
BACKSPACE, and ENDFILE statements
2015-05-XX Steven G. Kargl
PR fortran/66039
* gfortran.dg/filepos1.f90: New test
--
Steve
Index: gcc/fortran/io.c
===
--- gcc/fortran/io.c (revision 223094)
+++ gcc/fortran/io.c (working copy)
@@ -2382,9 +2382,7 @@ match_filepos (gfc_statement st, gfc_exe
if (m == MATCH_NO)
{
m = gfc_match_expr (&fp->unit);
- if (m == MATCH_ERROR)
- goto done;
- if (m == MATCH_NO)
+ if (m == MATCH_ERROR || m == MATCH_NO)
goto syntax;
}
Index: gcc/testsuite/gfortran.dg/filepos1.f90
===
--- gcc/testsuite/gfortran.dg/filepos1.f90 (revision 0)
+++ gcc/testsuite/gfortran.dg/filepos1.f90 (working copy)
@@ -0,0 +1,25 @@
+! { dg-do compile }
+! PR fortran/66039
+!
+! Original code from Gerhard Steinmetz
+!
+subroutine p1
+ rewind (( ! { dg-error "Syntax error in REWIND" }
+ rewind (- ! { dg-error "Syntax error in REWIND" }
+end subroutine p1
+
+subroutine p2
+ flush (( ! { dg-error "Syntax error in FLUSH" }
+ flush (- ! { dg-error "Syntax error in FLUSH" }
+end subroutine p2
+
+subroutine p4
+ backspace (( ! { dg-error "Syntax error in BACKSPACE" }
+ backspace (- ! { dg-error "Syntax error in BACKSPACE" }
+end subroutine p4
+
+subroutine p3
+ endfile (( ! { dg-error "Expecting END SUBROUTINE" }
+ endfile (- ! { dg-error "Expecting END SUBROUTINE" }
+end subroutine p3
+
[PATCH] fortran/66040 -- convert internal error into normal error
The attached patch has been regression tested on
trunk. OK to commit?
As shown by the testcase, each individual line of code
is legal, but the ordering is incorrect. This leads to
an ICE, which can be correctly reported as a user error.
2015-05-XX Steven G. Kargl
PR fortran/66040
* parse.c(verify_st_order): Replace a gfc_internal_error with your
generic gfc_error.
2015-05-XX Steven G. Kargl
PR fortran/66040
* gfortran.dg/misplaced_statement.f90: New test.
--
Steve
Index: gcc/fortran/parse.c
===
--- gcc/fortran/parse.c (revision 223094)
+++ gcc/fortran/parse.c (working copy)
@@ -2425,8 +2425,7 @@ verify_st_order (st_state *p, gfc_statem
break;
default:
- gfc_internal_error ("Unexpected %s statement in verify_st_order() at %C",
- gfc_ascii_statement (st));
+ return false;
}
/* All is well, record the statement in case we need it next time. */
Index: gcc/testsuite/gfortran.dg/misplaced_statement.f90
===
--- gcc/testsuite/gfortran.dg/misplaced_statement.f90 (revision 0)
+++ gcc/testsuite/gfortran.dg/misplaced_statement.f90 (working copy)
@@ -0,0 +1,20 @@
+!{ dg-do compile }
+! PR fortran/66040
+!
+! Original code from Gerhard Steinmetz
+!
+real function f1(x)
+ sequence ! { dg-error "Unexpected SEQUENCE statement" }
+end function f1
+
+real function f2()
+ else ! { dg-error "Unexpected ELSE statement" }
+end function f2
+
+real function f3()
+ block data! { dg-error "Unexpected BLOCK DATA statement" }
+end function f3
+
+real function f4()
+ program p ! { dg-error "Unexpected PROGRAM statement" }
+end function f4
[PATCH] fortran/66043 -- Check for NULL() in STORAGE_SIZE()
Regression tested on trunk. OK to commit?
The patch shuold be self-explanatory.
2015-05-XX Steven G. Kargl
PR fortran/66043
* check.c (gfc_check_storage_size): Prevent the direct use of NULL()
in STORAGE_SIZE() reference.
2015-05-XX Steven G. Kargl
PR fortran/66043
* gfortran.dg/storage_size_6.f90: New tests.
--
Steve
Index: gcc/fortran/check.c
===
--- gcc/fortran/check.c (revision 223094)
+++ gcc/fortran/check.c (working copy)
@@ -6243,6 +6243,17 @@ gfc_check_and (gfc_expr *i, gfc_expr *j)
bool
gfc_check_storage_size (gfc_expr *a, gfc_expr *kind)
{
+
+ if (a->expr_type == EXPR_NULL)
+{
+ gfc_error ("%qs argument of %qs intrinsic at %L shall not be an "
+ "unallocated allocatable variable or a disassociated or "
+ "undefined pointer",
+ gfc_current_intrinsic_arg[0]->name, gfc_current_intrinsic,
+ &a->where);
+ return false;
+}
+
if (a->ts.type == BT_ASSUMED)
{
gfc_error ("%qs argument of %qs intrinsic at %L shall not be TYPE(*)",
Index: gcc/testsuite/gfortran.dg/storage_size_6.f90
===
--- gcc/testsuite/gfortran.dg/storage_size_6.f90 (revision 0)
+++ gcc/testsuite/gfortran.dg/storage_size_6.f90 (working copy)
@@ -0,0 +1,8 @@
+! { dg-do compile }
+! PR fortran/66043
+!
+! Original code from Gerhard Steinmetz
+!
+program p
+ print *, storage_size(null()) ! { dg-error "shall not be an unallocated" }
+end
[PATCH] fortran/66044 -- Convert ICE into ordinary error
Regression tested on trunk. OK to commit?
Poorly written code could invoke an ICE when an ordinary
error could have been emitted.
2015-05-XX Steven G. Kargl
PR fortran/66044
* decl.c(gfc_match_entry): Change a gfc_internal_error() into
a gfc_error()
2015-05-XX Steven G. Kargl
PR fortran/66044
* gfortran.dg/entry_21.f90: New test.
--
Steve
Index: gcc/fortran/decl.c
===
--- gcc/fortran/decl.c (revision 223094)
+++ gcc/fortran/decl.c (working copy)
@@ -5592,7 +5592,7 @@ gfc_match_entry (void)
"a contained subprogram");
break;
default:
- gfc_internal_error ("gfc_match_entry(): Bad state");
+ gfc_error ("Unexpected ENTRY statement at %C");
}
return MATCH_ERROR;
}
Index: gcc/testsuite/gfortran.dg/entry_21.f90
===
--- gcc/testsuite/gfortran.dg/entry_21.f90 (revision 0)
+++ gcc/testsuite/gfortran.dg/entry_21.f90 (working copy)
@@ -0,0 +1,17 @@
+! { dg-do compile }
+! PR fortran/66044
+!
+! Original code from Gerhard Steinmetz
+!
+subroutine p
+end subroutine p
+
+entry e! { dg-error "Unexpected ENTRY statement" }
+end
+
+module m
+ type t
+ contains
+ entry e ! { dg-error "Unexpected ENTRY statement" }
+ end type
+end module m
Re: [PATCH 13/13] fix incompatible posix_memalign declaration on x86
On Mon, Apr 20, 2015 at 12:00 PM, Szabolcs Nagy wrote: > The posix_memalign declaration is incompatible with musl for C++, > because of the exception specification. It also pollutes the > namespace and lacks protection against a potential macro definition > that is allowed by POSIX. The fix avoids source level namespace > pollution but retains the dependency on the posix_memalign extern > libc symbol. > > The fix is ugly, but it is not possible to correctly redeclare a > libc function in a public gcc header for C++. > > > gcc/Changelog: > > 2015-04-16 Szabolcs Nagy > > * config/i386/pmm_malloc.h (posix_memalign): Renamed to ... > (__gcc_posix_memalign): This. Use posix_memalign as extern > symbol only. What does this try to achieve? Do you have a testcase which fails before and passes with this patch? -- H.J.
[PATCH] fortran/66045 -- NULL() on RHS of assignment
Regression tested on trunk. OK to commit?
The patch is self-explanatory.
Steven G. Kargl
PR fortran/66045
* expr.c (gfc_check_assign): Check for assignment of NULL() instead
of the (intended) pointer assignment.
2015-05-XX Steven G. Kargl
PR fortran/66045
* gfortran.dg/null1.f90: New test.
--
Steve
Index: gcc/fortran/expr.c
===
--- gcc/fortran/expr.c (revision 223094)
+++ gcc/fortran/expr.c (working copy)
@@ -3118,19 +3118,22 @@ gfc_check_assign (gfc_expr *lvalue, gfc_
bad_proc = true;
/* (ii) The assignment is in the main program; or */
- if (gfc_current_ns->proc_name->attr.is_main_program)
+ if (gfc_current_ns->proc_name
+ && gfc_current_ns->proc_name->attr.is_main_program)
bad_proc = true;
/* (iii) A module or internal procedure... */
- if ((gfc_current_ns->proc_name->attr.proc == PROC_INTERNAL
- || gfc_current_ns->proc_name->attr.proc == PROC_MODULE)
+ if (gfc_current_ns->proc_name
+ && (gfc_current_ns->proc_name->attr.proc == PROC_INTERNAL
+ || gfc_current_ns->proc_name->attr.proc == PROC_MODULE)
&& gfc_current_ns->parent
&& (!(gfc_current_ns->parent->proc_name->attr.function
|| gfc_current_ns->parent->proc_name->attr.subroutine)
|| gfc_current_ns->parent->proc_name->attr.is_main_program))
{
/* ... that is not a function... */
- if (!gfc_current_ns->proc_name->attr.function)
+ if (gfc_current_ns->proc_name
+ && !gfc_current_ns->proc_name->attr.function)
bad_proc = true;
/* ... or is not an entry and has a different name. */
Index: gcc/testsuite/gfortran.dg/null1.f90
===
--- gcc/testsuite/gfortran.dg/null1.f90 (revision 0)
+++ gcc/testsuite/gfortran.dg/null1.f90 (working copy)
@@ -0,0 +1,9 @@
+! { dg-do compile }
+! PR fortran/66045
+!
+! Original code from Gerhard Steinmetz
+!
+program p
+ contains
+ integer :: null=null() ! { dg-error "NULL appears on right-hand side" }
+end
[PATCH] fortran/66057 -- detect malformed GENERIC statement
Regression tested on trunk. OK to commit?
See the test case for the type of error that this patch
catches.
2015-05-XX Steven G. Kargl
PR fortran/66057
* decl.c(gfc_match_generic): Detected a malformed GENERIC statement.
2015-05-XX Steven G. Kargl
PR fortran/66057
* gfortran.dg/generic_29.f90: New tests.
--
Steve
Index: gcc/fortran/decl.c
===
--- gcc/fortran/decl.c (revision 223094)
+++ gcc/fortran/decl.c (working copy)
@@ -8510,6 +8511,11 @@ gfc_match_generic (void)
gfc_op2string (op));
break;
+case INTERFACE_NAMELESS:
+ gfc_error ("Malformed GENERIC statement at %C");
+ goto error;
+ break;
+
default:
gcc_unreachable ();
}
Index: gcc/testsuite/gfortran.dg/generic_29.f90
===
--- gcc/testsuite/gfortran.dg/generic_29.f90 (revision 0)
+++ gcc/testsuite/gfortran.dg/generic_29.f90 (working copy)
@@ -0,0 +1,11 @@
+! { dg-do compile}
+! PR fortran/66057
+!
+! Original code from Gerhard Steinmetz
+!
+program p
+ type t
+ contains
+ generic :: ! { dg-error "Malformed GENERIC" }
+ end type
+end
[PATCH] fortran/66106 -- Enforce C1202 from F2008
Regression tested on trunk. OK to commit?
The patch allows gfortran to check a portion of C1202
that was previously mishandled.
2015-05-XX Steven G. Kargl
PR fortran/66057
* interface.c(gfc_match_end_interface): Enfornce F2008 C1202 (R1201).
2015-05-XX Steven G. Kargl
PR fortran/66057
* gfortran.dg/interface_operator_1.f90: New tests.
--
Steve
Index: gcc/fortran/interface.c
===
--- gcc/fortran/interface.c (revision 223094)
+++ gcc/fortran/interface.c (working copy)
@@ -346,8 +346,12 @@ gfc_match_end_interface (void)
break;
m = MATCH_ERROR;
- gfc_error ("Expecting % at %C, "
- "but got %s", s1, s2);
+ if (strcmp(s2, "none") == 0)
+ gfc_error ("Expecting % "
+ "at %C, ", s1);
+ else
+ gfc_error ("Expecting % at %C, "
+ "but got %s", s1, s2);
}
}
Index: gcc/testsuite/gfortran.dg/interface_operator_1.f90
===
--- gcc/testsuite/gfortran.dg/interface_operator_1.f90 (revision 0)
+++ gcc/testsuite/gfortran.dg/interface_operator_1.f90 (working copy)
@@ -0,0 +1,10 @@
+! { dg-do compile }
+! PR fortran/66106
+!
+! Original code from Gerhard Steinmetz
+!
+program p
+ interface operator ( .gt. )
+ end interface operator! { dg-error "END INTERFACE OPERATOR" }
+end program p! { dg-error "END INTERFACE" }
+! { dg-excess-errors "Unexpected end of file" }
[PATCH] fortran/66052 -- Prevent dereference of NULL pointer
Regression tested on trunk. OK to commit?
Subject says it all.
2015-05-XX Steven G. Kargl
PR fortran/66052
* decl.c(gfc_match_protected): Prevent dereference of NULL pointer.
2015-05-XX Steven G. Kargl
PR fortran/66052
* gfortran.dg/protected_9.f90: New test.
--
Steve
Index: gcc/fortran/decl.c
===
--- gcc/fortran/decl.c (revision 223094)
+++ gcc/fortran/decl.c (working copy)
@@ -6968,7 +6968,8 @@ gfc_match_protected (void)
gfc_symbol *sym;
match m;
- if (gfc_current_ns->proc_name->attr.flavor != FL_MODULE)
+ if (gfc_current_ns->proc_name
+ && gfc_current_ns->proc_name->attr.flavor != FL_MODULE)
{
gfc_error ("PROTECTED at %C only allowed in specification "
"part of a module");
Index: gcc/testsuite/gfortran.dg/protected_9.f90
===
--- gcc/testsuite/gfortran.dg/protected_9.f90 (revision 0)
+++ gcc/testsuite/gfortran.dg/protected_9.f90 (working copy)
@@ -0,0 +1,16 @@
+! { dg-do compile }
+! PR fortran/66052
+!
+!
+! Original code from Gerhard Steinmetz
+!
+module a
+ contains
+ protected x ! { dg-error "Unexpected attribute declaration statement" }
+end module a
+
+program p
+ contains
+ protected x ! { dg-error "Unexpected attribute declaration statement" }
+end
+
Re: [PATCH] fortran/66043 -- Check for NULL() in STORAGE_SIZE()
Hi Steve,
+ gfc_error ("%qs argument of %qs intrinsic at %L shall not be an "
+"unallocated allocatable variable or a disassociated or "
+"undefined pointer”,
Given that we know explicitly that the expr is NULL, wouldn’t it be nicer to
give only the relevant condition (here, I guess it’s “undefined pointer”)?
Regarding the other example mention in the PR’s comment #2, I guess there’s no
requirement for the compiler to diagnose this, is there?
FX
[PATCH v2 6/6] i386: Implement asm flag outputs
Version 2 includes proper test cases and documentation.
Hopefully the documentation even makes sense. Suggestions
and improvements there gratefully appreciated.
r~
---
gcc/config/i386/constraints.md | 5 ++
gcc/config/i386/i386.c | 137 +++--
gcc/doc/extend.texi| 76
gcc/testsuite/gcc.target/i386/asm-flag-0.c | 15
gcc/testsuite/gcc.target/i386/asm-flag-1.c | 18
gcc/testsuite/gcc.target/i386/asm-flag-2.c | 16
gcc/testsuite/gcc.target/i386/asm-flag-3.c | 22 +
gcc/testsuite/gcc.target/i386/asm-flag-4.c | 20 +
gcc/testsuite/gcc.target/i386/asm-flag-5.c | 19
9 files changed, 321 insertions(+), 7 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/i386/asm-flag-0.c
create mode 100644 gcc/testsuite/gcc.target/i386/asm-flag-1.c
create mode 100644 gcc/testsuite/gcc.target/i386/asm-flag-2.c
create mode 100644 gcc/testsuite/gcc.target/i386/asm-flag-3.c
create mode 100644 gcc/testsuite/gcc.target/i386/asm-flag-4.c
create mode 100644 gcc/testsuite/gcc.target/i386/asm-flag-5.c
diff --git a/gcc/config/i386/constraints.md b/gcc/config/i386/constraints.md
index 2271bd1..d16e728 100644
--- a/gcc/config/i386/constraints.md
+++ b/gcc/config/i386/constraints.md
@@ -146,10 +146,15 @@
"@internal Lower SSE register when avoiding REX prefix and all SSE registers
otherwise.")
;; We use the B prefix to denote any number of internal operands:
+;; f FLAGS_REG
;; s Sibcall memory operand, not valid for TARGET_X32
;; w Call memory operand, not valid for TARGET_X32
;; z Constant call address operand.
+(define_constraint "Bf"
+ "@internal Flags register operand."
+ (match_operand 0 "flags_reg_operand"))
+
(define_constraint "Bs"
"@internal Sibcall memory operand."
(and (not (match_test "TARGET_X32"))
diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index de333d8..868316a 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -45447,21 +45447,144 @@ ix86_c_mode_for_suffix (char suffix)
/* Worker function for TARGET_MD_ASM_ADJUST.
- We do this in the new i386 backend to maintain source compatibility
+ We implement asm flag outputs, and maintain source compatibility
with the old cc0-based compiler. */
static rtx_insn *
-ix86_md_asm_adjust (vec &/*outputs*/, vec &/*inputs*/,
- vec &/*constraints*/,
+ix86_md_asm_adjust (vec &outputs, vec &/*inputs*/,
+ vec &constraints,
vec &clobbers, HARD_REG_SET &clobbered_regs)
{
- clobbers.safe_push (gen_rtx_REG (CCmode, FLAGS_REG));
clobbers.safe_push (gen_rtx_REG (CCFPmode, FPSR_REG));
-
- SET_HARD_REG_BIT (clobbered_regs, FLAGS_REG);
SET_HARD_REG_BIT (clobbered_regs, FPSR_REG);
- return NULL;
+ bool saw_asm_flag = false;
+
+ start_sequence ();
+ for (unsigned i = 0, n = outputs.length (); i < n; ++i)
+{
+ const char *con = constraints[i];
+ if (strncmp (con, "=@cc", 4) != 0)
+ continue;
+ con += 4;
+ if (strchr (con, ',') != NULL)
+ {
+ error ("alternatives not allowed in asm flag output");
+ continue;
+ }
+
+ bool invert = false;
+ if (con[0] == 'n')
+ invert = true, con++;
+
+ machine_mode mode = CCmode;
+ rtx_code code = UNKNOWN;
+
+ switch (con[0])
+ {
+ case 'a':
+ if (con[1] == 0)
+ mode = CCAmode, code = EQ;
+ else if (con[1] == 'e' && con[2] == 0)
+ mode = CCCmode, code = EQ;
+ break;
+ case 'b':
+ if (con[1] == 0)
+ mode = CCCmode, code = EQ;
+ else if (con[1] == 'e' && con[2] == 0)
+ mode = CCAmode, code = NE;
+ break;
+ case 'c':
+ if (con[1] == 0)
+ mode = CCCmode, code = EQ;
+ break;
+ case 'e':
+ if (con[1] == 0)
+ mode = CCZmode, code = EQ;
+ break;
+ case 'g':
+ if (con[1] == 0)
+ mode = CCGCmode, code = GT;
+ else if (con[1] == 'e' && con[2] == 0)
+ mode = CCGCmode, code = GE;
+ break;
+ case 'l':
+ if (con[1] == 0)
+ mode = CCGCmode, code = LT;
+ else if (con[1] == 'e' && con[2] == 0)
+ mode = CCGCmode, code = LE;
+ break;
+ case 'o':
+ if (con[1] == 0)
+ mode = CCOmode, code = EQ;
+ break;
+ case 'p':
+ if (con[1] == 0)
+ mode = CCPmode, code = EQ;
+ break;
+ case 's':
+ if (con[1] == 0)
+ mode = CCSmode, code = EQ;
+ break;
+ case 'z':
+ if (con[1] == 0)
+ mode = CCZmode, code = EQ;
+ break;
+ }
+ if (code == UNKNOWN)
+ {
+ error ("unknown asm flag output %qs", constraints[i]);
+ continue;
+ }
+ if (invert)
+ code = reverse_condition (code);
+
+ rtx dest = outp
Re: [PATCH] fortran/66106 -- Enforce C1202 from F2008
- You’re missing the match.c part of the patch (I really had a hard time figuring out how the patch was supposed to work! then I read the one posted in the PR) - Let’s avoid the strcmp() string comparison and use “op == INTRINSIC_NONE” as the condition (seems clearer and more efficient to me) - Maybe not doing all the tests (those after "The following if-statements”) if op == INTRINSIC_NONE, as all the s2 comparisons will be false. - The PR number in the ChangeLog is wrong (both times) With the above fixed, it’s OK. Thanks for taking care of all this! FX
Re: [PATCH] fortran/66043 -- Check for NULL() in STORAGE_SIZE()
On Fri, May 15, 2015 at 05:34:14PM +0200, FX wrote:
> Hi Steve,
>
> + gfc_error ("%qs argument of %qs intrinsic at %L shall not be an "
> + "unallocated allocatable variable or a disassociated or "
> + "undefined pointer???,
>
> Given that we know explicitly that the expr is NULL, wouldn't
> it be nicer to give only the relevant condition (here, I guess
> it's "undefined pointer")?
The text above is essentially the wording from the standard.
I was unsure if I could catch all types of errors when I wrote
the error string.
I can change the error to something like
"Intrinsic function NULL() cannot be an actual argument to
STORAGE_SIZE because it returns a disassociated pointer"
>
> Regarding the other example mention in the PR's comment #2,
> I guess there's no requirement for the compiler to diagnose
> this, is there?
>
No. This isn't a numbered constraint. I spent an hour or so
tracing though parent namespaces and symbol trees to see if
I could catch the problem in comment #2. I think that this
would require a runtime error (or adding an internal attribute
that explicitly tracks associate/disassociation and
allocation/deallocation).
--
Steve
Re: [PATCH] fortran/66106 -- Enforce C1202 from F2008
On Fri, May 15, 2015 at 05:45:54PM +0200, FX wrote: > - You're missing the match.c part of the patch (I really had > a hard time figuring out how the patch was supposed to work! > then I read the one posted in the PR) Whoops, sorry about that. As you can see I have small patches sitting in my tree. I tried to untangle the 'svn diff' > - Let's avoid the strcmp() string comparison and use > 'op == INTRINSIC_NONE' as the condition (seems clearer and more > efficient to me) > - Maybe not doing all the tests (those after "The following > if-statements") if op == INTRINSIC_NONE, as all the s2 > comparisons will be false. > - The PR number in the ChangeLog is wrong (both times) Whoops copy-n-paste from one log to another. > With the above fixed, it???s OK. > Thanks for taking care of all this! I'll rework the patch to address your 2 suggestions. -- Steve
Re: [PATCH 13/13] fix incompatible posix_memalign declaration on x86
On 15/05/15 16:05, H.J. Lu wrote: > On Mon, Apr 20, 2015 at 12:00 PM, Szabolcs Nagy wrote: >> The posix_memalign declaration is incompatible with musl for C++, >> because of the exception specification. It also pollutes the >> namespace and lacks protection against a potential macro definition >> that is allowed by POSIX. The fix avoids source level namespace >> pollution but retains the dependency on the posix_memalign extern >> libc symbol. >> >> The fix is ugly, but it is not possible to correctly redeclare a >> libc function in a public gcc header for C++. >> >> >> gcc/Changelog: >> >> 2015-04-16 Szabolcs Nagy >> >> * config/i386/pmm_malloc.h (posix_memalign): Renamed to ... >> (__gcc_posix_memalign): This. Use posix_memalign as extern >> symbol only. > > What does this try to achieve? Do you have a testcase which > fails before and passes with this patch? if posix_memalign is defined in stdlib.h according to posix then the exception specifier used in mm_malloc.h is incompatible: $ echo "#include " | ./x86_64-linux-musl-g++ -pedantic -c -xc++ -D_POSIX_C_SOURCE=200809L - In file included from :1:0: /data/cross/x86_64-linux-musl/lib/gcc/x86_64-linux-musl/6.0.0/include/mm_malloc.h:34:64: error: declaration of 'int posix_memalign(void**, size_t, size_t) throw ()' has a different exception specifier extern "C" int posix_memalign (void **, size_t, size_t) throw () ^ In file included from /data/cross/x86_64-linux-musl/lib/gcc/x86_64-linux-musl/6.0.0/include/mm_malloc.h:27:0, from :1: /data/cross/x86_64-linux-musl/x86_64-linux-musl/include/stdlib.h:98:5: error: from previous declaration 'int posix_memalign(void**, size_t, size_t)' int posix_memalign (void **, size_t, size_t); ^ however it seems without -pedantic there is no error anymore, the code is accepted even in standard conforming mode. (it used to be an error that could not be silenced). this means the patch is no longer critical for musl support.
Re: [PATCH] fortran/66043 -- Check for NULL() in STORAGE_SIZE()
> I can change the error to something like > > "Intrinsic function NULL() cannot be an actual argument to > STORAGE_SIZE because it returns a disassociated pointer” I think that’d be better, indeed. > No. This isn't a numbered constraint. I spent an hour or so > tracing though parent namespaces and symbol trees to see if > I could catch the problem in comment #2. I think that this > would require a runtime error (or adding an internal attribute > that explicitly tracks associate/disassociation and > allocation/deallocation). That seems likely. Thus: OK to commit with adjusted error message (and testcase pattern). FX
[PATCH] Fix memory orders description in atomic ops built-ins docs.
This patch improves the documentation of the built-ins for atomic
operations.
First, to be consistent with the C++11 standard and make it easier for
users to relate the built-ins to the C++11 specification, the patch
changes 'memory model' to 'memory order'. The memory model is the whole
thing, whereas memory orders are specific classes ordering requirements
that an atomic operations are parametrized by.
Second, the patch clarifies that the descriptions of the memory orders
are for illustrative purposes only, and adds illustrative descriptions
that are less incorrect and incomplete than what we had before. There
is no way we can precisely summarize the C++ memory model in the docs,
and there isn't really a need either.
OK for trunk?
2015-05-15 Torvald Riegel
* doc/extend.texi (__atomic Builtins): Use 'memory order' instead of
'memory model' to align with C++11; fix description of memory orders;
fix a few typos.
commit d29b0bbeecf2267085f4ecdbaa61a0f969665468
Author: Torvald Riegel
Date: Fri May 15 18:14:40 2015 +0200
Fix memory order description in atomic ops built-ins docs.
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 6004681..5b2ded8 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -8853,19 +8853,19 @@ are not prevented from being speculated to before the barrier.
@section Built-in Functions for Memory Model Aware Atomic Operations
The following built-in functions approximately match the requirements
-for C++11 concurrency and memory models. They are all
+for the C++11 memory model. They are all
identified by being prefixed with @samp{__atomic} and most are
overloaded so that they work with multiple types.
These functions are intended to replace the legacy @samp{__sync}
-builtins. The main difference is that the memory model to be used is a
-parameter to the functions. New code should always use the
+builtins. The main difference is that the memory order that is requested
+is a parameter to the functions. New code should always use the
@samp{__atomic} builtins rather than the @samp{__sync} builtins.
Note that the @samp{__atomic} builtins assume that programs will
-conform to the C++11 model for concurrency. In particular, they assume
+conform to the C++11 memory model. In particular, they assume
that programs are free of data races. See the C++11 standard for
-detailed definitions.
+detailed requirements.
The @samp{__atomic} builtins can be used with any integral scalar or
pointer type that is 1, 2, 4, or 8 bytes in length. 16-byte integral
@@ -8875,136 +8875,140 @@ supported by the architecture.
The four non-arithmetic functions (load, store, exchange, and
compare_exchange) all have a generic version as well. This generic
version works on any data type. If the data type size maps to one
-of the integral sizes that may have lock free support, the generic
-version uses the lock free built-in function. Otherwise an
+of the integral sizes that may support lock-freedom, the generic
+version uses the lock-free built-in function. Otherwise an
external call is left to be resolved at run time. This external call is
the same format with the addition of a @samp{size_t} parameter inserted
as the first parameter indicating the size of the object being pointed to.
All objects must be the same size.
-There are 6 different memory models that can be specified. These map
-to the C++11 memory models with the same names, see the C++11 standard
+There are 6 different memory orders that can be specified. These map
+to the C++11 memory orders with the same names, see the C++11 standard
or the @uref{http://gcc.gnu.org/wiki/Atomic/GCCMM/AtomicSync,GCC wiki
on atomic synchronization} for detailed definitions. Individual
-targets may also support additional memory models for use on specific
+targets may also support additional memory orders for use on specific
architectures. Refer to the target documentation for details of
these.
-The memory models integrate both barriers to code motion as well as
-synchronization requirements with other threads. They are listed here
-in approximately ascending order of strength.
+An atomic operation can both constrain code motion by the compiler and
+be mapped to a hardware instruction for synchronization between threads
+(e.g., a fence). To which extent this happens is controlled by the
+memory orders, which are listed here in approximately ascending order of
+strength. The description of each memory order is only meant to roughly
+illustrate the effects and is not a specification; see the C++11
+memory model for precise semantics.
@table @code
@item __ATOMIC_RELAXED
-No barriers or synchronization.
+Implies no inter-thread ordering constraints.
@item __ATOMIC_CONSUME
-Data dependency only for both barrier and synchronization with another
-thread.
+This is currently implemented using the stronger @code{__ATOMIC_ACQUIRE}
+memory order because of a deficiency in
Re: [PATCH i386] Allow sibcalls in no-PLT PIC
Ping? Any comment about this patch? On Mon, 4 May 2015, Alexander Monakov wrote: > With -fno-plt, we don't have to reject even direct calls as sibcall > candidates. > > This patch depends on '-fplt' flag that is introduced in another patch. > > This patch requires that with -fno-plt all sibcall candidates go through > prepare_call_address that transforms the call to a GOT lookup. > > OK? > * config/i386/i386.c (ix86_function_ok_for_sibcall): Check flag_plt. > > diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > index f29e053..b734350 100644 > --- a/gcc/config/i386/i386.c > +++ b/gcc/config/i386/i386.c > @@ -5448,12 +5448,13 @@ ix86_function_ok_for_sibcall (tree decl, tree exp) >/* If we are generating position-independent code, we cannot sibcall > optimize any indirect call, or a direct call to a global function, > as the PLT requires %ebx be live. (Darwin does not have a PLT.) */ >if (!TARGET_MACHO >&& !TARGET_64BIT >&& flag_pic > + && flag_plt >&& (decl && !targetm.binds_local_p (decl))) > return false; > >/* If we need to align the outgoing stack, then sibcalling would > unalign the stack, which may break the called function. */ >if (ix86_minimum_incoming_stack_boundary (true) >
Re: [PATCH i386] Allow sibcalls in no-PLT PIC
On Fri, May 15, 2015 at 9:27 AM, Alexander Monakov wrote: > Ping? Any comment about this patch? > > On Mon, 4 May 2015, Alexander Monakov wrote: > >> With -fno-plt, we don't have to reject even direct calls as sibcall >> candidates. >> >> This patch depends on '-fplt' flag that is introduced in another patch. >> >> This patch requires that with -fno-plt all sibcall candidates go through >> prepare_call_address that transforms the call to a GOT lookup. >> >> OK? >> * config/i386/i386.c (ix86_function_ok_for_sibcall): Check flag_plt. >> >> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c >> index f29e053..b734350 100644 >> --- a/gcc/config/i386/i386.c >> +++ b/gcc/config/i386/i386.c >> @@ -5448,12 +5448,13 @@ ix86_function_ok_for_sibcall (tree decl, tree exp) >>/* If we are generating position-independent code, we cannot sibcall >> optimize any indirect call, or a direct call to a global function, >> as the PLT requires %ebx be live. (Darwin does not have a PLT.) */ >>if (!TARGET_MACHO >>&& !TARGET_64BIT >>&& flag_pic >> + && flag_plt >>&& (decl && !targetm.binds_local_p (decl))) >> return false; >> >>/* If we need to align the outgoing stack, then sibcalling would >> unalign the stack, which may break the called function. */ >>if (ix86_minimum_incoming_stack_boundary (true) >> I think it should be done via psABI change similar to https://groups.google.com/forum/#!topic/x86-64-abi/n8GYMpqvBxI which I have implemented on users/hjl/relax branch in binutils. -- H.J.
Re: [patch, fortran] fix for PR 60780, PR 40958
>> The change may be small enough that an assignment isn't needed. >> We (ie, the gfortran developers) will need to check. > > I think that’s small enough, compared to what we’ve accepted as such in the > past. > If not, a disclaimer by Russell putting his change in the public domain would > also be a quick way: https://gcc.gnu.org/contribute.html#legal Let me know either way. > Regarding the patch itself, it seems OK. (I first wondered if the strcmp() is > necessary, but it appears module strings at this point are not GCC > identifiers, but normal strings.) That was my understanding. At any rate, I based the logic on the existing check for unused equivalences. > Russell, you said “tested on x86_64-linux”. Could you explicitly confirm that > you have bootstrapped it and regression-tested the full gfortran testsuite ? Yes, as long as `make check-fortran` is the full gfortran testsuite. On Fri, May 15, 2015 at 4:08 AM, FX wrote: >> The change may be small enough that an assignment isn't needed. >> We (ie, the gfortran developers) will need to check. > > I think that’s small enough, compared to what we’ve accepted as such in the > past. > If not, a disclaimer by Russell putting his change in the public domain would > also be a quick way: https://gcc.gnu.org/contribute.html#legal > >> Having an assignment will help when you submit additional patches. :-) > > Indeed! > > > Regarding the patch itself, it seems OK. (I first wondered if the strcmp() is > necessary, but it appears module strings at this point are not GCC > identifiers, but normal strings.) > > Russell, you said “tested on x86_64-linux”. Could you explicitly confirm that > you have bootstrapped it and regression-tested the full gfortran testsuite ? > > Cheers, > FX
Re: Fwd: [PING 2][PATCH] libgcc: Add CFI directives to the soft floating point support code for ARM
On 13/05/15 19:11, Martin Galvan wrote:
Here's the new patch. I downloaded the gcc sources from the SVN
repository, removed the extra semicolon from my version of the files
and re-generated the patch using svn diff, making sure the context
info had all the tabs from the original. I then e-mailed the patch to
myself as an attachment, applied it to the fresh SVN sources by doing
patch --dry-run -p0 < cfi-svn.patch and checked that the compilation
and tests were successful.
Thanks for doing this now the patch looks much better and I can apply it
! Unfortunately there are still problems with it and I'm not sure about
your testing procedures.
mov ip, r0
mov r0, r2
@@ -1149,12 +1250,15 @@
mov r3, ip
b 6f
-ARM_FUNC_START aeabi_cdcmpeq
ARM_FUNC_ALIAS aeabi_cdcmple aeabi_cdcmpeq
How did this even build ?
I've cleaned up some lines which were > 80 characters, added a
definition for aeabi_cdcmpeq again and applied the attached after
*doing* a full bootstrap and test run on arm-none-linux-gnueabihf along
with a test run on arm-none-eabi.
regards
Ramana
2015-05-15 Martin Galvan
* config/arm/lib1funcs.S (CFI_START_FUNCTION, CFI_END_FUNCTION):
New macros.
* config/arm/ieee754-df.S: Add CFI directives.
* config/arm/ieee754-sf.S: Add CFI directives.
Index: libgcc/config/arm/ieee754-df.S
===
--- libgcc/config/arm/ieee754-df.S (revision 223219)
+++ libgcc/config/arm/ieee754-df.S (working copy)
@@ -33,8 +33,12 @@
* Only the default rounding mode is intended for best performances.
* Exceptions aren't supported yet, but that can be added quite easily
* if necessary without impacting performances.
+ *
+ * In the CFI related comments, 'previousOffset' refers to the previous offset
+ * from sp used to compute the CFA.
*/
+ .cfi_sections .debug_frame
#ifndef __ARMEB__
#define xl r0
@@ -53,11 +57,13 @@
ARM_FUNC_START negdf2
ARM_FUNC_ALIAS aeabi_dneg negdf2
+ CFI_START_FUNCTION
@ flip sign bit
eor xh, xh, #0x8000
RET
+ CFI_END_FUNCTION
FUNC_END aeabi_dneg
FUNC_END negdf2
@@ -66,6 +72,7 @@
#ifdef L_arm_addsubdf3
ARM_FUNC_START aeabi_drsub
+ CFI_START_FUNCTION
eor xh, xh, #0x8000 @ flip sign bit of first arg
b 1f
@@ -81,7 +88,11 @@
ARM_FUNC_START adddf3
ARM_FUNC_ALIAS aeabi_dadd adddf3
-1: do_push {r4, r5, lr}
+1: do_push {r4, r5, lr}@ sp -= 12
+ .cfi_adjust_cfa_offset 12 @ CFA is now sp + previousOffset + 12
+ .cfi_rel_offset r4, 0 @ Registers are saved from sp to sp + 8
+ .cfi_rel_offset r5, 4
+ .cfi_rel_offset lr, 8
@ Look for zeroes, equal values, INF, or NAN.
shift1 lsl, r4, xh, #1
@@ -148,6 +159,11 @@
@ Since this is not common case, rescale them off line.
teq r4, r5
beq LSYM(Lad_d)
+
+@ CFI note: we're lucky that the branches to Lad_* that appear after this
function
+@ have a CFI state that's exactly the same as the one we're in at this
+@ point. Otherwise the CFI would change to a different state after the branch,
+@ which would be disastrous for backtracing.
LSYM(Lad_x):
@ Compensate for the exponent overlapping the mantissa MSB added later
@@ -413,6 +429,7 @@
orrne xh, xh, #0x0008 @ quiet NAN
RETLDM "r4, r5"
+ CFI_END_FUNCTION
FUNC_END aeabi_dsub
FUNC_END subdf3
FUNC_END aeabi_dadd
@@ -420,12 +437,19 @@
ARM_FUNC_START floatunsidf
ARM_FUNC_ALIAS aeabi_ui2d floatunsidf
+ CFI_START_FUNCTION
teq r0, #0
do_it eq, t
moveq r1, #0
RETc(eq)
- do_push {r4, r5, lr}
+
+ do_push {r4, r5, lr}@ sp -= 12
+ .cfi_adjust_cfa_offset 12 @ CFA is now sp + previousOffset + 12
+ .cfi_rel_offset r4, 0 @ Registers are saved from sp + 0 to sp + 8.
+ .cfi_rel_offset r5, 4
+ .cfi_rel_offset lr, 8
+
mov r4, #0x400 @ initial exponent
add r4, r4, #(52-1 - 1)
mov r5, #0 @ sign bit is 0
@@ -435,17 +459,25 @@
mov xh, #0
b LSYM(Lad_l)
+ CFI_END_FUNCTION
FUNC_END aeabi_ui2d
FUNC_END floatunsidf
ARM_FUNC_START floatsidf
ARM_FUNC_ALIAS aeabi_i2d floatsidf
+ CFI_START_FUNCTION
teq r0, #0
do_it eq, t
moveq r1, #0
RETc(eq)
- do_push {r4, r5, lr}
+
+ do_push {r4, r5, lr}@ sp -= 12
+ .cfi_adjust_cfa_offset 12 @ CFA is now sp + previousOffset + 12
+ .cfi_rel_offset r4, 0 @ Registers are saved from sp + 0 to sp + 8.
+ .cfi_rel_offset r5, 4
+ .cfi_rel_offset lr, 8
+
mov r4, #0x400 @ initial exponent
add r4, r4, #(52-1 -
*ping* Re: [Patch, fortran] PR65792 - allocation of scalar elemental function with structure constructor fails
Le 09/05/2015 15:12, Mikael Morin a écrit : > Le 01/05/2015 20:25, Paul Richard Thomas a écrit : >> Dear All, >> >> By the time I went to commit, something had changed and the patch >> caused a regression. I presume that the version that I had of Andre's >> patch was not the same as the one committed. I'll cast an eye over it >> this weekend and see if I can understand what gives. >> > Hello Paul, > > to get things moving again, I propose the attached fix to your patch. > Tested on alloc_comp_deep_copy_1 only for now. Hello Paul, any news? Mikael
Re: [PATCH 13/13] fix incompatible posix_memalign declaration on x86
On Fri, May 15, 2015 at 9:09 AM, Szabolcs Nagy wrote: > > > On 15/05/15 16:05, H.J. Lu wrote: >> On Mon, Apr 20, 2015 at 12:00 PM, Szabolcs Nagy >> wrote: >>> The posix_memalign declaration is incompatible with musl for C++, >>> because of the exception specification. It also pollutes the >>> namespace and lacks protection against a potential macro definition >>> that is allowed by POSIX. The fix avoids source level namespace >>> pollution but retains the dependency on the posix_memalign extern >>> libc symbol. >>> >>> The fix is ugly, but it is not possible to correctly redeclare a >>> libc function in a public gcc header for C++. >>> >>> >>> gcc/Changelog: >>> >>> 2015-04-16 Szabolcs Nagy >>> >>> * config/i386/pmm_malloc.h (posix_memalign): Renamed to ... >>> (__gcc_posix_memalign): This. Use posix_memalign as extern >>> symbol only. >> >> What does this try to achieve? Do you have a testcase which >> fails before and passes with this patch? > > if posix_memalign is defined in stdlib.h according to posix then > the exception specifier used in mm_malloc.h is incompatible: > > $ echo "#include " | ./x86_64-linux-musl-g++ -pedantic -c -xc++ > -D_POSIX_C_SOURCE=200809L - > In file included from :1:0: > /data/cross/x86_64-linux-musl/lib/gcc/x86_64-linux-musl/6.0.0/include/mm_malloc.h:34:64: > error: declaration of 'int posix_memalign(void**, size_t, > size_t) throw ()' has a different exception specifier > extern "C" int posix_memalign (void **, size_t, size_t) throw () > ^ > In file included from > /data/cross/x86_64-linux-musl/lib/gcc/x86_64-linux-musl/6.0.0/include/mm_malloc.h:27:0, > from :1: > /data/cross/x86_64-linux-musl/x86_64-linux-musl/include/stdlib.h:98:5: error: > from previous declaration 'int posix_memalign(void**, size_t, size_t)' > int posix_memalign (void **, size_t, size_t); > ^ > > however it seems without -pedantic there is no error anymore, > the code is accepted even in standard conforming mode. > (it used to be an error that could not be silenced). > > this means the patch is no longer critical for musl support. > 2 more comments: 1. You need a tecase, independent of MUSL. 2. Please replace __gcc_posix_memalign with _mm_posix_memalign. -- H.J.
match.pd: (x | y) & ~x -> y & ~x
Hello,
we already have the more complicated: x & ~(x & y) -> x & ~y (which I am
reindenting by the way) and the simpler: (~x | y) & x -> x & y, so I am
proposing this one for completeness. Regtested on ppc64le-redhat-linux.
2015-05-15 Marc Glisse
gcc/
* match.pd ((x | y) & ~x -> y & ~x, (x & y) | ~x -> y | ~x): New
simplifications.
gcc/testsuite/
* gcc.dg/nand.c: New testcase.
--
Marc GlisseIndex: match.pd
===
--- match.pd(revision 223217)
+++ match.pd(working copy)
@@ -257,24 +257,32 @@ along with GCC; see the file COPYING3.
/* x + (x & 1) -> (x + 1) & ~1 */
(simplify
(plus:c @0 (bit_and@2 @0 integer_onep@1))
(if (TREE_CODE (@2) != SSA_NAME || has_single_use (@2))
(bit_and (plus @0 @1) (bit_not @1
/* x & ~(x & y) -> x & ~y */
/* x | ~(x | y) -> x | ~y */
(for bitop (bit_and bit_ior)
- (simplify
-(bitop:c @0 (bit_not (bitop:c@2 @0 @1)))
- (if (TREE_CODE (@2) != SSA_NAME || has_single_use (@2))
- (bitop @0 (bit_not @1)
+ (simplify
+ (bitop:c @0 (bit_not (bitop:c@2 @0 @1)))
+ (if (TREE_CODE (@2) != SSA_NAME || has_single_use (@2))
+(bitop @0 (bit_not @1)
+
+/* (x | y) & ~x -> y & ~x */
+/* (x & y) | ~x -> y | ~x */
+(for bitop (bit_and bit_ior)
+ rbitop (bit_ior bit_and)
+ (simplify
+ (bitop:c (rbitop:c @0 @1) (bit_not@2 @0))
+ (bitop @1 @2)))
(simplify
(abs (negate @0))
(abs @0))
(simplify
(abs tree_expr_nonnegative_p@0)
@0)
/* Try to fold (type) X op CST -> (type) (X op ((type-x) CST))
Index: testsuite/gcc.dg/nand.c
===
--- testsuite/gcc.dg/nand.c (revision 0)
+++ testsuite/gcc.dg/nand.c (working copy)
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-O -fdump-tree-original" } */
+
+unsigned f(unsigned x, unsigned y){
+ return (x | y) & ~x;
+}
+unsigned g(unsigned x, unsigned y){
+ return ~x & (y | x);
+}
+
+/* { dg-final { scan-tree-dump-times "return ~x & y;" 2 "original" } } */
+/* { dg-final { cleanup-tree-dump "original" } } */
Re: Fwd: [PING 2][PATCH] libgcc: Add CFI directives to the soft floating point support code for ARM
On Fri, May 15, 2015 at 1:58 PM, Ramana Radhakrishnan wrote: > Thanks for doing this now the patch looks much better and I can apply it ! > Unfortunately there are still problems with it and I'm not sure about your > testing procedures. > >> >> >> mov ip, r0 >> mov r0, r2 >> @@ -1149,12 +1250,15 @@ >> mov r3, ip >> b 6f >> >> -ARM_FUNC_START aeabi_cdcmpeq >> ARM_FUNC_ALIAS aeabi_cdcmple aeabi_cdcmpeq > > > How did this even build ? > > I've cleaned up some lines which were > 80 characters, added a definition > for aeabi_cdcmpeq again and applied the attached after *doing* a full > bootstrap and test run on arm-none-linux-gnueabihf along with a test run on > arm-none-eabi. Thanks a lot! I guess I should've mentioned I've tested it on arm-rtems411. I'm not sure why it didn't throw me any errors, though, but I'm glad it's finally commited and working. Again, thanks a lot and sorry for the inconvenience. -- Martin Galvan Software Engineer Taller Technologies Argentina San Lorenzo 47, 3rd Floor, Office 5 Córdoba, Argentina Phone: 54 351 4217888 / +54 351 4218211
Re: [RFC: Patch, PR 60158] gcc/varasm.c : Pass actual alignment value to output_constant_pool_2
On 05/15/2015 04:37 AM, Dharmakan Rohit Arul Raj wrote: -Original Message- From: Jeff Law [mailto:l...@redhat.com] Sent: Friday, May 15, 2015 10:30 AM Just to summarize: By default in GCC v4.7.x, all the constants are put into '.rodata.str1.4' section. In GCC v4.8.x from r192719 onwards, one of the move instruction of the string constant ".LC0" is getting spilled. The reload pass, for any constants that aren't allowed and can't be reloaded in to registers tries to change them into memory references. Then while emitting that string constant to asm code (A:varasm.c: output_constant_pool_1), it explicitly passes the alignment as 1 which prevents the generation of fix-up table entries in 'B: rs6000.c:rs6000_assemble_integer' because the data is considered unaligned now. The bug seems to have gone latent with an unrelated trunk commit r204695 [* tree-ssa-loop-ivopts.c (force_expr_to_var_cost): Refactor the code. Handle type conversion.]. This commit chooses different spill candidates hence all the string constants are being put in to '.rodata.str1.4´section. The check I had in the test case is that if there is a '.data.rel.ro.local', then there should be '.fixup' section generated. Please let me know if you need any other details. Thanks. Even though I wasn't able to trigger the bug with the testcase from 65018, I went ahead and committed this patch to the trunk. It can't hurt and it's the right thing to do. Thanks for your patience, Jeff, Thanks for checking-in the changes. Can we apply the patch to GCC v4.8 and GCC v4.9 branch as well? That's up to the branch maintainers. I'd think it's safe, but it's ultimately their call. jeff
Re: [PATCH] [AArch32] Additional bics patterns.
On 01/05/15 10:28, Kyrill Tkachov wrote:
Can you please confirm that bootstraps with both arm and thumb pass?
That is, configured with --with-mode=arm and --with-mode=thumb
Hi Kyrill,
Bootstrapped on arm-none-gnueabihf with arm and thumb mode.
Following patch requires bics shift operand on Thumb2 to be
const int, as bics shifted by register is not supported by
Thumb2.
Is patch ok?
gcc
2015-05-15 Alex Velenko
* config/arm/arm.md (andsi_not_shiftsi_si_scc): New pattern.
* (andsi_not_shiftsi_si_scc_no_reuse): New pattern.
gcc/testsuite
2015-05-15 Alex Velenko
* gcc.target/arm/bics_1.c : New testcase.
* gcc.target/arm/bics_2.c : New testcase.
* gcc.target/arm/bics_3.c : New testcase.
* gcc.target/arm/bics_4.c : New testcase.
---
gcc/config/arm/arm.md | 49 ++
gcc/testsuite/gcc.target/arm/bics_1.c | 54
+
gcc/testsuite/gcc.target/arm/bics_2.c | 57
+++
gcc/testsuite/gcc.target/arm/bics_3.c | 41 +
gcc/testsuite/gcc.target/arm/bics_4.c | 49 ++
5 files changed, 250 insertions(+)
create mode 100644 gcc/testsuite/gcc.target/arm/bics_1.c
create mode 100644 gcc/testsuite/gcc.target/arm/bics_2.c
create mode 100644 gcc/testsuite/gcc.target/arm/bics_3.c
create mode 100644 gcc/testsuite/gcc.target/arm/bics_4.c
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index 164ac13..26d3ad2 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -2768,6 +2768,55 @@
(const_string "logic_shift_reg")))]
)
+;; Shifted bics pattern used to set up CC status register and not reusing
+;; bics output. Pattern restricts Thumb2 shift operand as bics for Thumb2
+;; does not support shift by register.
+(define_insn "andsi_not_shiftsi_si_scc_no_reuse"
+ [(set (reg:CC_NOOV CC_REGNUM)
+ (compare:CC_NOOV
+ (and:SI (not:SI (match_operator:SI 0 "shift_operator"
+ [(match_operand:SI 1 "s_register_operand" "r")
+(match_operand:SI 2 "arm_rhs_operand" "rM")]))
+ (match_operand:SI 3 "s_register_operand" "r"))
+ (const_int 0)))
+ (clobber (match_scratch:SI 4 "=3Dr"))]
+ "TARGET_ARM || (TARGET_THUMB2 && CONST_INT_P (operands[2]))"
+ "bic%.%?\\t%4, %3, %1%S0"
+ [(set_attr "predicable" "yes")
+ (set_attr "predicable_short_it" "no")
+ (set_attr "conds" "set")
+ (set_attr "shift" "1")
+ (set (attr "type") (if_then_else (match_operand 2
"const_int_operand" "")
+ (const_string "logic_shift_imm")
+ (const_string "logic_shift_reg")))]
+)
+
+;; Same as andsi_not_shiftsi_si_scc_no_reuse, but the bics result is also
+;; getting reused later.
+(define_insn "andsi_not_shiftsi_si_scc"
+ [(parallel [(set (reg:CC_NOOV CC_REGNUM)
+ (compare:CC_NOOV
+ (and:SI (not:SI (match_operator:SI 0 "shift_operator"
+ [(match_operand:SI 1 "s_register_operand" "r")
+(match_operand:SI 2 "arm_rhs_operand" "rM")]))
+ (match_operand:SI 3 "s_register_operand" "r"))
+ (const_int 0)))
+ (set (match_operand:SI 4 "s_register_operand" "=3Dr")
+(and:SI (not:SI (match_op_dup 0
+[(match_dup 1)
+ (match_dup 2)]))
+(match_dup 3)))])]
+ "TARGET_ARM || (TARGET_THUMB2 && CONST_INT_P (operands[2]))"
+ "bic%.%?\\t%4, %3, %1%S0"
+ [(set_attr "predicable" "yes")
+ (set_attr "predicable_short_it" "no")
+ (set_attr "conds" "set")
+ (set_attr "shift" "1")
+ (set (attr "type") (if_then_else (match_operand 2
"const_int_operand" "")
+ (const_string "logic_shift_imm")
+ (const_string "logic_shift_reg")))]
+)
+
(define_insn "*andsi_notsi_si_compare0"
[(set (reg:CC_NOOV CC_REGNUM)
(compare:CC_NOOV
diff --git a/gcc/testsuite/gcc.target/arm/bics_1.c
b/gcc/testsuite/gcc.target/arm/bics_1.c
new file mode 100644
index 000..173eb89
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/bics_1.c
@@ -0,0 +1,54 @@
+/* { dg-do run } */
+/* { dg-options "-O2 --save-temps -fno-inline" } */
+/* { dg-require-effective-target arm32 } */
+
+extern void abort (void);
+
+int
+bics_si_test1 (int a, int b, int c)
+{
+ int d =3D a & ~b;
+
+ /* { dg-final { scan-assembler-times "bics\tr\[0-9\]+, r\[0-9\]+,
r\[0-9\]+" 2 } } */
+ if (d =3D=3D 0)
+return a + c;
+ else
+return b + d + c;
+}
+
+int
+bics_si_test2 (int a, int b, int c)
+{
+ int d =3D a & ~(b << 3);
+
+ /* { dg-final { scan-assembler-times "bics\tr\[0-9\]+, r\[0-9\]+,
r\[0-9\]+, .sl \#3" 1 } } */
+ if (d =3D=3D 0)
+return a + c;
+ else
+return b + d + c;
+}
+
+int
+main ()
+{
+ int x;
+
+ x =3D bics_si_test1 (29, ~4, 5);
+ if (x !=3D ((29 & 4) + ~4 + 5))
+abort ();
+
+ x =3D bics_si_test1 (5, ~2, 20
Re: *ping* Re: [Patch, fortran] PR65792 - allocation of scalar elemental function with structure constructor fails
Dear Mikael, dear all, I am struggling to find time for gfortran at present because of the change in my personal circumstances. I am still working full time as a consultant, whilst trying to sell the house and fix up all those things that l'Administration Francaise makes so "interesting" (Having had quasi-diplomatic status, there are all sorts of issues involved in cars, pensions and... yes, the 2014 tax return). Things will improve on a timescale of three months because I am expecting an acceptable offer for the house, next week. In the meantime, to show willing, I will attend to this right now :-) Cheers Paul On 15 May 2015 at 19:01, Mikael Morin wrote: > Le 09/05/2015 15:12, Mikael Morin a écrit : >> Le 01/05/2015 20:25, Paul Richard Thomas a écrit : >>> Dear All, >>> >>> By the time I went to commit, something had changed and the patch >>> caused a regression. I presume that the version that I had of Andre's >>> patch was not the same as the one committed. I'll cast an eye over it >>> this weekend and see if I can understand what gives. >>> >> Hello Paul, >> >> to get things moving again, I propose the attached fix to your patch. >> Tested on alloc_comp_deep_copy_1 only for now. > > Hello Paul, > > any news? > > Mikael -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx
[PATCH] [PATCH][ARM] Fix thumb1-far-jump-2.c testcase.
Hi,
This patch fixes testcase thumb1-far-jump-2.c to confirm to newer compilation
defaults.
Is patch ok?
gcc/testsuite
2015-05-15 Alex Velenko
* gcc.target/arm/thumb1-far-jump-2.c (r4): Added int in definition.
---
gcc/testsuite/gcc.target/arm/thumb1-far-jump-2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gcc/testsuite/gcc.target/arm/thumb1-far-jump-2.c
b/gcc/testsuite/gcc.target/arm/thumb1-far-jump-2.c
index c6878f8..78fcafa 100644
--- a/gcc/testsuite/gcc.target/arm/thumb1-far-jump-2.c
+++ b/gcc/testsuite/gcc.target/arm/thumb1-far-jump-2.c
@@ -5,7 +5,7 @@
/* { dg-options "-Os" } */
/* { dg-skip-if "" { ! { arm_thumb1 } } } */
-volatile register r4 asm("r4");
+volatile register int r4 asm ("r4");
void f3(int i)
{
#define GO(n) \
--
1.8.1.2
[patch,gomp4] error on invalid acc loop clauses
This patch teaches the c and c++ front ends to error on invalid and
conflicting acc loop clauses. E.g., an acc loop cannot have 'gang seq'
and the worker and vector clauses inside parallel regions cannot have
optional kernel-specific arguments.
The c and c++ front end also error when it detects a parallel or kernels
region nested inside a parallel or kernels region. E.g.
#pragma acc parallel
{
#pragma acc parallel
...
}
This is technically supported by OpenACC 2.0a, but there are a couple of
unresolved technical issues preventing its inclusion for gcc 6.0. For
starters, we don't have the runtime rigged up to handle CUDA's dynamic
parallelism, and it's unclear if the spec itself requires the runtime to
do so. Then there's the issue of mapping gangs within gangs, which
presumably gets handled by dynamic parallelism.
I included two new test cases in this patch. They are mostly identical
but, unfortunately, the c and c++ front ends emit slightly different
error messages.
The front ends still need to be cleaned before this functionality should
be considered for mainline. So for the time being I've applied this
patch to gomp-4_0-branch.
Cesar
2015-05-15 Cesar Philippidis
gcc/c/
* c-parser.c (typedef struct c_parser): Add BOOL_BITFIELDS
oacc_parallel_region and oacc_kernels_region.
(c_parser_oacc_shape_clause): Only use op1 for the static argument
in the gang clause. Check for incompatible clause arguments inside
parallel regions.
(c_parser_oacc_loop): Error on conflicting loop clauses.
(c_parser_oacc_kernels): Error in nested parallel and kernels.
(c_parser_oacc_parallel): Likewise.
gcc/cp/
* parser.h (typedef struct cp_parser): Add bool oacc_parallel_region
and oacc_kernels_region.
* parser.c (cp_parser_oacc_shape_clause): Only use op1 for the static
argument in the gang clause. Check for incompatible clause arguments
inside parallel regions.
(cp_parser_oacc_loop): Error on conflicting loop clauses.
(cp_parser_oacc_parallel_kernels): Error in nested parallel and
kernels.
gcc/
* omp-low.c (scan_omp_for): Remove check for nested parallel and
kernels regions.
gcc/testsuite/
* c-c++-common/goacc/nesting-fail-1.c: Update error messages.
* g++.dg/goacc/loop-4.C: New test.
* gcc.dg/goacc/loop-1.c: New test.
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 74adeb8..f508b91 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -234,6 +234,10 @@ typedef struct GTY(()) c_parser {
/* True if we are in a context where the Objective-C "Property attribute"
keywords are valid. */
BOOL_BITFIELD objc_property_attr_context : 1;
+ /* True if we are inside a OpenACC parallel region. */
+ BOOL_BITFIELD oacc_parallel_region : 1;
+ /* True if we are inside a OpenACC kernels region. */
+ BOOL_BITFIELD oacc_kernels_region : 1;
/* Cilk Plus specific parser/lexer information. */
@@ -10839,6 +10843,7 @@ c_parser_oacc_shape_clause (c_parser *parser, pragma_omp_clause c_kind,
mark_exp_read (expr);
require_positive_expr (expr, expr_loc, str);
*op_to_parse = expr;
+ op_to_parse = &op0;
}
while (!c_parser_next_token_is (parser, CPP_CLOSE_PAREN));
c_parser_consume_token (parser);
@@ -10852,6 +10857,17 @@ c_parser_oacc_shape_clause (c_parser *parser, pragma_omp_clause c_kind,
if (op1)
OMP_CLAUSE_OPERAND (c, 1) = op1;
OMP_CLAUSE_CHAIN (c) = list;
+
+ if (parser->oacc_parallel_region && (op0 != NULL || op1 != NULL))
+{
+ if (c_kind != PRAGMA_OACC_CLAUSE_GANG)
+ c_parser_error (parser, c_kind == PRAGMA_OACC_CLAUSE_WORKER ?
+ "worker clause arguments are not supported in OpenACC parallel regions"
+ : "vector clause arguments are not supported in OpenACC parallel regions");
+ else if (op0 != NULL)
+ c_parser_error (parser, "non-static argument to clause gang");
+}
+
return c;
}
@@ -12721,7 +12737,10 @@ static tree
c_parser_oacc_loop (location_t loc, c_parser *parser, char *p_name,
omp_clause_mask mask, tree *cclauses)
{
- tree stmt, clauses, block;
+ tree stmt, clauses, block, c;
+ bool gwv = false;
+ bool auto_clause = false;
+ bool seq_clause = false;
strcat (p_name, " loop");
mask |= OACC_LOOP_CLAUSE_MASK;
@@ -12732,6 +12751,33 @@ c_parser_oacc_loop (location_t loc, c_parser *parser, char *p_name,
if (cclauses)
clauses = oacc_split_loop_clauses (clauses, cclauses);
+ for (c = clauses; c; c = OMP_CLAUSE_CHAIN (c))
+{
+ switch (OMP_CLAUSE_CODE (c))
+ {
+ case OMP_CLAUSE_GANG:
+ case OMP_CLAUSE_WORKER:
+ case OMP_CLAUSE_VECTOR:
+ gwv = true;
+ break;
+ case OMP_CLAUSE_AUTO:
+ auto_clause = true;
+ break;
+ case OMP_CLAUSE_SEQ:
+ seq_clause = true;
+ break;
+ default:
+ ;
+ }
+}
+
+ if (gwv && auto_clause)
+c_parser_error (parser, "incompatible use of clause %");
+ else if (gwv && seq_clause)
+c_parser_error (parser, "incompatible use of clause %");
+ else if (auto_clause && seq_clause)
+c_parser_error (parser, "incompatib
Re: [PATCH] fortran/66106 -- Enforce C1202 from F2008
> Whoops, sorry about that. As you can see I have small patches > sitting in my tree. I tried to untangle the 'svn diff’ Thanks for cleaning and submitting those! I’ll try to review them over the next 2 days, if nobody beats me to it :) FX
Re: [patch, c, ping] Fix PR c/48956: diagnostics for conversions involving complex types (reviewed)
On 04/17/2015 05:01 AM, Mikhail Maltsev wrote:
> I would like to ping the following patch:
> https://gcc.gnu.org/ml/gcc-patches/2014-12/msg01925.html
> Review: https://gcc.gnu.org/ml/gcc-patches/2015-01/msg02672.html
>
> I fixed minor issues mentioned in the review and updated the changelog
> message. Rebased on current trunk (r222157), bootstrapped and regtested
> on x86_64-unknown-linux-gnu.
> If it is OK for trunk, please assist with applying (I don't have write
> access), and I will then create a new PR in bugzilla for the remaining
> cases mentioned in review.
Since I now have write access, I am installing this patch into trunk. I
discovered and fixed a minor issue in testcase (no warning when
converting from long to int on -m32; changed long to long long. I hope
it's OK to be considered as obvious fix).
Bootstrapped/regtested on x86_64-unknown-linux-gnu {,-m32}.
--
Regards,
Mikhail Maltsev
diff --git a/gcc/c-family/ChangeLog b/gcc/c-family/ChangeLog
index 48d483e..199ba43 100644
--- a/gcc/c-family/ChangeLog
+++ b/gcc/c-family/ChangeLog
@@ -1,3 +1,13 @@
+2015-05-15 Mikhail Maltsev
+
+ PR c/48956
+ * c-common.c (int_safely_convertible_to_real_p): Define.
+ (unsafe_conversion_p): Check conversions involving complex types.
+ (conversion_warning): Add new warning message for conversions which
+ discard imaginary component.
+ * c-common.h: (enum conversion_safety): Add new enumerator for such
+ conversions.
+
2015-05-14 Marek Polacek
PR c/66066
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 31c4c0d..8c7fdd2 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -2707,17 +2707,42 @@ shorten_binary_op (tree result_type, tree op0, tree op1, bool bitwise)
return result_type;
}
-/* Checks if expression EXPR of real/integer type cannot be converted
- to the real/integer type TYPE. Function returns non-zero when:
+/* Returns true iff any integer value of type FROM_TYPE can be represented as
+ real of type TO_TYPE. This is a helper function for unsafe_conversion_p. */
+
+static bool
+int_safely_convertible_to_real_p (const_tree from_type, const_tree to_type)
+{
+ tree type_low_bound = TYPE_MIN_VALUE (from_type);
+ tree type_high_bound = TYPE_MAX_VALUE (from_type);
+ REAL_VALUE_TYPE real_low_bound =
+ real_value_from_int_cst (0, type_low_bound);
+ REAL_VALUE_TYPE real_high_bound =
+ real_value_from_int_cst (0, type_high_bound);
+
+ return exact_real_truncate (TYPE_MODE (to_type), &real_low_bound)
+ && exact_real_truncate (TYPE_MODE (to_type), &real_high_bound);
+}
+
+/* Checks if expression EXPR of complex/real/integer type cannot be converted
+ to the complex/real/integer type TYPE. Function returns non-zero when:
* EXPR is a constant which cannot be exactly converted to TYPE.
* EXPR is not a constant and size of EXPR's type > than size of TYPE,
- for EXPR type and TYPE being both integers or both real.
+ for EXPR type and TYPE being both integers or both real, or both
+ complex.
+ * EXPR is not a constant of complex type and TYPE is a real or
+ an integer.
* EXPR is not a constant of real type and TYPE is an integer.
* EXPR is not a constant of integer type which cannot be
exactly converted to real type.
+
Function allows conversions between types of different signedness and
can return SAFE_CONVERSION (zero) in that case. Function can produce
- signedness warnings if PRODUCE_WARNS is true. */
+ signedness warnings if PRODUCE_WARNS is true.
+
+ Function allows conversions from complex constants to non-complex types,
+ provided that imaginary part is zero and real part can be safely converted
+ to TYPE. */
enum conversion_safety
unsafe_conversion_p (location_t loc, tree type, tree expr, bool produce_warns)
@@ -2728,6 +2753,11 @@ unsafe_conversion_p (location_t loc, tree type, tree expr, bool produce_warns)
if (TREE_CODE (expr) == REAL_CST || TREE_CODE (expr) == INTEGER_CST)
{
+ /* If type is complex, we are interested in compatibility with
+ underlying type. */
+ if (TREE_CODE (type) == COMPLEX_TYPE)
+ type = TREE_TYPE (type);
+
/* Warn for real constant that is not an exact integer converted
to integer type. */
if (TREE_CODE (expr_type) == REAL_TYPE
@@ -2777,6 +2807,63 @@ unsafe_conversion_p (location_t loc, tree type, tree expr, bool produce_warns)
}
}
}
+
+ else if (TREE_CODE (expr) == COMPLEX_CST)
+{
+ tree imag_part = TREE_IMAGPART (expr);
+ /* Conversion from complex constant with zero imaginary part,
+ perform check for conversion of real part. */
+ if ((TREE_CODE (imag_part) == REAL_CST
+ && real_zerop (imag_part))
+ || (TREE_CODE (imag_part) == INTEGER_CST
+ && integer_zerop (imag_part)))
+ /* Note: in this branch we use recursive call to unsafe_conversion_p
+ with different type of EXPR, but it is still safe, because when EXPR
+ is a constant, it's type is not used in text of generated wa
[v3 patch] Fix some Filesystem TS operations
Testing revealed a few bugs in how I handled paths that don't exist.
The new __gnu_test::nonexistent_path() function is a bit hacky but
should be good enough for the testsuite.
I've also made filesystem::temp_directory_path() check the TMP,
TEMPDIR and TEMP environment variables, as suggested in
[fs.op.temp_dir_path] at
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n4099.html#fs.op.temp_dir_path
rather than my original implementation which only checked TMPDIR
(because I think that's the only one mentioned by POSIX). I'm not
unsure whether it's better to follow the non-normative note or only
check for the POSIX name. Anyone else have any thoughts?
Tested x86_64-linux, committed to trunk.
commit 7bb46da1e629300e84641a59303d32ecbee7047c
Author: Jonathan Wakely
Date: Fri May 15 13:06:09 2015 +0100
* src/filesystem/ops.cc (stat_type): Define alias for struct stat and
use throughout the file.
(make_file_type): New function.
(file_size(const path&, error_code&)): Report an error for anything
that isn't a regular file.
(status(const path&), symlink_status(const path&)): Do not throw for
file_type::not_found.
(temp_directory_path()): Check additional environment variables.
* testsuite/experimental/filesystem/operations/exists.cc: New.
* testsuite/experimental/filesystem/operations/file_size.cc: New.
* testsuite/experimental/filesystem/operations/status.cc: New.
* testsuite/experimental/filesystem/operations/temp_directory_path.cc:
New.
diff --git a/libstdc++-v3/src/filesystem/ops.cc b/libstdc++-v3/src/filesystem/ops.cc
index f24cc19..661685a 100644
--- a/libstdc++-v3/src/filesystem/ops.cc
+++ b/libstdc++-v3/src/filesystem/ops.cc
@@ -150,33 +150,39 @@ namespace
#ifdef _GLIBCXX_HAVE_SYS_STAT_H
namespace
{
- fs::file_status
- make_file_status(const struct ::stat& st)
+ typedef struct ::stat stat_type;
+
+ inline fs::file_type
+ make_file_type(const stat_type& st)
{
-using fs::file_status;
using fs::file_type;
-using fs::perms;
-file_type ft;
-perms perm = static_cast(st.st_mode) & perms::mask;
#ifdef _GLIBCXX_HAVE_S_ISREG
if (S_ISREG(st.st_mode))
- ft = file_type::regular;
+ return file_type::regular;
else if (S_ISDIR(st.st_mode))
- ft = file_type::directory;
+ return file_type::directory;
else if (S_ISCHR(st.st_mode))
- ft = file_type::character;
+ return file_type::character;
else if (S_ISBLK(st.st_mode))
- ft = file_type::block;
+ return file_type::block;
else if (S_ISFIFO(st.st_mode))
- ft = file_type::fifo;
+ return file_type::fifo;
else if (S_ISLNK(st.st_mode))
- ft = file_type::symlink;
+ return file_type::symlink;
else if (S_ISSOCK(st.st_mode))
- ft = file_type::socket;
-else
+ return file_type::socket;
#endif
- ft = file_type::unknown;
-return file_status{ft, perm};
+return file_type::unknown;
+
+ }
+
+ inline fs::file_status
+ make_file_status(const stat_type& st)
+ {
+return fs::file_status{
+ make_file_type(st),
+ static_cast(st.st_mode) & fs::perms::mask
+};
}
inline bool
@@ -186,7 +192,7 @@ namespace
}
inline fs::file_time_type
- file_time(const struct ::stat& st)
+ file_time(const stat_type& st)
{
using namespace std::chrono;
return fs::file_time_type{
@@ -201,10 +207,10 @@ namespace
bool
do_copy_file(const fs::path& from, const fs::path& to,
fs::copy_options option,
- struct ::stat* from_st, struct ::stat* to_st,
+ stat_type* from_st, stat_type* to_st,
std::error_code& ec) noexcept
{
-struct ::stat st1, st2;
+stat_type st1, st2;
fs::file_status t, f;
if (to_st == nullptr)
@@ -342,7 +348,7 @@ fs::copy(const path& from, const path& to, copy_options options,
bool use_lstat = create_symlinks || skip_symlinks;
file_status f, t;
- struct ::stat from_st, to_st;
+ stat_type from_st, to_st;
if (use_lstat
? ::lstat(from.c_str(), &from_st)
: ::stat(from.c_str(), &from_st))
@@ -564,7 +570,7 @@ fs::create_directory(const path& p, const path& attributes,
error_code& ec) noexcept
{
#ifdef _GLIBCXX_HAVE_SYS_STAT_H
- struct ::stat st;
+ stat_type st;
if (::stat(attributes.c_str(), &st))
{
ec.assign(errno, std::generic_category());
@@ -747,7 +753,7 @@ bool
fs::equivalent(const path& p1, const path& p2, error_code& ec) noexcept
{
#ifdef _GLIBCXX_HAVE_SYS_STAT_H
- struct ::stat st1, st2;
+ stat_type st1, st2;
if (::stat(p1.c_str(), &st1) == 0 && ::stat(p2.c_str(), &st2) == 0)
{
file_status s1 = make_file_status(st1);
@@ -789,7 +795,7 @@ namespace
do_stat(const fs::path& p, std::error_code& ec, Accessor f, T deflt)
{
#ifdef _GLIBCXX_HAVE_SYS_STAT_H
- struct ::stat st;
+ stat_type st;
if (::stat(p.c_str(), &st))
{
ec.assign(errno, std::generic_category());
@@ -807,8 +813,24 @@ name
Re: Fix RTL checking failure in emit_pattern_{after|before}_setloc
> You might consider a comment in active_insn_p and
> emit_pattern_{after,before}_setloc so that if someone fixes
> active_insn_p, they'll know to fix emit_pattern_{after,before}_setloc.
> "FIXME" doesn't capture this interaction at all ;-)
I beg to differ, it's the same file and anyone fiddling with JUMP_TABLE_DATA_P
in active_insn_p should do a grep on the file/the entire RTL middle-end.
--
Eric Botcazou
Re: [PATCH] Fix PR ipa/65908.
> +/* Return true if DECL_ARGUMENT types are valid to be merged. */
Perhaps bettter as
Perform additional check needed to match types function parameters that are
used. Unlike for normal parameters it matters if type is TYPE_RESTRICT and we
make an assumption that REFERENCE_TYPE parameters are always non-NULL.
> +
> +bool
> +sem_function::compatible_parm_types_p ()
> +{
> + tree parm1, parm2;
> + unsigned i = 0;
> +
> + for (parm1 = DECL_ARGUMENTS (decl),
> + parm2 = DECL_ARGUMENTS (m_compared_func->decl);
> + parm1 && parm2;
> + parm1 = DECL_CHAIN (parm1), parm2 = DECL_CHAIN (parm2), i++)
I think this is still not right. What you wan to to do is to have
1) comparible_parm_types_p (t1,t2, index) that returns true if T1 and T2 are
matching with checks bellow:
> + {
> +if (!param_used_p (i))
> + continue;
> +
> +if (POINTER_TYPE_P (parm1)
> + && (TYPE_RESTRICT (parm1) != TYPE_RESTRICT (parm2)))
> + return return_false_with_msg ("argument restrict flag mismatch");
> +/* nonnull_arg_p implies non-zero range to REFERENCE types. */
> +if (POINTER_TYPE_P (parm1)
> + && TREE_CODE (parm1) != TREE_CODE (parm2)
> + && opt_for_fn (decl, flag_delete_null_pointer_checks))
> + return return_false_with_msg ("pointer wrt reference mismatch");
> + }
withtout actually walking the chain.
2) make equals_wpa to walk TYPE_ARG_TYPES of the function type and match them.
This is because DECL_ARGUMENTS are part of function body and before you
read it into memory, these are NULL
Walking DECL_ARGUMENTS here may cause ipa-icf to give up in case one body is
read (and thus have some arguments) and other is not.
3) make equals_private walk DECL_ARGUMENTS and match them
(well at the time you populate the map.)
You probalby can skip matching PARM_DECLS that are !parm_used_p (i)
for anything else than types_compatible_p.
We only care they are passed the same way by ABI. Everything else is not
relevant.
Honza
Re: [v3 patch] Fix some Filesystem TS operations
On 15/05/15 19:37 +0100, Jonathan Wakely wrote: I've also made filesystem::temp_directory_path() check the TMP, TEMPDIR and TEMP environment variables, as suggested in [fs.op.temp_dir_path] at http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n4099.html#fs.op.temp_dir_path rather than my original implementation which only checked TMPDIR (because I think that's the only one mentioned by POSIX). I'm not unsure whether it's better to follow the non-normative note or only check for the POSIX name. Anyone else have any thoughts? Oops, unintended double negative. I meant I'm unsure.
Re: [patch, c, ping] Fix PR c/48956: diagnostics for conversions involving complex types (reviewed)
On 05/15/2015 12:25 PM, Mikhail Maltsev wrote:
On 04/17/2015 05:01 AM, Mikhail Maltsev wrote:
I would like to ping the following patch:
https://gcc.gnu.org/ml/gcc-patches/2014-12/msg01925.html
Review: https://gcc.gnu.org/ml/gcc-patches/2015-01/msg02672.html
I fixed minor issues mentioned in the review and updated the changelog
message. Rebased on current trunk (r222157), bootstrapped and regtested
on x86_64-unknown-linux-gnu.
If it is OK for trunk, please assist with applying (I don't have write
access), and I will then create a new PR in bugzilla for the remaining
cases mentioned in review.
Since I now have write access, I am installing this patch into trunk. I
discovered and fixed a minor issue in testcase (no warning when
converting from long to int on -m32; changed long to long long. I hope
it's OK to be considered as obvious fix).
Bootstrapped/regtested on x86_64-unknown-linux-gnu {,-m32}.
And just for the record, Joseph approved this patch back in Jan, but
asked Mikhail to wait until stage1 development was open before installing.
jeff
Re: Fix RTL checking failure in emit_pattern_{after|before}_setloc
On 05/15/2015 12:39 PM, Eric Botcazou wrote:
You might consider a comment in active_insn_p and
emit_pattern_{after,before}_setloc so that if someone fixes
active_insn_p, they'll know to fix emit_pattern_{after,before}_setloc.
"FIXME" doesn't capture this interaction at all ;-)
I beg to differ, it's the same file and anyone fiddling with JUMP_TABLE_DATA_P
in active_insn_p should do a grep on the file/the entire RTL middle-end.
Fair enough.
Jeff
Re: [PATCH 1/2] dbgcnt: enhance behavior.
On 05/15/2015 04:36 AM, mliska wrote: Hi. Following patch changes GCC so that it exits after -fdbg-cnt-list and counters are sorted in dbgcnt.def file. Ready for trunk? Thanks, Martin gcc/ChangeLog: 2015-05-15 Martin Liska * dbgcnt.def: Sort counters. * opts.c (common_handle_option): Do not compile if -fdbg-cnt-list is enabled. OK. We're obviously assuming that nobody cares about the change in behaviour for -fdbg-cnt-list :-) jeff
Re: [PATCH 2/2] IPA ICF: add support for debug counter.
On 05/15/2015 03:03 AM, mliska wrote: Hi. Following patch add support of dbg-cnt for IPA ICF pass. Ready for trunk? Thanks, Martin gcc/ChangeLog: 2015-05-15 Martin Liska * dbgcnt.def: Add new counter. * ipa-icf.c (sem_item_optimizer::merge_classes): Use the counter. OK. jeff
Re: [PATCH i386] Allow sibcalls in no-PLT PIC
> On Fri, May 15, 2015 at 9:27 AM, Alexander Monakov wrote: > > Ping? Any comment about this patch? > > > > On Mon, 4 May 2015, Alexander Monakov wrote: > > > >> With -fno-plt, we don't have to reject even direct calls as sibcall > >> candidates. > >> > >> This patch depends on '-fplt' flag that is introduced in another patch. > >> > >> This patch requires that with -fno-plt all sibcall candidates go through > >> prepare_call_address that transforms the call to a GOT lookup. > >> > >> OK? > >> * config/i386/i386.c (ix86_function_ok_for_sibcall): Check flag_plt. > >> > >> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c > >> index f29e053..b734350 100644 > >> --- a/gcc/config/i386/i386.c > >> +++ b/gcc/config/i386/i386.c > >> @@ -5448,12 +5448,13 @@ ix86_function_ok_for_sibcall (tree decl, tree exp) > >>/* If we are generating position-independent code, we cannot sibcall > >> optimize any indirect call, or a direct call to a global function, > >> as the PLT requires %ebx be live. (Darwin does not have a PLT.) */ > >>if (!TARGET_MACHO > >>&& !TARGET_64BIT > >>&& flag_pic > >> + && flag_plt > >>&& (decl && !targetm.binds_local_p (decl))) > >> return false; > >> > >>/* If we need to align the outgoing stack, then sibcalling would > >> unalign the stack, which may break the called function. */ > >>if (ix86_minimum_incoming_stack_boundary (true) > >> > > I think it should be done via psABI change similar to > > https://groups.google.com/forum/#!topic/x86-64-abi/n8GYMpqvBxI > > which I have implemented on users/hjl/relax branch in binutils. OK, I am trying to understand how relax branch works and what difference it makes. As I underestand it, the main purpose is to be able to make relaxed call of call function that will, in 64bit mode, either result to RIP relative call with extra NOP just before the instruction if FUNCTION binds within the DSO or to indirect call through GOT bypassing the PLT. This saves overhead of PLT and increase every such call by extra NOP for no-LTO builds and even in LTO when the symbol is defined but interposable. This is actually really nice trick. Now this is about 32bit mode where explicit GOT pointer register is needed (how this work with large code model on x86-64?). It is needed by PLT, but I suppose to implement the same relaxation for 32bit it would need to use EBX to lookup the GOT pointer, too, so the check above would still be valid. The patches makes sense to be given that we support -fno-plt now. Honza > > -- > H.J.
Re: [PATCH i386] Allow sibcalls in no-PLT PIC
On Fri, May 15, 2015 at 12:48 PM, Jan Hubicka wrote: >> On Fri, May 15, 2015 at 9:27 AM, Alexander Monakov >> wrote: >> > Ping? Any comment about this patch? >> > >> > On Mon, 4 May 2015, Alexander Monakov wrote: >> > >> >> With -fno-plt, we don't have to reject even direct calls as sibcall >> >> candidates. >> >> >> >> This patch depends on '-fplt' flag that is introduced in another patch. >> >> >> >> This patch requires that with -fno-plt all sibcall candidates go through >> >> prepare_call_address that transforms the call to a GOT lookup. >> >> >> >> OK? >> >> * config/i386/i386.c (ix86_function_ok_for_sibcall): Check flag_plt. >> >> >> >> diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c >> >> index f29e053..b734350 100644 >> >> --- a/gcc/config/i386/i386.c >> >> +++ b/gcc/config/i386/i386.c >> >> @@ -5448,12 +5448,13 @@ ix86_function_ok_for_sibcall (tree decl, tree exp) >> >>/* If we are generating position-independent code, we cannot sibcall >> >> optimize any indirect call, or a direct call to a global function, >> >> as the PLT requires %ebx be live. (Darwin does not have a PLT.) */ >> >>if (!TARGET_MACHO >> >>&& !TARGET_64BIT >> >>&& flag_pic >> >> + && flag_plt >> >>&& (decl && !targetm.binds_local_p (decl))) >> >> return false; >> >> >> >>/* If we need to align the outgoing stack, then sibcalling would >> >> unalign the stack, which may break the called function. */ >> >>if (ix86_minimum_incoming_stack_boundary (true) >> >> >> >> I think it should be done via psABI change similar to >> >> https://groups.google.com/forum/#!topic/x86-64-abi/n8GYMpqvBxI >> >> which I have implemented on users/hjl/relax branch in binutils. > > OK, I am trying to understand how relax branch works and what difference it > makes. > As I underestand it, the main purpose is to be able to make relaxed call of > >call function > > that will, in 64bit mode, either result to RIP relative call with extra NOP > just > before the instruction if FUNCTION binds within the DSO or to indirect call > through > GOT bypassing the PLT. This saves overhead of PLT and increase every such > call > by extra NOP for no-LTO builds and even in LTO when the symbol is defined but > interposable. This is actually really nice trick. > > Now this is about 32bit mode where explicit GOT pointer register is needed > (how this work with large code model on x86-64?). It is needed by PLT, but I > suppose > to implement the same relaxation for 32bit it would need to use EBX to lookup > the > GOT pointer, too, so the check above would still be valid. > With relax branch in 32-bit, there are 2 cases: 1. PIC or PIE: We generate set up EBX relax call foo@PLT It is almost the same as we do now, except for the relax prefix. If foo is defined in another shared library or may be preempted, linker will generate call *foo@GOTPLT(%ebx) If foo turns out local, linker will output relax call foo 2. Non PIC/PIE: We generate relax call foo If foo is defined in a DSO, linker will generate call/jmp *foo@GOTPLT We don't set up EBX in this case. If foo turns out local, linker will output relax call foo -- H.J.
Re: [PATCH i386] Allow sibcalls in no-PLT PIC
On Fri, May 15, 2015 at 01:08:15PM -0700, H.J. Lu wrote: > With relax branch in 32-bit, there are 2 cases: > > 1. PIC or PIE: We generate > > set up EBX > relax call foo@PLT > > It is almost the same as we do now, except for the relax prefix. > If foo is defined in another shared library or may be preempted, > linker will generate > > call *foo@GOTPLT(%ebx) > > If foo turns out local, linker will output > > relax call foo This does not address the initial and primary motivation for no-plt on 32-bit: eliminating the awful codegen constraint costs of the GOT-register (ebx, and equivalent on other targets) ABI for calling PLT entries. If instead you generated code that sets up an expression for the GOT slot using arbitrary registers, and relaxed it to a direct call (possibly rendering the register setup useless), it would be comparable to the no-plt approach. So for example: set up ecx (or whatever register) relax call *foo@GOT(%ecx) and relax to: set up ecx (or whatever register; now useless) relax call foo But the no-plt approach is still superior in that the address load from the GOT can be hoisted out of loops, etc., resulting in something like: call *%esi This could be valuable in loops calling a math function repeatedly, for example. Overall I'm still not a fan of the relaxation approach. There are very few places it would actually help that couldn't already be improved better with use of visibility, and it can't give codegen as good as no-plt option. Rich
Re: [PATCH i386] Allow sibcalls in no-PLT PIC
On Fri, May 15, 2015 at 1:23 PM, Rich Felker wrote: > On Fri, May 15, 2015 at 01:08:15PM -0700, H.J. Lu wrote: >> With relax branch in 32-bit, there are 2 cases: >> >> 1. PIC or PIE: We generate >> >> set up EBX >> relax call foo@PLT >> >> It is almost the same as we do now, except for the relax prefix. >> If foo is defined in another shared library or may be preempted, >> linker will generate >> >> call *foo@GOTPLT(%ebx) >> >> If foo turns out local, linker will output >> >> relax call foo > > This does not address the initial and primary motivation for no-plt on > 32-bit: eliminating the awful codegen constraint costs of the > GOT-register (ebx, and equivalent on other targets) ABI for calling > PLT entries. If instead you generated code that sets up an expression > for the GOT slot using arbitrary registers, and relaxed it to a direct > call (possibly rendering the register setup useless), it would be > comparable to the no-plt approach. So for example: > > set up ecx (or whatever register) > relax call *foo@GOT(%ecx) > > and relax to: > > set up ecx (or whatever register; now useless) > relax call foo > > But the no-plt approach is still superior in that the address load > from the GOT can be hoisted out of loops, etc., resulting in something > like: > > call *%esi > > This could be valuable in loops calling a math function repeatedly, > for example. > > Overall I'm still not a fan of the relaxation approach. There are very > few places it would actually help that couldn't already be improved > better with use of visibility, and it can't give codegen as good as > no-plt option. With no-plt option, compiler has to know if a function is external or may be preempted. If compiler guessed wrong, the generated DSO or executable will always go through indirect branch even though the target is local. With relax branch, the decision is left to linker. Of course, EBX must be used unless we add a new PLT relocation for each register used to to hold GOT base, like relax call foo@PLT_ECX relax call foo@PLT_EDX ... -- H.J.
Re: [PATCH] fortran/66057 -- detect malformed GENERIC statement
Does it also catch the other cases shown in the PR? Things like: generic :: 2 generic :: 2 = generic :: 1 => x generic :: ? etc. OK if so. If not, maybe it’s worth adding them at the same time? FX
Re: [PATCH] fortran/66040 -- convert internal error into normal error
> 2015-05-XX Steven G. Kargl > > PR fortran/66040 > * parse.c(verify_st_order): Replace a gfc_internal_error with your > generic gfc_error. > > 2015-05-XX Steven G. Kargl > > PR fortran/66040 > * gfortran.dg/misplaced_statement.f90: New test. OK
Re: [PATCH] fortran/66039 -- Check for mangled file positioning
> 2015-05-XX Steven G. Kargl > > PR fortran/66039 > * io.c (match_filepos): Check for incomplete/mangled REWIND, FLUSH, > BACKSPACE, and ENDFILE statements > > 2015-05-XX Steven G. Kargl > > PR fortran/66039 > * gfortran.dg/filepos1.f90: New test OK
Re: [PATCH i386] Allow sibcalls in no-PLT PIC
On Fri, May 15, 2015 at 01:35:14PM -0700, H.J. Lu wrote: > On Fri, May 15, 2015 at 1:23 PM, Rich Felker wrote: > > On Fri, May 15, 2015 at 01:08:15PM -0700, H.J. Lu wrote: > >> With relax branch in 32-bit, there are 2 cases: > >> > >> 1. PIC or PIE: We generate > >> > >> set up EBX > >> relax call foo@PLT > >> > >> It is almost the same as we do now, except for the relax prefix. > >> If foo is defined in another shared library or may be preempted, > >> linker will generate > >> > >> call *foo@GOTPLT(%ebx) > >> > >> If foo turns out local, linker will output > >> > >> relax call foo > > > > This does not address the initial and primary motivation for no-plt on > > 32-bit: eliminating the awful codegen constraint costs of the > > GOT-register (ebx, and equivalent on other targets) ABI for calling > > PLT entries. If instead you generated code that sets up an expression > > for the GOT slot using arbitrary registers, and relaxed it to a direct > > call (possibly rendering the register setup useless), it would be > > comparable to the no-plt approach. So for example: > > > > set up ecx (or whatever register) > > relax call *foo@GOT(%ecx) > > > > and relax to: > > > > set up ecx (or whatever register; now useless) > > relax call foo > > > > But the no-plt approach is still superior in that the address load > > from the GOT can be hoisted out of loops, etc., resulting in something > > like: > > > > call *%esi > > > > This could be valuable in loops calling a math function repeatedly, > > for example. > > > > Overall I'm still not a fan of the relaxation approach. There are very > > few places it would actually help that couldn't already be improved > > better with use of visibility, and it can't give codegen as good as > > no-plt option. > > With no-plt option, compiler has to know if a function is external > or may be preempted. I still don't see significant practical cases where the linker would know this but the compiler can't. If you use visibility properly, the compiler knows, and if you do LTO and -Bsymbolic[-functions], the compiler should have that information available at LTO time (this is an enhancement that needs to be made, though). > If compiler guessed wrong, the generated > DSO or executable will always go through indirect branch even > though the target is local. The only way this is avoided now is with -Bsymbolic[-functions] which is not widely used. Otherwise interposition is always allowed for default-visibility functions, so I don't see how the indirect branch here is suboptimal. > With relax branch, the decision is left > to linker. Of course, EBX must be used unless we add a new PLT > relocation for each register used to to hold GOT base, like > > relax call foo@PLT_ECX > relax call foo@PLT_EDX No, that's not needed. If the linker doesn't make the relaxation, the instruction the compiler generated remains in place, and has the effective address expression using whichever register it wanted: relax call *foo@GOT(%ecx) relax call *foo@GOT(%edx) etc. If the linker chooses to relax it to a direct call, no register at all is needed, so the linker can just throw this away and use: call foo for all of them. Rich
Re: [PATCH] fortran/66044 -- Convert ICE into ordinary error
> 2015-05-XX Steven G. Kargl > > PR fortran/66044 > * decl.c(gfc_match_entry): Change a gfc_internal_error() into > a gfc_error() > > 2015-05-XX Steven G. Kargl > > PR fortran/66044 > * gfortran.dg/entry_21.f90: New test. OK
Re: Last part of verify_type_vairant checks
> - Ada builds !TYPE_ARTIFICIAL variant of TYPE_ARTIFICIAL type.
>Eric, I can not really make sense of this. Is this expected?
>If we have artificial type, how it can become not?
Yes, that's a bit unexpected and probably comes from the case where we build a
variant and it's not really appropriate to do so (gnat_pushdecl):
/* ??? Copy and original type are not supposed to be variant but we
really need a variant for the placeholder machinery to work. */
if (TYPE_IS_FAT_POINTER_P (t))
tt = build_variant_type_copy (t);
else
{
/* TYPE_NEXT_PTR_TO is a chain of main variants. */
tt = build_distinct_type_copy (TYPE_MAIN_VARIANT (t));
if (TREE_CODE (t) == POINTER_TYPE)
TYPE_NEXT_PTR_TO (TYPE_MAIN_VARIANT (t)) = tt;
tt = build_qualified_type (tt, TYPE_QUALS (t));
}
On the other hand, if the type declared in the source code is volatile, one
could imagine that the non-volatile main variant be TYPE_ARTIFICIAL and the
declared type be !TYPE_ARTIFICIAL.
> - During Ada builds there are types that differ in TYPE_SIZES_GIMPLIFIED.
>This is probably harmless but I am not sure it ishappen to happen.
That it's harmless I'm sure, otherwise it would already have bitten us. :-)
But I agree that it would be better to always gimplify the main variant first
and propagate to the other variants, as done in gimplify_type_sizes.
If you have a patchlet for this one, you can open a PR and attach it.
--
Eric Botcazou
Re: [PATCH] [PATCH][ARM] Fix thumb1-far-jump-2.c testcase.
On May 15, 2015, at 11:07 AM, Alex Velenko wrote: > This patch fixes testcase thumb1-far-jump-2.c to confirm to newer compilation > defaults. > > Is patch ok? Ok.
Re: [PATCH i386] Allow sibcalls in no-PLT PIC
On Fri, May 15, 2015 at 1:42 PM, Rich Felker wrote:
> On Fri, May 15, 2015 at 01:35:14PM -0700, H.J. Lu wrote:
>> On Fri, May 15, 2015 at 1:23 PM, Rich Felker wrote:
>> > On Fri, May 15, 2015 at 01:08:15PM -0700, H.J. Lu wrote:
>> >> With relax branch in 32-bit, there are 2 cases:
>> >>
>> >> 1. PIC or PIE: We generate
>> >>
>> >> set up EBX
>> >> relax call foo@PLT
>> >>
>> >> It is almost the same as we do now, except for the relax prefix.
>> >> If foo is defined in another shared library or may be preempted,
>> >> linker will generate
>> >>
>> >> call *foo@GOTPLT(%ebx)
>> >>
>> >> If foo turns out local, linker will output
>> >>
>> >> relax call foo
>> >
>> > This does not address the initial and primary motivation for no-plt on
>> > 32-bit: eliminating the awful codegen constraint costs of the
>> > GOT-register (ebx, and equivalent on other targets) ABI for calling
>> > PLT entries. If instead you generated code that sets up an expression
>> > for the GOT slot using arbitrary registers, and relaxed it to a direct
>> > call (possibly rendering the register setup useless), it would be
>> > comparable to the no-plt approach. So for example:
>> >
>> > set up ecx (or whatever register)
>> > relax call *foo@GOT(%ecx)
>> >
>> > and relax to:
>> >
>> > set up ecx (or whatever register; now useless)
>> > relax call foo
>> >
>> > But the no-plt approach is still superior in that the address load
>> > from the GOT can be hoisted out of loops, etc., resulting in something
>> > like:
>> >
>> > call *%esi
>> >
>> > This could be valuable in loops calling a math function repeatedly,
>> > for example.
>> >
>> > Overall I'm still not a fan of the relaxation approach. There are very
>> > few places it would actually help that couldn't already be improved
>> > better with use of visibility, and it can't give codegen as good as
>> > no-plt option.
>>
>> With no-plt option, compiler has to know if a function is external
>> or may be preempted.
>
> I still don't see significant practical cases where the linker would
> know this but the compiler can't. If you use visibility properly, the
> compiler knows, and if you do LTO and -Bsymbolic[-functions], the
> compiler should have that information available at LTO time (this is
> an enhancement that needs to be made, though).
There are codes like
extern void foo (void);
void
bar (void)
{
foo ();
}
Even with LTO, compiler may have to assume foo is external
when foo is compiled with LTO.
>> If compiler guessed wrong, the generated
>> DSO or executable will always go through indirect branch even
>> though the target is local.
>
> The only way this is avoided now is with -Bsymbolic[-functions] which
> is not widely used. Otherwise interposition is always allowed for
> default-visibility functions, so I don't see how the indirect branch
> here is suboptimal.
Relax branch is to avoid indirect branch to local targets. If
you don't think indirect branch to local targets is a performance
issue, relax branch isn't for you.
>> With relax branch, the decision is left
>> to linker. Of course, EBX must be used unless we add a new PLT
>> relocation for each register used to to hold GOT base, like
>>
>> relax call foo@PLT_ECX
>> relax call foo@PLT_EDX
>
> No, that's not needed. If the linker doesn't make the relaxation, the
> instruction the compiler generated remains in place, and has the
> effective address expression using whichever register it wanted:
>
> relax call *foo@GOT(%ecx)
> relax call *foo@GOT(%edx)
> etc.
relax branch is only used for direct branch and it isn't for indirect
branch. I will implement
relax call foo@PLT(%reg)
The compiler can pick any registers to hold GOT base. Lazy
binding is supported only when EBX is used.
> If the linker chooses to relax it to a direct call, no register at all
> is needed, so the linker can just throw this away and use:
>
> call foo
>
> for all of them.
>
> Rich
--
H.J.
[match-and-simplify] fix incorrect code-gen in 'for' pattern
Hi, genmatch generates incorrect code for following (artificial) pattern: (for op (plus) op2 (op) (simplify (op @x @y) (op2 @x @y) generated gimple code: http://pastebin.com/h1uau9qB 'op' is not replaced in the generated code on line 33: *res_code = op; I think it would be a better idea to make op2 iterate over same set of operators (op2->substitutes = op->substitutes). I have attached patch for the same. Bootstrap + testing in progress on x86_64-unknown-linux-gnu. OK for trunk after bootstrap+testing completes ? I wonder if we really need is_oper_list flag in user_id ? We can determine if user_id is an operator list if user_id::substitutes is not empty ? That will lose the ability to distinguish between user-defined operator list and list-iterator in for like op/op2, but I suppose we (so far) don't need to distinguish between them ? Thanks, Prathamesh Index: genmatch.c === --- genmatch.c (revision 223225) +++ genmatch.c (working copy) @@ -3329,7 +3329,7 @@ "others with arity %d", oper, idb->nargs, arity); user_id *p = dyn_cast (idb); - if (p && p->is_oper_list) + if (p && !p->substitutes.is_empty()) // p is either user-defined operator list or a list iterator op->substitutes.safe_splice (p->substitutes); else op->substitutes.safe_push (idb); ChangeLog Description: Binary data
Re: [PATCH] Handle OS X deployment targets correctly
On May 14, 2015, at 11:42 PM, Lawrence Velázquez wrote: > As described in PR target/63810, this addresses several problems > With this change, trunk matches the behavior of Apple LLVM Compiler > 6.1.0 on 8,451 of 8,464 generated test inputs. (The discrepancies are > due to a bug in Clang.) I don't notice any testsuite regressions on > OS X 10.10 Yosemite x86-64. Ok. You will need to repost as an attachment, as Apple never quite figured out how to send email. Hopes springs eternal that one day, they might wake up and figure out how to send email.
Re: [PATCH] fortran/64925 -- fix conflict between names
> 2015-05-XX Steven G. Kargl > > PR fortran/64925 > * symbol.c(check_conflict): Check for a conflict between a dummy > argument and an internal procedure name. > > 2015-05-XX Steven G. Kargl > > PR fortran/64925 > * gfortran.dg/pr64925.f90: New test. OK
Re: [PATCH i386] Allow sibcalls in no-PLT PIC
Hello,
>
> There are codes like
>
> extern void foo (void);
>
> void
> bar (void)
> {
> foo ();
> }
>
> Even with LTO, compiler may have to assume foo is external
> when foo is compiled with LTO.
This is not exactly true if FOO is defined in other translation unit
compiled with LTO and hidden visibility.
OK, so as I get it, we get the following cases:
1) compiler knows it is generating call to a local symbol a current
unit (binds_to_current_def_p returns true).
We handle this correctly by doing IP relative call.
2) compiler knows it is generating call to a local symbol in DSO
(binds_local_p return true)
Currently I think this is only the -fno-pic case or case of explicit
hidden visibility and in this case we do IP relative call.
We may want to propose plugin API update adding PREVAILING_DEF_EXP.
So copiler would be able to default to this case for PREVAILING_DEF
and we will also catch cases where the symbol is defined in current
DSO as weak symbol, but the definition is not LTO.
This would be also way to communicate -Bsymbolic[-functions] across
the plugin API.
3) compiler knows there is going to be definition in the current DSO
(by seeing a COMDAT function body or resolution info) that is interposable
but because the function is inline or -fno-semantic-interposition happens,
the semantics will not change.
In this case it would be nice to arrange IP relative call to the
hidden alias. This may require an extension both on compiler and linker
side.
I was thinking of doing so for comdats by adding hidden alias with
fixed mangling, like __gnu_.hiddenalias, and referring it.
But I think it is not safe as linker may throw away section that
is produced by GCC and prevail section that is not leaving to an undefined
symbol?
I think this is rather common case in C++ (never made any stats) because
uninlined comdats are quite common.
4) compiler has no clue but linker may know better
Here we traditionally always produce a PLT call. In cases the call
is known to be hot in the program it makes sense to trade lazy binding
for performance and produce call via GOT reference (-fno-plt).
I also see that H.J.'s branch helps us to actually avoid the GOT
reference in cases the symbol ends up binding locally. How the lazy
binding with relaxation works?
We may try to communicate down the information whether the symbol can
or can not semantically interpose to the linker, so it can do
-Bsymbolic by default for inline and COMDAT functions.
Actually perhaps the linker can just default to this for all comdat
defined symbols?
I think it still make sense to work on non-LTO codegen improvements.
As much as I would like everyone to LTO and FDO, most people don't.
5) Compiler knows it is generating call to external function.
We do not special case this, but we could add binds_external_p and
make it to determine this case from resolution info during LTO.
I do not see if this case is any different from 4 from PIC codegen
perspective except that perhaps the relax relocation will allow us to lazy
bind?
Honza
Re: [PATCH i386] Allow sibcalls in no-PLT PIC
On Fri, May 15, 2015 at 4:08 PM, Jan Hubicka wrote:
> Hello,
>>
>> There are codes like
>>
>> extern void foo (void);
>>
>> void
>> bar (void)
>> {
>> foo ();
>> }
>>
>> Even with LTO, compiler may have to assume foo is external
>> when foo is compiled with LTO.
>
> This is not exactly true if FOO is defined in other translation unit
> compiled with LTO and hidden visibility.
I was meant to say " when foo is compiled without LTO.".
> OK, so as I get it, we get the following cases:
>
> 1) compiler knows it is generating call to a local symbol a current
> unit (binds_to_current_def_p returns true).
>
> We handle this correctly by doing IP relative call.
>
> 2) compiler knows it is generating call to a local symbol in DSO
> (binds_local_p return true)
> Currently I think this is only the -fno-pic case or case of explicit
> hidden visibility and in this case we do IP relative call.
>
> We may want to propose plugin API update adding PREVAILING_DEF_EXP.
> So copiler would be able to default to this case for PREVAILING_DEF
> and we will also catch cases where the symbol is defined in current
> DSO as weak symbol, but the definition is not LTO.
> This would be also way to communicate -Bsymbolic[-functions] across
> the plugin API.
>
> 3) compiler knows there is going to be definition in the current DSO
> (by seeing a COMDAT function body or resolution info) that is interposable
> but because the function is inline or -fno-semantic-interposition happens,
> the semantics will not change.
>
> In this case it would be nice to arrange IP relative call to the
> hidden alias. This may require an extension both on compiler and linker
> side.
>
> I was thinking of doing so for comdats by adding hidden alias with
> fixed mangling, like __gnu_.hiddenalias, and referring it.
> But I think it is not safe as linker may throw away section that
> is produced by GCC and prevail section that is not leaving to an undefined
> symbol?
>
> I think this is rather common case in C++ (never made any stats) because
> uninlined comdats are quite common.
>
> 4) compiler has no clue but linker may know better
>
> Here we traditionally always produce a PLT call. In cases the call
> is known to be hot in the program it makes sense to trade lazy binding
> for performance and produce call via GOT reference (-fno-plt).
> I also see that H.J.'s branch helps us to actually avoid the GOT
> reference in cases the symbol ends up binding locally. How the lazy
> binding with relaxation works?
If there is no GOT slot allocated for symbol foo, linker should resolve
foo@GOTPLT(%ebx) to to its PLT slot address + 6, which is the push
instruction, to support lazy binding. Otherwise, linker should resolve it
to its GOT slot address.
> We may try to communicate down the information whether the symbol can
> or can not semantically interpose to the linker, so it can do
> -Bsymbolic by default for inline and COMDAT functions.
> Actually perhaps the linker can just default to this for all comdat
> defined symbols?
>
> I think it still make sense to work on non-LTO codegen improvements.
> As much as I would like everyone to LTO and FDO, most people don't.
>
> 5) Compiler knows it is generating call to external function.
> We do not special case this, but we could add binds_external_p and
> make it to determine this case from resolution info during LTO.
>
> I do not see if this case is any different from 4 from PIC codegen
> perspective except that perhaps the relax relocation will allow us to lazy
> bind?
My relax branch proposal works even without LTO.
--
H.J.
Re: [PATCH i386] Allow sibcalls in no-PLT PIC
On Fri, May 15, 2015 at 4:14 PM, H.J. Lu wrote: > My relax branch proposal works even without LTO. > I will borrow GOTPCREL from x86-64 and do [hjl@gnu-6 relax-4]$ cat b.S call *foo@GOTPCREL(%eax) [hjl@gnu-6 relax-4]$ ./as -32 -o b.o b.S [hjl@gnu-6 relax-4]$ ./objdump -dwr b.o b.o: file format elf32-i386 Disassembly of section .text: <.text>: 0: ff 90 fc ff ff ff call *-0x4(%eax) 2: R_386_RELAX_GOT32 foo [hjl@gnu-6 relax-4]$ And linker can turn it into relax call foo if foo is defined locally. -- H.J.
Re: [PATCH i386] Allow sibcalls in no-PLT PIC
On Fri, May 15, 2015 at 4:30 PM, H.J. Lu wrote: > On Fri, May 15, 2015 at 4:14 PM, H.J. Lu wrote: >> My relax branch proposal works even without LTO. >> > > I will borrow GOTPCREL from x86-64 and do > > [hjl@gnu-6 relax-4]$ cat b.S > call *foo@GOTPCREL(%eax) call *foo@GOTPLT(%eax) is a better choice. > [hjl@gnu-6 relax-4]$ ./as -32 -o b.o b.S > [hjl@gnu-6 relax-4]$ ./objdump -dwr b.o > > b.o: file format elf32-i386 > > > Disassembly of section .text: > > <.text>: >0: ff 90 fc ff ff ff call *-0x4(%eax) 2: R_386_RELAX_GOT32 foo > [hjl@gnu-6 relax-4]$ > > And linker can turn it into > > relax call foo > > if foo is defined locally. -- H.J.
Re: [PATCH i386] Allow sibcalls in no-PLT PIC
On Fri, May 15, 2015 at 04:14:07PM -0700, H.J. Lu wrote:
> On Fri, May 15, 2015 at 4:08 PM, Jan Hubicka wrote:
> > Hello,
> >>
> >> There are codes like
> >>
> >> extern void foo (void);
> >>
> >> void
> >> bar (void)
> >> {
> >> foo ();
> >> }
> >>
> >> Even with LTO, compiler may have to assume foo is external
> >> when foo is compiled with LTO.
> >
> > This is not exactly true if FOO is defined in other translation unit
> > compiled with LTO and hidden visibility.
>
> I was meant to say " when foo is compiled without LTO.".
>
> > OK, so as I get it, we get the following cases:
> >
> > 1) compiler knows it is generating call to a local symbol a current
> > unit (binds_to_current_def_p returns true).
> >
> > We handle this correctly by doing IP relative call.
> >
> > 2) compiler knows it is generating call to a local symbol in DSO
> > (binds_local_p return true)
> > Currently I think this is only the -fno-pic case or case of explicit
> > hidden visibility and in this case we do IP relative call.
> >
> > We may want to propose plugin API update adding PREVAILING_DEF_EXP.
> > So copiler would be able to default to this case for PREVAILING_DEF
> > and we will also catch cases where the symbol is defined in current
> > DSO as weak symbol, but the definition is not LTO.
> > This would be also way to communicate -Bsymbolic[-functions] across
> > the plugin API.
> >
> > 3) compiler knows there is going to be definition in the current DSO
> > (by seeing a COMDAT function body or resolution info) that is
> > interposable
> > but because the function is inline or -fno-semantic-interposition
> > happens,
> > the semantics will not change.
> >
> > In this case it would be nice to arrange IP relative call to the
> > hidden alias. This may require an extension both on compiler and linker
> > side.
> >
> > I was thinking of doing so for comdats by adding hidden alias with
> > fixed mangling, like __gnu_.hiddenalias, and referring it.
> > But I think it is not safe as linker may throw away section that
> > is produced by GCC and prevail section that is not leaving to an
> > undefined
> > symbol?
> >
> > I think this is rather common case in C++ (never made any stats) because
> > uninlined comdats are quite common.
> >
> > 4) compiler has no clue but linker may know better
> >
> > Here we traditionally always produce a PLT call. In cases the call
> > is known to be hot in the program it makes sense to trade lazy binding
> > for performance and produce call via GOT reference (-fno-plt).
> > I also see that H.J.'s branch helps us to actually avoid the GOT
> > reference in cases the symbol ends up binding locally. How the lazy
> > binding with relaxation works?
>
> If there is no GOT slot allocated for symbol foo, linker should resolve
> foo@GOTPLT(%ebx) to to its PLT slot address + 6, which is the push
> instruction, to support lazy binding. Otherwise, linker should resolve it
> to its GOT slot address.
Forget lazy binding. It's dead anyway because serious distros want
PIE+relro+bindnow+... If people really want lazy binding, they can use
options which support it, but I don't want to keep suffering the
codegen cost of lazy binding despite never using it. There should be
an option to generate optimal code equivalent to what you get with
Alexander Monakov's patches for those of us who aren't trying to
support this legacy feature that precludes good performance and
precludes hardening.
Rich
Re: [PATCH i386] Allow sibcalls in no-PLT PIC
On Fri, May 15, 2015 at 04:34:57PM -0700, H.J. Lu wrote: > On Fri, May 15, 2015 at 4:30 PM, H.J. Lu wrote: > > On Fri, May 15, 2015 at 4:14 PM, H.J. Lu wrote: > >> My relax branch proposal works even without LTO. > >> > > > > I will borrow GOTPCREL from x86-64 and do > > > > [hjl@gnu-6 relax-4]$ cat b.S > > call *foo@GOTPCREL(%eax) > > call *foo@GOTPLT(%eax) > > is a better choice. foo@GOTPCREL is preferable (but does not yet exist for ia32, so the reloc type would have to be added) since it saves a useless add. Instead of: call __x86.get_pc_thunk.ax addl $_GLOBAL_OFFSET_TABLE_, %eax call *foo@GOTPLT(%eax) you can just do: call __x86.get_pc_thunk.ax call *foo@GOTPCREL(%eax) Note that it also works to have extra instructions between: call __x86.get_pc_thunk.ax 1: ... call *foo@GOTPCREL+(1b-.)(%eax) I may not have gotten the syntax quite right, but hopefully yoy get the idea. This same approach (with GOTPCREL) can be used for _all_ GOT accesses, including global data, to eliminate the useless add. Rich
Re: [patch] Clean up detection of SJLJ exceptions in target libraries
On Wed, May 13, 2015 at 10:52 AM, Eric Botcazou wrote: >> The libgo parts are fine, but since libgo is mirrored from an external >> repository I'll commit those parts myself. > > Thanks! > >> I assume I can go ahead and commit them now? > > Yes, you can, the libgo bits are independent. libgo portion of this patch committed to mainline. Ian
Re: [RFC]: Remove Mem/address type assumption in combiner
On Fri, 1 May 2015, Segher Boessenkool wrote:
> On Wed, Apr 29, 2015 at 12:03:35PM -0500, Segher Boessenkool wrote:
> > On Wed, Apr 29, 2015 at 09:25:21AM +, Kumar, Venkataramanan wrote:
> > > diff --git a/gcc/combine.c b/gcc/combine.c
> > > index 5c763b4..945abdb 100644
> > > --- a/gcc/combine.c
> > > +++ b/gcc/combine.c
> > > @@ -7703,8 +7703,6 @@ make_compound_operation (rtx x, enum rtx_code
> > > in_code)
> > > but once inside, go back to our default of SET. */
> > >
> > >next_code = (code == MEM ? MEM
> > > - : ((code == PLUS || code == MINUS)
> > > - && SCALAR_INT_MODE_P (mode)) ? MEM
> > >: ((code == COMPARE || COMPARISON_P (x))
> > > && XEXP (x, 1) == const0_rtx) ? COMPARE
> > >: in_code == COMPARE ? SET : in_code);
> > >
> > >
> > > On X86_64, it passes bootstrap and regression tests.
> > > But on Aarch64 the test in PR passed, but I got a few test case failures.
> >
> > > There are few patterns based on multiplication operations in Aarch64
> > > backend which are used to match with the pattern combiner generated.
> > > Now those patterns have to be fixed to use SHIFTS. Also need to see any
> > > impact on other targets.
> >
> > Right. It would be good if you could find out for what targets it matters.
> > The thing is, if a target expects only the patterns as combine used to make
> > them, it will regress (as you've seen on aarch64); but it will regress
> > _silently_. Which isn't so nice.
> >
> > > But before that I wanted to check if the assumption in combiner, can
> > > simply be removed ?
> >
> > Seeing for what targets / patterns it makes a difference would tell us the
> > answer to that, too :-)
> >
> > I'll run some tests with and without your patch.
>
> So I ran the tests. These are text sizes for vmlinux built for most
> architectures (before resp. after the patch).
Thanks for actually checking the impact.
> Results are good, but
> they show many targets need some work...
>
>
> BIGGER
> 2212322 2212706 cris
Also observable as noted in PR66171, a regression:
Running /tmp/hpautotest-gcc0/gcc/gcc/testsuite/gcc.target/cris/cris.exp
...
FAIL: gcc.target/cris/biap.c scan-assembler addi
FAIL: gcc.target/cris/biap.c scan-assembler-not lsl
I confess the test-case-"guarded" addi pattern should have been
expressed with a shift in addition to the multiplication. ("In
addition to" as the canonically wrong one used to be the
combine-matching pattern; I'm not sure I should really drop that
just yet.) I'll have to check that expressing using a shift
fixes this issue.
Supposedly more noteworthy: this now-stricter canonicalization
leads to a requirement to rewrite patterns that used to be:
(parallel
[(set reg0 (mem (plus (mult reg1 N) reg2)))
(set reg3 (plus (mult reg1 N) reg2))])
into the awkwardly asymmetric:
(parallel
[(set reg0 (mem (plus (mult reg1 2) reg2)))
(set reg3 (plus (ashift reg1 M) reg2))])
(where M = log2 N)
and of course a need to verify that combine actually *does* make
use of the pattern after the change at least as much as it did
before.
> Some targets need some love (but what else is new).
Meh. Well, you now got some whine to go with that cheese.
brgds, H-P
Re: Mostly rewrite genrecog
Hi Richard, I see regressions with the current IBM z13 vector patchset which appear to be related to the new genrecog. The following two insn definitions only differ in the mode and predicate of the shift count operand. (define_insn "lshr3" [(set (match_operand:VI 0 "register_operand" "=v") (lshiftrt:VI (match_operand:VI 1 "register_operand" "v") (match_operand:SI 2 "shift_count_or_setmem_operand" "Y")))] "TARGET_VX" "vesrl\t%v0,%v1,%Y2" [(set_attr "op_type" "VRS")]) (define_insn "vlshr3" [(set (match_operand:VI 0 "register_operand" "=v") (lshiftrt:VI (match_operand:VI 1 "register_operand" "v") (match_operand:VI 2 "register_operand" "v")))] "TARGET_VX" "vesrlv\t%v0,%v1,%v2" [(set_attr "op_type" "VRR")]) However, the insn-recog.c code only seem to check the predicate. This is a problem since shift_count_or_setmem_operand does not check the mode. if (shift_count_or_setmem_operand (operands[2], SImode) && #line 717 "/home3/andreas/patched/../gcc/gcc/config/s390/vector.md" (TARGET_VX)) return 600; /* lshrv2qi3 */ if (register_operand (operands[2], V2QImode) && #line 747 "/home3/andreas/patched/../gcc/gcc/config/s390/vector.md" (TARGET_VX)) return 630; /* vlshrv2qi3 */ break; I could add a mode check to the predicate. However, I just wanted to check whether this change was intentional. Bye, -Andreas-
