[committed] Move array bounds checking out of vrp_prop and into its own class.
Howdy.
There's really no reason why the array bounds code should live in
tree-vrp, when all it really needs is a get_value_range() call back.
Particularly painful is that it partially lives inside the vrp_prop class.
This patch moves the array bounds code into its own class, while taking
in a vr_values in the constructor which is ONLY used to get at
get_value_range().
It is my ultimate desire to move this class into its own file (as a
follow-up), minimizing distraction in tree-vrp.c. It can take a
vr_values (or similar object), and can be made to work with any ranger
(evrp, vrp, or ranger).
Approved by Jeff, and committed to mainline.
Aldy
commit 65d44272bd988f695a9b5fa7e1b88c266c3089cb
Author: Aldy Hernandez
Date: Tue May 5 18:40:44 2020 +0200
Move array bounds checking out of vrp_prop and into its own class.
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 4f48d443235..64681dd97fe 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,15 @@
+2020-05-17 Aldy Hernandez
+
+ * tree-vrp.c (class vrp_prop): Move check_all_array_refs,
+ check_array_ref, check_mem_ref, and search_for_addr_array
+ into new class...
+ (class array_bounds_checker): ...here.
+ (class check_array_bounds_dom_walker): Adjust to use
+ array_bounds_checker.
+ (check_all_array_refs): Move into array_bounds_checker and rename
+ to check.
+ (class vrp_folder): Make fold_predicate_in private.
+
2020-05-15 Jeff Law
* config/h8300/h8300.md (SFI iterator): New iterator for
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index 1001c7f41d8..6e3510856a5 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -3521,7 +3521,7 @@ vrp_insert::insert_range_assertions (void)
class vrp_prop : public ssa_propagation_engine
{
- public:
+public:
enum ssa_prop_result visit_stmt (gimple *, edge *, tree *) FINAL OVERRIDE;
enum ssa_prop_result visit_phi (gphi *) FINAL OVERRIDE;
@@ -3529,12 +3529,10 @@ class vrp_prop : public ssa_propagation_engine
void vrp_initialize (struct function *);
void vrp_finalize (bool);
- void check_all_array_refs (void);
- bool check_array_ref (location_t, tree, bool);
- bool check_mem_ref (location_t, tree, bool);
- void search_for_addr_array (tree, location_t);
class vr_values vr_values;
+
+private:
/* Temporary delegator to minimize code churn. */
const value_range_equiv *get_value_range (const_tree op)
{ return vr_values.get_value_range (op); }
@@ -3552,6 +3550,29 @@ class vrp_prop : public ssa_propagation_engine
void extract_range_from_phi_node (gphi *phi, value_range_equiv *vr)
{ vr_values.extract_range_from_phi_node (phi, vr); }
};
+
+/* Array bounds checking pass. */
+
+class array_bounds_checker
+{
+ friend class check_array_bounds_dom_walker;
+
+public:
+ array_bounds_checker (struct function *fun, class vr_values *v)
+: fun (fun), ranges (v) { }
+ void check ();
+
+private:
+ static tree check_array_bounds (tree *tp, int *walk_subtree, void *data);
+ bool check_array_ref (location_t, tree, bool ignore_off_by_one);
+ bool check_mem_ref (location_t, tree, bool ignore_off_by_one);
+ void check_addr_expr (location_t, tree);
+ const value_range_equiv *get_value_range (const_tree op)
+{ return ranges->get_value_range (op); }
+ struct function *fun;
+ class vr_values *ranges;
+};
+
/* Checks one ARRAY_REF in REF, located at LOCUS. Ignores flexible arrays
and "struct" hacks. If VRP can determine that the
array subscript is a constant, check if it is outside valid
@@ -3561,8 +3582,8 @@ class vrp_prop : public ssa_propagation_engine
Returns true if a warning has been issued. */
bool
-vrp_prop::check_array_ref (location_t location, tree ref,
- bool ignore_off_by_one)
+array_bounds_checker::check_array_ref (location_t location, tree ref,
+ bool ignore_off_by_one)
{
if (TREE_NO_WARNING (ref))
return false;
@@ -3760,8 +3781,8 @@ vrp_prop::check_array_ref (location_t location, tree ref,
Returns true if a warning has been issued. */
bool
-vrp_prop::check_mem_ref (location_t location, tree ref,
- bool ignore_off_by_one)
+array_bounds_checker::check_mem_ref (location_t location, tree ref,
+ bool ignore_off_by_one)
{
if (TREE_NO_WARNING (ref))
return false;
@@ -4038,7 +4059,7 @@ vrp_prop::check_mem_ref (location_t location, tree ref,
address of an ARRAY_REF, and call check_array_ref on it. */
void
-vrp_prop::search_for_addr_array (tree t, location_t location)
+array_bounds_checker::check_addr_expr (location_t location, tree t)
{
/* Check each ARRAY_REF and MEM_REF in the reference chain. */
do
@@ -4122,14 +4143,12 @@ vrp_prop::search_for_addr_array (tree t, location_t location)
}
}
-/* walk_tree() callback that checks if *TP is
- an ARRAY_REF inside an ADDR_EXPR (in which an array
- subscript one outside the valid range is allowed). Call
- check_array_ref for each ARRAY_REF found. The location is
- passed in DATA. */
+/* Callback for walk_tree
Re: [PATCH] Refactor tree-vrp.c
Funny, I had queued up more or less the same changes on my tree, but
Giuliano beat me to it.
I am committing the attached patch that has been approved by Jeff.
I moved a couple more functions into the vrp_insert class, as well as
abstract out the liveness bitmap into its own class. I also renamed
the various dump/debug routines into overloaded methods.
Finally, I noticed the indentation for the class was off, and fixed it.
Committed to mainline.
Aldy
On Fri, May 8, 2020 at 6:21 PM Jeff Law via Gcc-patches
wrote:
>
> On Fri, 2020-05-08 at 13:06 -0300, Giuliano Belinassi via Gcc-patches wrote:
> > Hi,
> >
> > This patch Refactors tree-vrp.c to eliminate all global variables except
> > 'x_vrp_values', which will require that 'thread_outgoing_edges' to
> > accept an extra argument and pass it to the 'simplify' callback.
> >
> > It also removes every access to 'cfun', retrieving the function being
> > compiled from the pass engine.
> >
> > Bootstrapped and ran the testsuite on Linux x86_64.
> >
> > gcc/ChangeLog
> > 2020-05-08 Giuliano Belinassi
> >
> > * tree-vrp.c (class liveness): New.
> > (insert_range_assertions): Move to class liveness.
> > (dump_all_asserts): Same as above.
> > (dump_asserts_for): Same as above.
> > (live): Same as above.
> > (need_assert_for): Same as above.
> > (live_on_edge): Same as above.
> > (finish_register_edge_assert_for): Same as above.
> > (find_switch_asserts): Same as above.
> > (find_assert_locations): Same as above.
> > (find_assert_locations_1): Same as above.
> > (find_conditional_asserts): Same as above.
> > (process_assert_insertions): Same as above.
> > (register_new_assert_for): Same as above.
> > (vrp_prop): New variable fun.
> > (vrp_initialize): New parameter.
> > (identify_jump_threads): Same as above.
> > (execute_vrp): Same as above.
> Just a note. While the old VRP implementation in tree-vrp.c is on the
> chopping
> block, but it'll likely be the end of summer before we know if further work in
> the new Ranger based implementation will be needed to totally replace tree-vrp
> w/o introducing any performance regressions.
>
> Thus, IMHO, we should go forward with the review.
>
> Jeff
>
>
>
commit f119b4e6319031e72d02f6b1c7088cb3b7710e2d
Author: Aldy Hernandez
Date: Sat May 16 20:56:19 2020 +0200
More refactoring of tree-vrp.c.
New class live_names to maintain the set of SSA names live.
Fix whitespace in vrp_insert.
Move a few more methods related to ASSERT_EXPR insertion into vrp_insert.
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 64681dd97fe..1e0330d9d55 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,18 @@
+2020-05-17 Aldy Hernandez
+
+ * tree-vrp.c (class live_names): New.
+ (live_on_edge): Move into live_names.
+ (build_assert_expr_for): Move into vrp_insert.
+ (find_assert_locations_in_bb): Rename from
+ find_assert_locations_1.
+ (process_assert_insertions_for): Move into vrp_insert.
+ (compare_assert_loc): Same.
+ (remove_range_assertions): Same.
+ (dump_asserts_for): Rename to vrp_insert::dump.
+ (debug_asserts_for): Rename to vrp_insert::debug.
+ (dump_all_asserts): Rename to vrp_insert::dump.
+ (debug_all_asserts): Rename to vrp_insert::debug.
+
2020-05-17 Aldy Hernandez
* tree-vrp.c (class vrp_prop): Move check_all_array_refs,
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index 6e3510856a5..e4bc774dd50 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -68,6 +68,99 @@ along with GCC; see the file COPYING3. If not see
#include "builtins.h"
#include "range-op.h"
+/* Set of SSA names found live during the RPO traversal of the function
+ for still active basic-blocks. */
+class live_names
+{
+public:
+ live_names ();
+ ~live_names ();
+ void set (tree, basic_block);
+ void clear (tree, basic_block);
+ void merge (basic_block dest, basic_block src);
+ bool live_on_block_p (tree, basic_block);
+ bool live_on_edge_p (tree, edge);
+ bool block_has_live_names_p (basic_block);
+ void clear_block (basic_block);
+
+private:
+ sbitmap *live;
+ unsigned num_blocks;
+ void init_bitmap_if_needed (basic_block);
+};
+
+void
+live_names::init_bitmap_if_needed (basic_block bb)
+{
+ unsigned i = bb->index;
+ if (!live[i])
+{
+ live[i] = sbitmap_alloc (num_ssa_names);
+ bitmap_clear (live[i]);
+}
+}
+
+bool
+live_names::block_has_live_names_p (basic_block bb)
+{
+ unsigned i = bb->index;
+ return live[i] && bitmap_empty_p (live[i]);
+}
+
+void
+live_names::clear_block (basic_block bb)
+{
+ unsigned i = bb->index;
+ if (live[i])
+{
+ sbitmap_free (live[i]);
+ live[i] = NULL;
+}
+}
+
+void
+live_names::merge (basic_block dest, basic_block src)
+{
+ init_bitmap_if_needed (dest);
+ init_bitmap_if_needed (src);
+ bitmap_ior (live[dest->index], live[dest->index], live[src->index]);
+}
+
+void
+live_names::set (tree name, basic_block bb)
Re: [PATCH] Refactor tree-vrp.c
Missed this on the previous patch. The method live_on_edge() no longer needs to be inside the vrp_insert class. I've removed the prototype. Committed to mainline. On Sun, May 17, 2020 at 1:35 PM Aldy Hernandez wrote: > > Funny, I had queued up more or less the same changes on my tree, but > Giuliano beat me to it. > > I am committing the attached patch that has been approved by Jeff. > > I moved a couple more functions into the vrp_insert class, as well as > abstract out the liveness bitmap into its own class. I also renamed > the various dump/debug routines into overloaded methods. > > Finally, I noticed the indentation for the class was off, and fixed it. > > Committed to mainline. > Aldy > > On Fri, May 8, 2020 at 6:21 PM Jeff Law via Gcc-patches > wrote: > > > > On Fri, 2020-05-08 at 13:06 -0300, Giuliano Belinassi via Gcc-patches wrote: > > > Hi, > > > > > > This patch Refactors tree-vrp.c to eliminate all global variables except > > > 'x_vrp_values', which will require that 'thread_outgoing_edges' to > > > accept an extra argument and pass it to the 'simplify' callback. > > > > > > It also removes every access to 'cfun', retrieving the function being > > > compiled from the pass engine. > > > > > > Bootstrapped and ran the testsuite on Linux x86_64. > > > > > > gcc/ChangeLog > > > 2020-05-08 Giuliano Belinassi > > > > > > * tree-vrp.c (class liveness): New. > > > (insert_range_assertions): Move to class liveness. > > > (dump_all_asserts): Same as above. > > > (dump_asserts_for): Same as above. > > > (live): Same as above. > > > (need_assert_for): Same as above. > > > (live_on_edge): Same as above. > > > (finish_register_edge_assert_for): Same as above. > > > (find_switch_asserts): Same as above. > > > (find_assert_locations): Same as above. > > > (find_assert_locations_1): Same as above. > > > (find_conditional_asserts): Same as above. > > > (process_assert_insertions): Same as above. > > > (register_new_assert_for): Same as above. > > > (vrp_prop): New variable fun. > > > (vrp_initialize): New parameter. > > > (identify_jump_threads): Same as above. > > > (execute_vrp): Same as above. > > Just a note. While the old VRP implementation in tree-vrp.c is on the > > chopping > > block, but it'll likely be the end of summer before we know if further work > > in > > the new Ranger based implementation will be needed to totally replace > > tree-vrp > > w/o introducing any performance regressions. > > > > Thus, IMHO, we should go forward with the review. > > > > Jeff > > > > > > commit 92b16b5ce7e5ab74b1807c7be96a15663fa17ddd Author: Aldy Hernandez Date: Sun May 17 13:40:09 2020 +0200 * tree-vrp.c (class vrp_insert): Remove prototype for live_on_edge. diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 1e0330d9d55..fecc494f380 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,8 @@ +2020-05-17 Aldy Hernandez + + * tree-vrp.c (class vrp_insert): Remove prototype for + live_on_edge. + 2020-05-17 Aldy Hernandez * tree-vrp.c (class live_names): New. diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c index e4bc774dd50..4b5df543c00 100644 --- a/gcc/tree-vrp.c +++ b/gcc/tree-vrp.c @@ -244,9 +244,6 @@ private: ASSERT_EXPRs for SSA name N_I should be inserted. */ assert_locus **asserts_for; - /* Return true if the SSA name NAME is live on the edge E. */ - bool live_on_edge (edge e, tree name); - /* Finish found ASSERTS for E and register them at GSI. */ void finish_register_edge_assert_for (edge e, gimple_stmt_iterator gsi, vec &asserts);
[committed] move operand_less_p to vr-values.c
There's no reason to export operand_less_p from tree-vrp when its only
use is in vr-values.c. Function moved.
Committed as obvious.
Aldy
commit 8bfc81876f9325891a52d036a9c454d0c81b3033
Author: Aldy Hernandez
Date: Fri May 8 13:36:32 2020 +0200
Move operand_less_p to vr-values.c.
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index fecc494f380..5c90953edb1 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@
+2020-05-17 Aldy Hernandez
+
+ * tree-vrp.c (operand_less_p): Move to...
+ * vr-values.c (operand_less_p): ...here.
+ * tree-vrp.h (operand_less_p): Remove.
+
2020-05-17 Aldy Hernandez
* tree-vrp.c (class vrp_insert): Remove prototype for
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index 4b5df543c00..f8191facaf9 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -685,32 +685,6 @@ build_symbolic_expr (tree type, tree sym, bool neg, tree inv)
return build2 (pointer_p ? POINTER_PLUS_EXPR : PLUS_EXPR, type, t, inv);
}
-/* Return
- 1 if VAL < VAL2
- 0 if !(VAL < VAL2)
- -2 if those are incomparable. */
-int
-operand_less_p (tree val, tree val2)
-{
- /* LT is folded faster than GE and others. Inline the common case. */
- if (TREE_CODE (val) == INTEGER_CST && TREE_CODE (val2) == INTEGER_CST)
-return tree_int_cst_lt (val, val2);
- else if (TREE_CODE (val) == SSA_NAME && TREE_CODE (val2) == SSA_NAME)
-return val == val2 ? 0 : -2;
- else
-{
- int cmp = compare_values (val, val2);
- if (cmp == -1)
- return 1;
- else if (cmp == 0 || cmp == 1)
- return 0;
- else
- return -2;
-}
-
- return 0;
-}
-
/* Compare two values VAL1 and VAL2. Return
-2 if VAL1 and VAL2 cannot be compared at compile-time,
diff --git a/gcc/tree-vrp.h b/gcc/tree-vrp.h
index aa8201f7359..b74fe0059fc 100644
--- a/gcc/tree-vrp.h
+++ b/gcc/tree-vrp.h
@@ -116,7 +116,6 @@ extern bool range_int_cst_p (const value_range *);
extern int compare_values (tree, tree);
extern int compare_values_warnv (tree, tree, bool *);
-extern int operand_less_p (tree, tree);
void range_fold_unary_expr (value_range *, enum tree_code, tree type,
const value_range *, tree op0_type);
diff --git a/gcc/vr-values.c b/gcc/vr-values.c
index 2e3a0788710..f7cba16aa63 100644
--- a/gcc/vr-values.c
+++ b/gcc/vr-values.c
@@ -1104,6 +1104,32 @@ vr_values::check_for_binary_op_overflow (enum tree_code subcode, tree type,
return true;
}
+/* Return
+ 1 if VAL < VAL2
+ 0 if !(VAL < VAL2)
+ -2 if those are incomparable. */
+static int
+operand_less_p (tree val, tree val2)
+{
+ /* LT is folded faster than GE and others. Inline the common case. */
+ if (TREE_CODE (val) == INTEGER_CST && TREE_CODE (val2) == INTEGER_CST)
+return tree_int_cst_lt (val, val2);
+ else if (TREE_CODE (val) == SSA_NAME && TREE_CODE (val2) == SSA_NAME)
+return val == val2 ? 0 : -2;
+ else
+{
+ int cmp = compare_values (val, val2);
+ if (cmp == -1)
+ return 1;
+ else if (cmp == 0 || cmp == 1)
+ return 0;
+ else
+ return -2;
+}
+
+ return 0;
+}
+
/* Try to derive a nonnegative or nonzero range out of STMT relying
primarily on generic routines in fold in conjunction with range data.
Store the result in *VR */
Re: [committed] move operand_less_p to vr-values.c
Whoops, I seem to have a discrepancy in my local tree and upstream. I've reverted this patch. Aldy On 5/17/20 1:50 PM, Aldy Hernandez wrote: There's no reason to export operand_less_p from tree-vrp when its only use is in vr-values.c. Function moved. Committed as obvious. Aldy
New Swedish PO file for 'gcc' (version 10.1.0)
Hello, gentle maintainer. This is a message from the Translation Project robot. A revised PO file for textual domain 'gcc' has been submitted by the Swedish team of translators. The file is available at: https://translationproject.org/latest/gcc/sv.po (This file, 'gcc-10.1.0.sv.po', has just now been sent to you in a separate email.) All other PO files for your package are available in: https://translationproject.org/latest/gcc/ Please consider including all of these in your next release, whether official or a pretest. Whenever you have a new distribution with a new version number ready, containing a newer POT file, please send the URL of that distribution tarball to the address below. The tarball may be just a pretest or a snapshot, it does not even have to compile. It is just used by the translators when they need some extra translation context. The following HTML page has been updated: https://translationproject.org/domain/gcc.html If any question arises, please contact the translation coordinator. Thank you for all your work, The Translation Project robot, in the name of your translation coordinator.
[committed] Use instead of for the C++ ABI changes.
Having noticed this in some other case I went through all of our
pages and found this this instance in the GCC 10 release notes where
.
commit f1d2be6c9fcc52d676266e7ede123953d150aaf3
Author: Jonathan Wakely
Date: Thu May 7 11:24:04 2020 +0100
Document C++17 ABI changes in GCC 10
The old form still works in all browsers I know of, but the W3C HTML 5
checker warns about it, so better to adjust.
Pushed.
Gerald
---
htdocs/gcc-10/changes.html | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/htdocs/gcc-10/changes.html b/htdocs/gcc-10/changes.html
index 2a94b13d..af40b3ad 100644
--- a/htdocs/gcc-10/changes.html
+++ b/htdocs/gcc-10/changes.html
@@ -411,7 +411,7 @@ int get_na??ve_pi() {
namespaces too.
-The ABI
+The ABI
of passing and returning certain C++ classes by value changed
on several targets in GCC 10, including
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94383";>AArch64,
--
2.26.2
Re: [PATCH 1/2, i386] cmpstrnsi needs string length
On Thu, Nov 3, 2016 at 12:51 AM Uros Bizjak wrote:
>
> Hello!
>
> > This patch adds a test to the cmpstrnsi pattern in i386.md so that it
> > will bail out (FAIL) if neither of the strings is a constant string. It
> > can only work as a proper strncmp if the length is not longer than both
> > of the strings. This change is required if expand_builtin_strncmp is
> > going to try expansion of strncmp when neither string argument is
> > constant.
>
> The patch needs a ChangeLog entry, but otherwise looks good.
I am checking in this patch to add a testcase for this condition.
--
H.J.
From 78170fb45ffc86a8c7fc18a662e3264045aced1b Mon Sep 17 00:00:00 2001
From: "H.J. Lu"
Date: Sun, 17 May 2020 06:44:59 -0700
Subject: [PATCH] x86: Add gcc.target/i386/strncmp-1.c
Add a strncmp test for the cmpstrn pattern with neither of the strings
is a constant string. We can expand the cmpstrn pattern to "repz cmpsb"
only if one of the strings is a constant so that expand_builtin_strncmp()
can write the length argument to be the minimum of the const string
length and the actual length argument. Otherwise, "repz cmpsb" may pass
the 0 byte.
* gcc.target/i386/strncmp-1.c: New test.
---
gcc/testsuite/gcc.target/i386/strncmp-1.c | 47 +++
1 file changed, 47 insertions(+)
create mode 100644 gcc/testsuite/gcc.target/i386/strncmp-1.c
diff --git a/gcc/testsuite/gcc.target/i386/strncmp-1.c b/gcc/testsuite/gcc.target/i386/strncmp-1.c
new file mode 100644
index 000..044fc5cc5fa
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/strncmp-1.c
@@ -0,0 +1,47 @@
+/* { dg-do run { target mmap } } */
+/* { dg-options "-O2" } */
+
+#include
+#include
+#include
+#include
+#include
+
+#ifndef MAP_ANONYMOUS
+#define MAP_ANONYMOUS MAP_ANON
+#endif
+
+int
+__attribute__ ((noclone, noinline))
+compare (char *d, char *s, unsigned int l)
+{
+ return __builtin_strncmp (d, s, l);
+}
+
+int
+main ()
+{
+ size_t page_size = sysconf(_SC_PAGESIZE);
+ char *buf = mmap (0, 2 * page_size, PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+ if (buf == MAP_FAILED)
+{
+ perror ("mmap");
+ abort ();
+}
+
+ if (mprotect (buf + page_size, page_size, PROT_NONE))
+{
+ perror ("mprotect");
+ abort ();
+}
+
+ char *src1 = buf + page_size - sizeof ("foo");
+ char *src2 = buf;
+ memcpy (src1, "foo", sizeof ("foo"));
+ memcpy (src2, "foo", sizeof ("foo"));
+ int result = compare (src1, src2, sizeof ("foo") + 16);
+ if (result != 0)
+abort ();
+ return 0;
+}
--
2.26.2
[committed][GCC10] d: Fix ICE in verify_gimple_stmt, at tree-cfg.c:4959
Hi,
This patch has been backported from mainline r11-154.
The ICE is also present on the gcc-9 branch, and this applies cleanly
there as well, so will send out another email for that backport.
Bootstrapped and regression tested on x86_64-linux-gnu, and committed to
the gcc-10 branch.
Regards
Iain
---
gcc/d/ChangeLog:
PR d/94970
* d-codegen.cc (force_target_expr): Move create_temporary_var
implementation inline here.
(create_temporary_var): Remove.
(maybe_temporary_var): Remove.
(bind_expr): Remove.
* d-convert.cc (d_array_convert): Use build_local_temp to generate
temporaries, and generate its assignment.
* d-tree.h (create_temporary_var): Remove.
(maybe_temporary_var): Remove.
(d_array_convert): Remove vars argument.
* expr.cc (ExprVisitor::visit (CatExp *)): Use build_local_temp to
generate temporaries, don't wrap them in a BIND_EXPR.
(ExprVisitor::visit (NewExp *)): Likewise.
gcc/testsuite/ChangeLog:
PR d/94970
* gdc.dg/pr94970.d: New test.
---
gcc/d/ChangeLog| 20 ++
gcc/d/d-codegen.cc | 67 +++---
gcc/d/d-convert.cc | 14 ---
gcc/d/d-tree.h | 4 +-
gcc/d/expr.cc | 33 +++--
gcc/testsuite/ChangeLog| 8
gcc/testsuite/gdc.dg/pr94970.d | 20 ++
7 files changed, 77 insertions(+), 89 deletions(-)
create mode 100644 gcc/testsuite/gdc.dg/pr94970.d
diff --git a/gcc/d/ChangeLog b/gcc/d/ChangeLog
index 1a8bedfef2d..9cf976d97e1 100644
--- a/gcc/d/ChangeLog
+++ b/gcc/d/ChangeLog
@@ -1,3 +1,23 @@
+2020-05-17 Iain Buclaw
+
+ Backport from mainline
+ 2020-05-06 Iain Buclaw
+
+ PR d/94970
+ * d-codegen.cc (force_target_expr): Move create_temporary_var
+ implementation inline here.
+ (create_temporary_var): Remove.
+ (maybe_temporary_var): Remove.
+ (bind_expr): Remove.
+ * d-convert.cc (d_array_convert): Use build_local_temp to generate
+ temporaries, and generate its assignment.
+ * d-tree.h (create_temporary_var): Remove.
+ (maybe_temporary_var): Remove.
+ (d_array_convert): Remove vars argument.
+ * expr.cc (ExprVisitor::visit (CatExp *)): Use build_local_temp to
+ generate temporaries, don't wrap them in a BIND_EXPR.
+ (ExprVisitor::visit (NewExp *)): Likewise.
+
2020-05-07 Release Manager
* GCC 10.1.0 released.
diff --git a/gcc/d/d-codegen.cc b/gcc/d/d-codegen.cc
index b4927a2de10..5efd4b9c43c 100644
--- a/gcc/d/d-codegen.cc
+++ b/gcc/d/d-codegen.cc
@@ -619,7 +619,12 @@ build_target_expr (tree decl, tree exp)
tree
force_target_expr (tree exp)
{
- tree decl = create_temporary_var (TREE_TYPE (exp));
+ tree decl = build_decl (input_location, VAR_DECL, NULL_TREE,
+ TREE_TYPE (exp));
+ DECL_CONTEXT (decl) = current_function_decl;
+ DECL_ARTIFICIAL (decl) = 1;
+ DECL_IGNORED_P (decl) = 1;
+ layout_decl (decl, 0);
return build_target_expr (decl, exp);
}
@@ -1766,66 +1771,6 @@ array_bounds_check (void)
}
}
-/* Return an undeclared local temporary of type TYPE
- for use with BIND_EXPR. */
-
-tree
-create_temporary_var (tree type)
-{
- tree decl = build_decl (input_location, VAR_DECL, NULL_TREE, type);
-
- DECL_CONTEXT (decl) = current_function_decl;
- DECL_ARTIFICIAL (decl) = 1;
- DECL_IGNORED_P (decl) = 1;
- layout_decl (decl, 0);
-
- return decl;
-}
-
-/* Return an undeclared local temporary OUT_VAR initialized
- with result of expression EXP. */
-
-tree
-maybe_temporary_var (tree exp, tree *out_var)
-{
- tree t = exp;
-
- /* Get the base component. */
- while (TREE_CODE (t) == COMPONENT_REF)
-t = TREE_OPERAND (t, 0);
-
- if (!DECL_P (t) && !REFERENCE_CLASS_P (t))
-{
- *out_var = create_temporary_var (TREE_TYPE (exp));
- DECL_INITIAL (*out_var) = exp;
- return *out_var;
-}
- else
-{
- *out_var = NULL_TREE;
- return exp;
-}
-}
-
-/* Builds a BIND_EXPR around BODY for the variables VAR_CHAIN. */
-
-tree
-bind_expr (tree var_chain, tree body)
-{
- /* Only handles one var. */
- gcc_assert (TREE_CHAIN (var_chain) == NULL_TREE);
-
- if (DECL_INITIAL (var_chain))
-{
- tree ini = build_assign (INIT_EXPR, var_chain, DECL_INITIAL (var_chain));
- DECL_INITIAL (var_chain) = NULL_TREE;
- body = compound_expr (ini, body);
-}
-
- return d_save_expr (build3 (BIND_EXPR, TREE_TYPE (body),
- var_chain, body, NULL_TREE));
-}
-
/* Returns the TypeFunction class for Type T.
Assumes T is already ->toBasetype(). */
diff --git a/gcc/d/d-convert.cc b/gcc/d/d-convert.cc
index e2921ec33f0..f93405ed956 100644
--- a/gcc/d/d-convert.cc
+++ b/gcc/d/d-convert.cc
@@ -774,21 +774,23 @@ d_array_convert (Expression *exp)
/* Convert EXP to a dynamic array, where ETYPE is the element type.
[committed][GCC9] d: Fix ICE in verify_gimple_stmt, at tree-cfg.c:4959
Hi,
This patch is the gcc-9 backport of mainline r11-154 / gcc-10 r10-8149.
Bootstrapped and regression tested on x86_64-linux-gnu, and committed to
the gcc-9 branch.
Regards
Iain
---
gcc/d/ChangeLog:
PR d/94970
* d-codegen.cc (force_target_expr): Move create_temporary_var
implementation inline here.
(create_temporary_var): Remove.
(maybe_temporary_var): Remove.
(bind_expr): Remove.
* d-convert.cc (d_array_convert): Use build_local_temp to generate
temporaries, and generate its assignment.
* d-tree.h (create_temporary_var): Remove.
(maybe_temporary_var): Remove.
(d_array_convert): Remove vars argument.
* expr.cc (ExprVisitor::visit (CatExp *)): Use build_local_temp to
generate temporaries, don't wrap them in a BIND_EXPR.
(ExprVisitor::visit (NewExp *)): Likewise.
gcc/testsuite/ChangeLog:
PR d/94970
* gdc.dg/pr94970.d: New test.
---
gcc/d/ChangeLog| 20 ++
gcc/d/d-codegen.cc | 67 +++---
gcc/d/d-convert.cc | 14 ---
gcc/d/d-tree.h | 4 +-
gcc/d/expr.cc | 33 +++--
gcc/testsuite/ChangeLog| 8
gcc/testsuite/gdc.dg/pr94970.d | 20 ++
7 files changed, 77 insertions(+), 89 deletions(-)
create mode 100644 gcc/testsuite/gdc.dg/pr94970.d
diff --git a/gcc/d/ChangeLog b/gcc/d/ChangeLog
index c3f56590298..5048d2ab400 100644
--- a/gcc/d/ChangeLog
+++ b/gcc/d/ChangeLog
@@ -1,3 +1,23 @@
+2020-05-17 Iain Buclaw
+
+ Backport from mainline
+ 2020-05-06 Iain Buclaw
+
+ PR d/94970
+ * d-codegen.cc (force_target_expr): Move create_temporary_var
+ implementation inline here.
+ (create_temporary_var): Remove.
+ (maybe_temporary_var): Remove.
+ (bind_expr): Remove.
+ * d-convert.cc (d_array_convert): Use build_local_temp to generate
+ temporaries, and generate its assignment.
+ * d-tree.h (create_temporary_var): Remove.
+ (maybe_temporary_var): Remove.
+ (d_array_convert): Remove vars argument.
+ * expr.cc (ExprVisitor::visit (CatExp *)): Use build_local_temp to
+ generate temporaries, don't wrap them in a BIND_EXPR.
+ (ExprVisitor::visit (NewExp *)): Likewise.
+
2020-05-16 Iain Buclaw
PR d/95155
diff --git a/gcc/d/d-codegen.cc b/gcc/d/d-codegen.cc
index 2abff92fc88..17f624b5a4f 100644
--- a/gcc/d/d-codegen.cc
+++ b/gcc/d/d-codegen.cc
@@ -612,7 +612,12 @@ build_target_expr (tree decl, tree exp)
tree
force_target_expr (tree exp)
{
- tree decl = create_temporary_var (TREE_TYPE (exp));
+ tree decl = build_decl (input_location, VAR_DECL, NULL_TREE,
+ TREE_TYPE (exp));
+ DECL_CONTEXT (decl) = current_function_decl;
+ DECL_ARTIFICIAL (decl) = 1;
+ DECL_IGNORED_P (decl) = 1;
+ layout_decl (decl, 0);
return build_target_expr (decl, exp);
}
@@ -1801,66 +1806,6 @@ array_bounds_check (void)
}
}
-/* Return an undeclared local temporary of type TYPE
- for use with BIND_EXPR. */
-
-tree
-create_temporary_var (tree type)
-{
- tree decl = build_decl (input_location, VAR_DECL, NULL_TREE, type);
-
- DECL_CONTEXT (decl) = current_function_decl;
- DECL_ARTIFICIAL (decl) = 1;
- DECL_IGNORED_P (decl) = 1;
- layout_decl (decl, 0);
-
- return decl;
-}
-
-/* Return an undeclared local temporary OUT_VAR initialized
- with result of expression EXP. */
-
-tree
-maybe_temporary_var (tree exp, tree *out_var)
-{
- tree t = exp;
-
- /* Get the base component. */
- while (TREE_CODE (t) == COMPONENT_REF)
-t = TREE_OPERAND (t, 0);
-
- if (!DECL_P (t) && !REFERENCE_CLASS_P (t))
-{
- *out_var = create_temporary_var (TREE_TYPE (exp));
- DECL_INITIAL (*out_var) = exp;
- return *out_var;
-}
- else
-{
- *out_var = NULL_TREE;
- return exp;
-}
-}
-
-/* Builds a BIND_EXPR around BODY for the variables VAR_CHAIN. */
-
-tree
-bind_expr (tree var_chain, tree body)
-{
- /* Only handles one var. */
- gcc_assert (TREE_CHAIN (var_chain) == NULL_TREE);
-
- if (DECL_INITIAL (var_chain))
-{
- tree ini = build_assign (INIT_EXPR, var_chain, DECL_INITIAL (var_chain));
- DECL_INITIAL (var_chain) = NULL_TREE;
- body = compound_expr (ini, body);
-}
-
- return d_save_expr (build3 (BIND_EXPR, TREE_TYPE (body),
- var_chain, body, NULL_TREE));
-}
-
/* Returns the TypeFunction class for Type T.
Assumes T is already ->toBasetype(). */
diff --git a/gcc/d/d-convert.cc b/gcc/d/d-convert.cc
index e9aa457d852..761ab3c5435 100644
--- a/gcc/d/d-convert.cc
+++ b/gcc/d/d-convert.cc
@@ -775,21 +775,23 @@ d_array_convert (Expression *exp)
/* Convert EXP to a dynamic array, where ETYPE is the element type.
Similar to above, except that EXP is allowed to be an element of an array.
- Temporary variables that need some kind of BIND_EX
Re: [PATCH] x86: Allow V1TI vector register pushes
On Sat, May 16, 2020 at 8:13 PM H.J. Lu wrote:
>
> On Fri, May 15, 2020 at 11:21:30AM +0200, Uros Bizjak wrote:
> > On Wed, May 13, 2020 at 5:58 PM H.J. Lu wrote:
> >
> > > > > > The question is, why STV pass creates its funny sequence? The
> > > > > > original
> > > > > > sequence should be easily solved by storing DImode from XMM register
> > > > > > and (with patched gcc) pushing DImode value from the same XMM
> > > > > > register.
> > > > >
> > > > > STV pass is mostly OK since it has to use XMM to move upper and lower
> > > > > 32 bits of a 64-bit integer. The problem is that push XMM becomes
> > > > > very
> > > > > expensive later.
> > > >
> > > > As shown in the patch, XMM push should be just decrement of SP reg and
> > > > move to the created stack slot.
> > >
> > > OK for master if there are no regressions?
> >
> > diff --git a/gcc/config/i386/i386-features.c
> > b/gcc/config/i386/i386-features.c
> > index 78fb373db6e..6cad125fa83 100644
> > --- a/gcc/config/i386/i386-features.c
> > +++ b/gcc/config/i386/i386-features.c
> > @@ -1264,7 +1264,8 @@ has_non_address_hard_reg (rtx_insn *insn)
> >FOR_EACH_INSN_DEF (ref, insn)
> > if (HARD_REGISTER_P (DF_REF_REAL_REG (ref))
> > && !DF_REF_FLAGS_IS_SET (ref, DF_REF_MUST_CLOBBER)
> > -&& DF_REF_REGNO (ref) != FLAGS_REG)
> > +&& DF_REF_REGNO (ref) != FLAGS_REG
> > +&& DF_REF_REGNO (ref) != SP_REG)
> >return true;
> >
> > I don't think this part is correct. The function comment says:
> >
> > /* Return 1 if INSN uses or defines a hard register.
> >Hard register uses in a memory address are ignored.
> >Clobbers and flags definitions are ignored. */
> >
> > and you are putting SP_REG into clobber part.
> >
> > OTOH, SP_REG in:
> >
> > (insn 28 27 29 3 (set (mem:DI (pre_dec:SI (reg/f:SI 7 sp)) [2 S8 A64])
> > (reg/v:DI 85 [ target ])) "x.i":19:5 40 {*pushdi2}
> >
> > is inside memory, so REG_SP should already be ignored. Please
> > investigate, why it is not the case.
>
> Push isn't a simple memory access since it also updates stack pointer.
> Fixed to handle pseudo register push.
>
> >
> > +(define_insn "*pushv1ti2"
> > + [(set (match_operand:V1TI 0 "push_operand" "=<")
> > +(match_operand:V1TI 1 "general_no_elim_operand" "v"))]
> > + ""
> > + "#"
> > + [(set_attr "type" "multi")
> > + (set_attr "mode" "TI")])
> > +
> > +(define_split
> > + [(set (match_operand:V1TI 0 "push_operand" "")
> > +(match_operand:V1TI 1 "sse_reg_operand" ""))]
> > + "TARGET_64BIT && reload_completed"
> > + [(set (reg:P SP_REG) (plus:P (reg:P SP_REG) (match_dup 2)))
> > +(set (match_dup 0) (match_dup 1))]
> > +{
> > + operands[2] = GEN_INT (-PUSH_ROUNDING (16));
> > + /* Preserve memory attributes. */
> > + operands[0] = replace_equiv_address (operands[0], stack_pointer_rtx);
> > +})
> >
> > The above part is wrong on many levels, e.g. using wrong predicate,
> > unnecessary rounding, it should be defined as define_insn_and_split,
> > conditionalized on TARGET_64BIT && TARGET_STV and put nearby existing
> > pushti patterns.
>
> Fixed.
>
> >
> > I will implement push changes (modulo V1T1mode) by myself, since they
> > are independent of STV stuff.
> >
>
> Here is the updated patch. Tested on Linux/x86 and Linux/x86-64. OK
> for master?
>
> Thanks.
>
> H.J.
> ---
> Add V1TI vector register push and split it after reload to a sequence
> of:
>
> (set (reg:P SP_REG) (plus:P SP_REG) (const_int -8)))
> (set (match_dup 0) (match_dup 1))
>
> so that STV pass can convert TI mode integer push to V1TI vector register
> push. Rename has_non_address_hard_reg to pseudo_reg_set, combine calls
> of single_set and has_non_address_hard_reg to pseudo_reg_set, to ignore
> pseudo register push.
>
> Remove c-c++-common/dfp/func-vararg-mixed-2.c since it is compiled with
> -mpreferred-stack-boundary=2 and leads to segfault:
>
> Dump of assembler code for function __bid_nesd2:
>0x08049210 <+0>: endbr32
>0x08049214 <+4>: push %esi
>0x08049215 <+5>: push %ebx
>0x08049216 <+6>: call 0x8049130 <__x86.get_pc_thunk.bx>
>0x0804921b <+11>:add$0x8de5,%ebx
>0x08049221 <+17>:sub$0x20,%esp
>0x08049224 <+20>:mov0x30(%esp),%esi
>0x08049228 <+24>:pushl 0x2c(%esp)
>0x0804922c <+28>:call 0x804e600 <__bid32_to_bid64>
>0x08049231 <+33>:mov%esi,(%esp)
>0x08049234 <+36>:movd %edx,%xmm1
>0x08049238 <+40>:movd %eax,%xmm0
>0x0804923c <+44>:punpckldq %xmm1,%xmm0
> => 0x08049240 <+48>:movaps %xmm0,0x10(%esp)
>0x08049245 <+53>:call 0x804e600 <__bid32_to_bid64>
>0x0804924a <+58>:push %edx
>0x0804924b <+59>:push %eax
>0x0804924c <+60>:pushl 0x1c(%esp)
>0x08049250 <+64>:pushl 0x1c(%esp)
>0x08049254 <+68>:call 0x804b260 <__bid64_quiet_not_equal>
>0x08049259 <+73>:add$0x34,%esp
>0x0804925c <+76>:pop%ebx
>0x0804925d <+77>:pop%esi
>
Re: [PATCH] i386: Define __ILP32__ and _ILP32 for all 32-bit targets
On Fri, 8 May 2020, Uros Bizjak wrote: >> A user reported that gcc -m32 on x86-64 does not define __ILP32__ >> and I found the same on i686 (with gcc -x c -dM -E /dev/null). : >> This patch does the same for all "regular" 32-bit x86 targets. >> Tested on i386-unknown-freebsd11.3 so far. > OK. Thank you, Uros. This is in mainline now. Okay to backport to the GCC 10 and ideally GCC 9 branches? (The latter is where the user report came from.) Gerald
Re: [PATCH] i386: Define __ILP32__ and _ILP32 for all 32-bit targets
On Sun, May 17, 2020 at 6:14 PM Gerald Pfeifer wrote: > > On Fri, 8 May 2020, Uros Bizjak wrote: > >> A user reported that gcc -m32 on x86-64 does not define __ILP32__ > >> and I found the same on i686 (with gcc -x c -dM -E /dev/null). > : > >> This patch does the same for all "regular" 32-bit x86 targets. > >> Tested on i386-unknown-freebsd11.3 so far. > > OK. > > Thank you, Uros. > > This is in mainline now. Okay to backport to the GCC 10 and ideally > GCC 9 branches? (The latter is where the user report came from.) This is OK for all release branches. Thanks, Uros.
Re: [PATCH] x86: Allow V1TI vector register pushes
On Sun, May 17, 2020 at 9:07 AM Uros Bizjak wrote:
>
> On Sat, May 16, 2020 at 8:13 PM H.J. Lu wrote:
> >
> > On Fri, May 15, 2020 at 11:21:30AM +0200, Uros Bizjak wrote:
> > > On Wed, May 13, 2020 at 5:58 PM H.J. Lu wrote:
> > >
> > > > > > > The question is, why STV pass creates its funny sequence? The
> > > > > > > original
> > > > > > > sequence should be easily solved by storing DImode from XMM
> > > > > > > register
> > > > > > > and (with patched gcc) pushing DImode value from the same XMM
> > > > > > > register.
> > > > > >
> > > > > > STV pass is mostly OK since it has to use XMM to move upper and
> > > > > > lower
> > > > > > 32 bits of a 64-bit integer. The problem is that push XMM becomes
> > > > > > very
> > > > > > expensive later.
> > > > >
> > > > > As shown in the patch, XMM push should be just decrement of SP reg and
> > > > > move to the created stack slot.
> > > >
> > > > OK for master if there are no regressions?
> > >
> > > diff --git a/gcc/config/i386/i386-features.c
> > > b/gcc/config/i386/i386-features.c
> > > index 78fb373db6e..6cad125fa83 100644
> > > --- a/gcc/config/i386/i386-features.c
> > > +++ b/gcc/config/i386/i386-features.c
> > > @@ -1264,7 +1264,8 @@ has_non_address_hard_reg (rtx_insn *insn)
> > >FOR_EACH_INSN_DEF (ref, insn)
> > > if (HARD_REGISTER_P (DF_REF_REAL_REG (ref))
> > > && !DF_REF_FLAGS_IS_SET (ref, DF_REF_MUST_CLOBBER)
> > > -&& DF_REF_REGNO (ref) != FLAGS_REG)
> > > +&& DF_REF_REGNO (ref) != FLAGS_REG
> > > +&& DF_REF_REGNO (ref) != SP_REG)
> > >return true;
> > >
> > > I don't think this part is correct. The function comment says:
> > >
> > > /* Return 1 if INSN uses or defines a hard register.
> > >Hard register uses in a memory address are ignored.
> > >Clobbers and flags definitions are ignored. */
> > >
> > > and you are putting SP_REG into clobber part.
> > >
> > > OTOH, SP_REG in:
> > >
> > > (insn 28 27 29 3 (set (mem:DI (pre_dec:SI (reg/f:SI 7 sp)) [2 S8 A64])
> > > (reg/v:DI 85 [ target ])) "x.i":19:5 40 {*pushdi2}
> > >
> > > is inside memory, so REG_SP should already be ignored. Please
> > > investigate, why it is not the case.
> >
> > Push isn't a simple memory access since it also updates stack pointer.
> > Fixed to handle pseudo register push.
> >
> > >
> > > +(define_insn "*pushv1ti2"
> > > + [(set (match_operand:V1TI 0 "push_operand" "=<")
> > > +(match_operand:V1TI 1 "general_no_elim_operand" "v"))]
> > > + ""
> > > + "#"
> > > + [(set_attr "type" "multi")
> > > + (set_attr "mode" "TI")])
> > > +
> > > +(define_split
> > > + [(set (match_operand:V1TI 0 "push_operand" "")
> > > +(match_operand:V1TI 1 "sse_reg_operand" ""))]
> > > + "TARGET_64BIT && reload_completed"
> > > + [(set (reg:P SP_REG) (plus:P (reg:P SP_REG) (match_dup 2)))
> > > +(set (match_dup 0) (match_dup 1))]
> > > +{
> > > + operands[2] = GEN_INT (-PUSH_ROUNDING (16));
> > > + /* Preserve memory attributes. */
> > > + operands[0] = replace_equiv_address (operands[0], stack_pointer_rtx);
> > > +})
> > >
> > > The above part is wrong on many levels, e.g. using wrong predicate,
> > > unnecessary rounding, it should be defined as define_insn_and_split,
> > > conditionalized on TARGET_64BIT && TARGET_STV and put nearby existing
> > > pushti patterns.
> >
> > Fixed.
> >
> > >
> > > I will implement push changes (modulo V1T1mode) by myself, since they
> > > are independent of STV stuff.
> > >
> >
> > Here is the updated patch. Tested on Linux/x86 and Linux/x86-64. OK
> > for master?
> >
> > Thanks.
> >
> > H.J.
> > ---
> > Add V1TI vector register push and split it after reload to a sequence
> > of:
> >
> > (set (reg:P SP_REG) (plus:P SP_REG) (const_int -8)))
> > (set (match_dup 0) (match_dup 1))
> >
> > so that STV pass can convert TI mode integer push to V1TI vector register
> > push. Rename has_non_address_hard_reg to pseudo_reg_set, combine calls
> > of single_set and has_non_address_hard_reg to pseudo_reg_set, to ignore
> > pseudo register push.
> >
> > Remove c-c++-common/dfp/func-vararg-mixed-2.c since it is compiled with
> > -mpreferred-stack-boundary=2 and leads to segfault:
> >
> > Dump of assembler code for function __bid_nesd2:
> >0x08049210 <+0>: endbr32
> >0x08049214 <+4>: push %esi
> >0x08049215 <+5>: push %ebx
> >0x08049216 <+6>: call 0x8049130 <__x86.get_pc_thunk.bx>
> >0x0804921b <+11>:add$0x8de5,%ebx
> >0x08049221 <+17>:sub$0x20,%esp
> >0x08049224 <+20>:mov0x30(%esp),%esi
> >0x08049228 <+24>:pushl 0x2c(%esp)
> >0x0804922c <+28>:call 0x804e600 <__bid32_to_bid64>
> >0x08049231 <+33>:mov%esi,(%esp)
> >0x08049234 <+36>:movd %edx,%xmm1
> >0x08049238 <+40>:movd %eax,%xmm0
> >0x0804923c <+44>:punpckldq %xmm1,%xmm0
> > => 0x08049240 <+48>:movaps %xmm0,0x10(%esp)
> >0x08049245 <+53>:call 0x804e600 <__bid32_to_bid64>
> >0x0804924a <+58>:
[PATCH] libphobos: Merge upstream druntime 5cc061a8, phobos 64ed4684f
Hi,
This patch merges the D runtime library with upstream druntime 5cc061a8,
and the D standard library with upstream phobos 64ed4684f.
Fixes included within are:
- core.cpuid has been fixed to not use i7 detection on AMD processors
(fixes PR95166).
- std.net.curl has been fixed to correctly handle HTTP/2 status lines
(fixes PR95168).
- std.zip has had a test fixed to not rely on unzip being installed
(fixes PR95167).
Bootstrapped and regression tested on x86_64-linux-gnu, and committed to
mainline.
Regards,
Iain
---
libphobos/libdruntime/MERGE| 2 +-
libphobos/libdruntime/core/cpuid.d | 18 ++--
libphobos/src/MERGE| 2 +-
libphobos/src/std/net/curl.d | 44 ++
libphobos/src/std/zip.d| 6
5 files changed, 56 insertions(+), 16 deletions(-)
diff --git a/libphobos/libdruntime/MERGE b/libphobos/libdruntime/MERGE
index c61ad7ca7ed..5e3bf3b795a 100644
--- a/libphobos/libdruntime/MERGE
+++ b/libphobos/libdruntime/MERGE
@@ -1,4 +1,4 @@
-7bdd83d7b4bd9fd4cb9ffca0d50babc90b31bfd6
+5cc061a8733731d5b40334c0eb7a927b6d6241ce
The first line of this file holds the git revision number of the last
merge done from the dlang/druntime repository.
diff --git a/libphobos/libdruntime/core/cpuid.d
b/libphobos/libdruntime/core/cpuid.d
index 839605aee1e..2ba13b55bf1 100644
--- a/libphobos/libdruntime/core/cpuid.d
+++ b/libphobos/libdruntime/core/cpuid.d
@@ -941,13 +941,27 @@ void cpuidX86()
datacache[0].lineSize = 32;
}
}
-if (max_cpuid >= 0x0B) {
+if (cf.probablyIntel && max_cpuid >= 0x0B) {
// For Intel i7 and later, use function 0x0B to determine
// cores and hyperthreads.
getCpuInfo0B();
} else {
if (hyperThreadingBit) cf.maxThreads = (apic>>>16) & 0xFF;
else cf.maxThreads = cf.maxCores;
+
+if (cf.probablyAMD && max_extended_cpuid >= 0x8000_001E) {
+version (GNU) asm pure nothrow @nogc {
+"cpuid" : "=a" (a), "=b" (b) : "a" (0x8000_001E) : "ecx",
"edx";
+} else {
+asm pure nothrow @nogc {
+mov EAX, 0x8000_001e;
+cpuid;
+mov b, EBX;
+}
+}
+ubyte coresPerComputeUnit = ((b >> 8) & 3) + 1;
+cf.maxCores = cf.maxThreads / coresPerComputeUnit;
+}
}
}
@@ -975,7 +989,7 @@ bool hasCPUID()
xor {(%%esp), %%eax|eax, [esp]}
# eax = whichever bits were changed
popf{l|d} # Restore original EFLAGS
-" : "=a" flags;
+" : "=a" (flags);
}
}
else version (D_InlineAsm_X86)
diff --git a/libphobos/src/MERGE b/libphobos/src/MERGE
index 6025cdcc1f7..5900ca7c6a4 100644
--- a/libphobos/src/MERGE
+++ b/libphobos/src/MERGE
@@ -1,4 +1,4 @@
-bf0d0a37c4c2d8762ceff7d8677e7584b770800f
+64ed4684fa2a0f2401f5b6df34f6dcb4c3973945
The first line of this file holds the git revision number of the last
merge done from the dlang/phobos repository.
diff --git a/libphobos/src/std/net/curl.d b/libphobos/src/std/net/curl.d
index 32ba45ce2de..445f996ea08 100644
--- a/libphobos/src/std/net/curl.d
+++ b/libphobos/src/std/net/curl.d
@@ -2451,7 +2451,6 @@ struct HTTP
in char[] value) callback)
{
import std.algorithm.searching : startsWith;
-import std.conv : to;
import std.regex : regex, match;
import std.uni : toLower;
@@ -2471,18 +2470,8 @@ struct HTTP
if (header.startsWith("HTTP/"))
{
headersIn.clear();
-
-const m = match(header, regex(r"^HTTP/(\d+)\.(\d+)
(\d+) (.*)$"));
-if (m.empty)
+if (parseStatusLine(header, status))
{
-// Invalid status line
-}
-else
-{
-status.majorVersion = to!ushort(m.captures[1]);
-status.minorVersion = to!ushort(m.captures[2]);
-status.code = to!ushort(m.captures[3]);
-status.reason = m.captures[4].idup;
if (onReceiveStatusLine != null)
onReceiveStatusLine(status);
}
@@ -2517,6 +2506,37 @@ struct HTTP
private RefCounted!Impl p;
+/// Parse status line, as received from / generated by cURL.
+private static bool parseStatusLine(in char[] header, out StatusLine
status) @safe
+{
+import std.conv : to;
+import std.regex : regex, match;
+
+const m = match(header, regex(r"^HTTP/(\d+)(?:\.(\d+))? (\d+)(?:
(
[PATCH] x86: Add cmpmemsi for -mgeneral-regs-only
Duplicate the cmpstrn pattern for cmpmem. The only difference is that
the length argument of cmpmem is guaranteed to be less than or equal to
lengths of 2 memory areas. Since "repz cmpsb" can be much slower than
memcmp function implemented with vector instruction, see
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=43052
expand cmpmem to "repz cmpsb" only with -mgeneral-regs-only.
Tested on Linux/x86 and Linux/x86-64. OK for master?
Thanks.
H.J.
---
gcc/
PR target/95151
* config/i386/i386-expand.c (ix86_expand_cmpstrn_or_cmpmem): New
function. Duplicated from the cmpstrn pattern. Expand memcmp
to "repz cmpsb" only with -mgeneral-regs-only.
* config/i386/i386-protos.h (ix86_expand_cmpstrn_or_cmpmem): New
prototype.
* config/i386/i386.md (cmpmemsi): New pattern.
(cmpstrnsi): Use ix86_expand_cmpstrn_or_cmpmem.
gcc/testsuite/
PR target/95151
* gcc.target/i386/pr95151-1.c: New test.
* gcc.target/i386/pr95151-2.c: Likewise.
* gcc.target/i386/pr95151-3.c: Likewise.
* gcc.target/i386/pr95151-4.c: Likewise.
---
gcc/config/i386/i386-expand.c | 80 +++
gcc/config/i386/i386-protos.h | 1 +
gcc/config/i386/i386.md | 80 ++-
gcc/testsuite/gcc.target/i386/pr95151-1.c | 18 +
gcc/testsuite/gcc.target/i386/pr95151-2.c | 11
gcc/testsuite/gcc.target/i386/pr95151-3.c | 18 +
gcc/testsuite/gcc.target/i386/pr95151-4.c | 11
7 files changed, 160 insertions(+), 59 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/i386/pr95151-1.c
create mode 100644 gcc/testsuite/gcc.target/i386/pr95151-2.c
create mode 100644 gcc/testsuite/gcc.target/i386/pr95151-3.c
create mode 100644 gcc/testsuite/gcc.target/i386/pr95151-4.c
diff --git a/gcc/config/i386/i386-expand.c b/gcc/config/i386/i386-expand.c
index 26531585c5f..05d3c33720c 100644
--- a/gcc/config/i386/i386-expand.c
+++ b/gcc/config/i386/i386-expand.c
@@ -7651,6 +7651,86 @@ ix86_expand_set_or_cpymem (rtx dst, rtx src, rtx
count_exp, rtx val_exp,
return true;
}
+/* Expand cmpstrn or memcmp. */
+
+bool
+ix86_expand_cmpstrn_or_cmpmem (rtx result, rtx src1, rtx src2,
+ rtx length, rtx align, bool is_cmpstrn)
+{
+ if (optimize_insn_for_size_p () && !TARGET_INLINE_ALL_STRINGOPS)
+return false;
+
+ /* Can't use this if the user has appropriated ecx, esi or edi. */
+ if (fixed_regs[CX_REG] || fixed_regs[SI_REG] || fixed_regs[DI_REG])
+return false;
+
+ if (is_cmpstrn)
+{
+ /* For strncmp, length is the maximum length, which can be larger
+than actual string lengths. We can expand the cmpstrn pattern
+to "repz cmpsb" only if one of the strings is a constant so
+that expand_builtin_strncmp() can write the length argument to
+be the minimum of the const string length and the actual length
+argument. Otherwise, "repz cmpsb" may pass the 0 byte. */
+ tree t1 = MEM_EXPR (src1);
+ tree t2 = MEM_EXPR (src2);
+ if (!((t1 && TREE_CODE (t1) == MEM_REF
+&& TREE_CODE (TREE_OPERAND (t1, 0)) == ADDR_EXPR
+&& (TREE_CODE (TREE_OPERAND (TREE_OPERAND (t1, 0), 0))
+== STRING_CST))
+ || (t2 && TREE_CODE (t2) == MEM_REF
+ && TREE_CODE (TREE_OPERAND (t2, 0)) == ADDR_EXPR
+ && (TREE_CODE (TREE_OPERAND (TREE_OPERAND (t2, 0), 0))
+ == STRING_CST
+ return false;
+}
+ else
+{
+ /* Expand memcmp to "repz cmpsb" only for -mgeneral-regs-only
+since "repz cmpsb" can be much slower than memcmp function
+implemented with vector instructions, see
+
+https://gcc.gnu.org/bugzilla/show_bug.cgi?id=43052
+ */
+ if (!TARGET_GENERAL_REGS_ONLY)
+ return false;
+}
+
+ rtx addr1 = copy_addr_to_reg (XEXP (src1, 0));
+ rtx addr2 = copy_addr_to_reg (XEXP (src2, 0));
+ if (addr1 != XEXP (src1, 0))
+src1 = replace_equiv_address_nv (src1, addr1);
+ if (addr2 != XEXP (src2, 0))
+src2 = replace_equiv_address_nv (src2, addr2);
+
+ rtx lengthreg = ix86_zero_extend_to_Pmode (length);
+
+ /* If we are testing strict equality, we can use known alignment to
+ good advantage. This may be possible with combine, particularly
+ once cc0 is dead. */
+ if (CONST_INT_P (length))
+{
+ if (length == const0_rtx)
+ {
+ emit_move_insn (result, const0_rtx);
+ return true;
+ }
+ emit_insn (gen_cmpstrnqi_nz_1 (addr1, addr2, lengthreg, align,
+src1, src2));
+}
+ else
+{
+ emit_insn (gen_cmp_1 (Pmode, lengthreg, lengthreg));
+ emit_insn (gen_cmpstrnqi_1 (addr1, addr2, lengthreg, align,
+ src1, src2));
+}
+
+ rtx out = gen_lowpart (QImode, result);
+ emit_insn (gen_cmpintqi (out));
+ emit_move_insn (result, gen_rtx
[committed][GCC10] libphobos: Backport library fixes from mainline
Hi,
This patch backports r11-445 to the gcc-10 release branch.
Bootstrapped and regression tested on x86_64-linux-gnu, and committed.
Regards,
Iain.
---
libphobos/ChangeLog:
PR d/95166
* libdruntime/core/cpuid.d (cpuidX86): Do not use i7 detection on AMD
processors.
(hasCPUID): Fix deprecated asm syntax.
PR d/95167
* src/std/zip.d (unittest): Skip test if unzip is not installed.
PR d/95168
* src/std/net/curl.d (HTTP.onReceiveHeader): Move status line parsing
to ...
(HTTP.parseStatusLine): ... here. New function. Add support for
parsing HTTP/2 status lines.
---
libphobos/libdruntime/core/cpuid.d | 18 ++--
libphobos/src/std/net/curl.d | 44 ++
libphobos/src/std/zip.d| 6
3 files changed, 54 insertions(+), 14 deletions(-)
diff --git a/libphobos/libdruntime/core/cpuid.d
b/libphobos/libdruntime/core/cpuid.d
index 839605aee1e..2ba13b55bf1 100644
--- a/libphobos/libdruntime/core/cpuid.d
+++ b/libphobos/libdruntime/core/cpuid.d
@@ -941,13 +941,27 @@ void cpuidX86()
datacache[0].lineSize = 32;
}
}
-if (max_cpuid >= 0x0B) {
+if (cf.probablyIntel && max_cpuid >= 0x0B) {
// For Intel i7 and later, use function 0x0B to determine
// cores and hyperthreads.
getCpuInfo0B();
} else {
if (hyperThreadingBit) cf.maxThreads = (apic>>>16) & 0xFF;
else cf.maxThreads = cf.maxCores;
+
+if (cf.probablyAMD && max_extended_cpuid >= 0x8000_001E) {
+version (GNU) asm pure nothrow @nogc {
+"cpuid" : "=a" (a), "=b" (b) : "a" (0x8000_001E) : "ecx",
"edx";
+} else {
+asm pure nothrow @nogc {
+mov EAX, 0x8000_001e;
+cpuid;
+mov b, EBX;
+}
+}
+ubyte coresPerComputeUnit = ((b >> 8) & 3) + 1;
+cf.maxCores = cf.maxThreads / coresPerComputeUnit;
+}
}
}
@@ -975,7 +989,7 @@ bool hasCPUID()
xor {(%%esp), %%eax|eax, [esp]}
# eax = whichever bits were changed
popf{l|d} # Restore original EFLAGS
-" : "=a" flags;
+" : "=a" (flags);
}
}
else version (D_InlineAsm_X86)
diff --git a/libphobos/src/std/net/curl.d b/libphobos/src/std/net/curl.d
index 32ba45ce2de..445f996ea08 100644
--- a/libphobos/src/std/net/curl.d
+++ b/libphobos/src/std/net/curl.d
@@ -2451,7 +2451,6 @@ struct HTTP
in char[] value) callback)
{
import std.algorithm.searching : startsWith;
-import std.conv : to;
import std.regex : regex, match;
import std.uni : toLower;
@@ -2471,18 +2470,8 @@ struct HTTP
if (header.startsWith("HTTP/"))
{
headersIn.clear();
-
-const m = match(header, regex(r"^HTTP/(\d+)\.(\d+)
(\d+) (.*)$"));
-if (m.empty)
+if (parseStatusLine(header, status))
{
-// Invalid status line
-}
-else
-{
-status.majorVersion = to!ushort(m.captures[1]);
-status.minorVersion = to!ushort(m.captures[2]);
-status.code = to!ushort(m.captures[3]);
-status.reason = m.captures[4].idup;
if (onReceiveStatusLine != null)
onReceiveStatusLine(status);
}
@@ -2517,6 +2506,37 @@ struct HTTP
private RefCounted!Impl p;
+/// Parse status line, as received from / generated by cURL.
+private static bool parseStatusLine(in char[] header, out StatusLine
status) @safe
+{
+import std.conv : to;
+import std.regex : regex, match;
+
+const m = match(header, regex(r"^HTTP/(\d+)(?:\.(\d+))? (\d+)(?:
(.*))?$"));
+if (m.empty)
+return false; // Invalid status line
+else
+{
+status.majorVersion = to!ushort(m.captures[1]);
+status.minorVersion = m.captures[2].length ?
to!ushort(m.captures[2]) : 0;
+status.code = to!ushort(m.captures[3]);
+status.reason = m.captures[4].idup;
+return true;
+}
+}
+
+@safe unittest
+{
+StatusLine status;
+assert(parseStatusLine("HTTP/1.1 200 OK", status)
+&& status == StatusLine(1, 1, 200, "OK"));
+assert(parseStatusLine("HTTP/1.0 304 Not Modified", status)
+&& status == StatusLine(1, 0, 304, "Not Modified"));
+// The
[committed] Another H8 cleanup in preparation for the cc0->CC_REG transition.
This patch collapses a couple existing patterns (branch_true, branch_false) into
a single pattern and updates all the combiner/peephole branches to use the same
basic structure. This makes the cc0->CC_REG transition easier in a variety of
ways, particularly with optimizing compare/branch sequences.
Specifically it introduces a new pc_or_label_operand instead of explicitly
referencing (pc) and (label_ref) expressions. In patterns it verifies that one
of the two operands must be the pc in the pattern's condition. This isn't
necessary for the peepholes.
In theory this could affect code generation allowing the peepholes to apply in
cases where they don't right now. But in practice I haven't seen any
differences.
Installing on the trunk.
Jeff
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 60ff1ffceef..5cb197958ba 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,11 @@
+2020-05-17 Jeff Law
+
+ * config/h8300/predicates.md (pc_or_label_operand): New predicate.
+ * config/h8300/jumpcall.md (branch_true, branch_false): Consolidate
+ into a single pattern using pc_or_label_operand.
+ * config/h8300/combiner.md (bit branch patterns): Likewise.
+ * config/h8300/peepholes.md (HImode and SImode branches): Likewise.
+
2020-05-17 Aldy Hernandez
Revert:
diff --git a/gcc/config/h8300/combiner.md b/gcc/config/h8300/combiner.md
index f59ff5f15bd..4f49c7fba34 100644
--- a/gcc/config/h8300/combiner.md
+++ b/gcc/config/h8300/combiner.md
@@ -740,9 +740,9 @@
(const_int 1)
(const_int 7))
(const_int 0))
- (label_ref (match_operand 1 "" ""))
- (pc)))]
- ""
+ (match_operand 1 "pc_or_label_operand" "")
+ (match_operand 2 "pc_or_label_operand" "")))]
+ "operands[1] == pc_rtx || operands[2] == pc_rtx"
"#"
""
[(set (cc0) (compare (match_dup 0)
@@ -750,9 +750,8 @@
(set (pc)
(if_then_else (ge (cc0)
(const_int 0))
- (label_ref (match_dup 1))
- (pc)))]
- "")
+ (match_dup 1)
+ (match_dup 2)))])
(define_insn_and_split ""
[(set (pc)
@@ -760,9 +759,9 @@
(const_int 1)
(const_int 7))
(const_int 0))
- (label_ref (match_operand 1 "" ""))
- (pc)))]
- ""
+ (match_operand 1 "pc_or_label_operand" "")
+ (match_operand 2 "pc_or_label_operand" "")))]
+ "operands[1] == pc_rtx || operands[2] == pc_rtx"
"#"
""
[(set (cc0) (compare (match_dup 0)
@@ -770,6 +769,5 @@
(set (pc)
(if_then_else (lt (cc0)
(const_int 0))
- (label_ref (match_dup 1))
- (pc)))]
- "")
+ (match_dup 1)
+ (match_dup 2)))])
diff --git a/gcc/config/h8300/jumpcall.md b/gcc/config/h8300/jumpcall.md
index 4c0a5ccf8af..7208fb6d86b 100644
--- a/gcc/config/h8300/jumpcall.md
+++ b/gcc/config/h8300/jumpcall.md
@@ -37,60 +37,44 @@
DONE;
})
-(define_insn "branch_true"
+(define_insn "branch"
[(set (pc)
- (if_then_else (match_operator 1 "comparison_operator"
+ (if_then_else (match_operator 2 "comparison_operator"
[(cc0) (const_int 0)])
- (label_ref (match_operand 0 "" ""))
- (pc)))]
- ""
+ (match_operand 0 "pc_or_label_operand" "")
+ (match_operand 1 "pc_or_label_operand" "")))]
+ "operands[0] == pc_rtx || operands[1] == pc_rtx"
{
if ((cc_status.flags & CC_OVERFLOW_UNUSABLE) != 0
- && (GET_CODE (operands[1]) == GT
- || GET_CODE (operands[1]) == GE
- || GET_CODE (operands[1]) == LE
- || GET_CODE (operands[1]) == LT))
+ && (GET_CODE (operands[2]) == GT
+ || GET_CODE (operands[2]) == GE
+ || GET_CODE (operands[2]) == LE
+ || GET_CODE (operands[2]) == LT))
{
cc_status.flags &= ~CC_OVERFLOW_UNUSABLE;
return 0;
}
- if (get_attr_length (insn) == 2)
-return "b%j1 %l0";
- else if (get_attr_length (insn) == 4)
-return "b%j1 %l0:16";
- else
-return "b%k1 .Lh8BR%=\;jmp @%l0\\n.Lh8BR%=:";
-}
- [(set_attr "type" "branch")
- (set_attr "cc" "none")])
-
-(define_insn "branch_false"
- [(set (pc)
- (if_then_else (match_operator 1 "comparison_operator"
- [(cc0) (const_int 0)])
- (pc)
- (label_ref (match_operand 0 "" ""]
- ""
-{
- if ((cc_status.flags & CC_OVERFLOW_UNUSABLE) != 0
- && (GET_CODE (operands[1]) == GT
- || GET_CODE (operands[1]) == GE
- || GET_CODE (operands[1]) == LE
- || GET_CODE (operands[1]) == LT))
+ if (operands[0] != pc_rtx)
{
- cc_status.flags &= ~CC_OVERFLOW_UNUSABLE;
- return 0;
+ if (get_attr_length (insn) == 2)
+ return "b%j2 %l0";
+ else if (get_attr_length (insn) == 4)
+ return "b%j2 %l0:16";
+ else
+ return "b%k2 .Lh8BR%=\;jmp @%l0\\n.Lh8BR%=:";
}
-
- if (get_attr_length (insn) == 2)
-return "b%k1 %l0";
- else if (get_attr_length (insn) == 4)
-return "b%k1 %l0:16";
else
-return "b%j1 .Lh8BR%=\;jmp @%l0\\n.Lh8BR%=:";
+{
+ if (get_attr_length (insn) == 2)
+ return "b%k2 %l1";
+ else if (get_attr_length (insn) == 4)
+ return "b%k2 %l1:16";
[committed] coroutines: Avoid a maybe used uninitialized warning. NFC.
Hi, This avoids a (bogus) warning that occurs with some bootstrap compilers. tested on x86-64-darwin, x86-64-linux-gnu, applied to master as obvious, thanks Iain gcc/cp/ChangeLog: 2020-05-17 Iain Sandoe * coroutines.cc (morph_fn_to_coro): Initialize the gro variable. --- gcc/cp/ChangeLog | 4 gcc/cp/coroutines.cc | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog index 7ac074a19b6..65fc6447e74 100644 --- a/gcc/cp/ChangeLog +++ b/gcc/cp/ChangeLog @@ -1,3 +1,7 @@ +2020-05-17 Iain Sandoe + + * coroutines.cc (morph_fn_to_coro): Initialize the gro variable. + 2020-05-16 Iain Sandoe * coroutines.cc (finish_co_return_stmt): Implement rules diff --git a/gcc/cp/coroutines.cc b/gcc/cp/coroutines.cc index facfafaaa86..4cbc0e09994 100644 --- a/gcc/cp/coroutines.cc +++ b/gcc/cp/coroutines.cc @@ -4285,7 +4285,8 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) tree gro_context_body = push_stmt_list (); bool gro_is_void_p = VOID_TYPE_P (TREE_TYPE (get_ro)); - tree gro, gro_bind_vars = NULL_TREE; + tree gro = NULL_TREE; + tree gro_bind_vars = NULL_TREE; /* We have to sequence the call to get_return_object before initial suspend. */ if (gro_is_void_p) -- 2.24.1
Mardi 19 mai à 20h30 : Redémarrez à zéro
Ne plus rien recevoir de notre part
[RFC] analyzer: Add exit, and _exit replacement, to sm-signal.
Hi,
While trying out -fanalyzer on the bzip2 source code I noticed that it
did warn about some unsafe calls in the signal handler, but din't warn
about the exit call:
https://sourceware.org/pipermail/bzip2-devel/2020q2/000107.html
It was easy to add exit to the async_signal_unsafe_fns, but since
there is a signal safe _exit call (which is what you should use in a
signal handler, so no unsafe calls are made, like to the at_exit
handlers) I also wanted to add a fixit hint.
The fixit hint is emitted, but it is somewhat hard to see. Is there a
better way to do this for analyzer warnings so that it is a bit more
prominent?
This is how it currently looks:
/opt/local/gcc/bin/gcc -g -O2 -fanalyzer -c bzip2.c
bzip2.c: In function ‘mySIGSEGVorSIGBUScatcher’:
bzip2.c:874:4: warning: call to ‘exit’ from within signal handler [CWE-479]
[-Wanalyzer-unsafe-call-within-signal-handler]
874 |exit(exitValue);
|^~~
|_exit
‘main’: events 1-2
|
| 1784 | IntNative main ( IntNative argc, Char *argv[] )
| | ^~~~
| | |
| | (1) entry to ‘main’
|..
| 1800 |smallMode = False;
| |~
| ||
| |(2) registering ‘mySIGSEGVorSIGBUScatcher’ as signal handler
|
event 3
|
|cc1:
| (3): later on, when the signal is delivered to the process
|
+--> ‘mySIGSEGVorSIGBUScatcher’: events 4-5
|
| 816 | void mySIGSEGVorSIGBUScatcher ( IntNative n )
| | ^~~~
| | |
| | (4) entry to ‘mySIGSEGVorSIGBUScatcher’
|..
| 874 |exit(exitValue);
| |~~~
| ||
| |(5) call to ‘exit’ from within signal handler
|
Thanks,
Mark
---
gcc/analyzer/sm-signal.cc | 34 +-
1 file changed, 29 insertions(+), 5 deletions(-)
diff --git a/gcc/analyzer/sm-signal.cc b/gcc/analyzer/sm-signal.cc
index 5935e229e77c..99e87706af63 100644
--- a/gcc/analyzer/sm-signal.cc
+++ b/gcc/analyzer/sm-signal.cc
@@ -108,8 +108,9 @@ class signal_unsafe_call
{
public:
signal_unsafe_call (const signal_state_machine &sm, const gcall *unsafe_call,
- tree unsafe_fndecl)
- : m_sm (sm), m_unsafe_call (unsafe_call), m_unsafe_fndecl (unsafe_fndecl)
+ tree unsafe_fndecl, const char *replacement)
+ : m_sm (sm), m_unsafe_call (unsafe_call), m_unsafe_fndecl (unsafe_fndecl),
+m_replacement (replacement)
{
gcc_assert (m_unsafe_fndecl);
}
@@ -126,6 +127,8 @@ public:
diagnostic_metadata m;
/* CWE-479: Signal Handler Use of a Non-reentrant Function. */
m.add_cwe (479);
+if (m_replacement != NULL)
+ rich_loc->add_fixit_replace (m_replacement);
return warning_meta (rich_loc, m,
OPT_Wanalyzer_unsafe_call_within_signal_handler,
"call to %qD from within signal handler",
@@ -156,6 +159,7 @@ private:
const signal_state_machine &m_sm;
const gcall *m_unsafe_call;
tree m_unsafe_fndecl;
+ const char *m_replacement;
};
/* signal_state_machine's ctor. */
@@ -259,6 +263,7 @@ get_async_signal_unsafe_fns ()
// TODO: populate this list more fully
static const char * const async_signal_unsafe_fns[] = {
/* This array must be kept sorted. */
+"exit",
"fprintf",
"free",
"malloc",
@@ -286,6 +291,20 @@ signal_unsafe_p (tree fndecl)
return fs.contains_decl_p (fndecl);
}
+/* Returns a replacement function as text if it exists. Currently
+ only "exit" has a signal-safe replacement "_exit", which does
+ slightly less, but can be used in a signal handler. */
+const char *
+replacement_fn (tree fndecl)
+{
+ gcc_assert (fndecl && DECL_P (fndecl));
+
+ if (strcmp ("exit", IDENTIFIER_POINTER (DECL_NAME (fndecl))) == 0)
+return "_exit";
+
+ return NULL;
+}
+
/* Implementation of state_machine::on_stmt vfunc for signal_state_machine. */
bool
@@ -315,9 +334,14 @@ signal_state_machine::on_stmt (sm_context *sm_ctxt,
if (const gcall *call = dyn_cast (stmt))
if (tree callee_fndecl = sm_ctxt->get_fndecl_for_call (call))
if (signal_unsafe_p (callee_fndecl))
- sm_ctxt->warn_for_state (node, stmt, NULL_TREE, m_in_signal_handler,
-new signal_unsafe_call (*this, call,
-callee_fndecl));
+ {
+ const char *replace = replacement_fn (callee_fndecl);
+ sm_ctxt->warn_for_state (node, stmt, NULL_TREE,
+ m_in_signal_handler,
+ new signal_unsafe_call (*this, call,
+ callee_fndecl,
+
Re: PR fortran/95053 - division by zero constants
Am 15.05.20 um 22:33 schrieb Harald Anlauf: Here's a new attempt to finally fix this PR and any known fallout. In order to handle division by zero in declarations, but still accept the code snippet adapted from 521.wrf_r (from spec2017), I removed the hunk that was added to fix PR94399, and deferred the handling to a later stage. One case regarding array shapes is handled in variable_decl in decl.c, the other one regarding PDTs is handled in gfc_get_pdt_instance in the same file. While at it, I improved the error message to notify the user if an expression that should be of INTEGER type isn't. Regtests with no new failures on x86_64-pc-linux-gnu, after adjusting error messages for test dec_structure_23.f90. OK for trunk? I think this is a good approach (and a better error message, too). So, OK, and thanks for the patch! Regards Thomas
Re: [RFC] analyzer: Add exit, and _exit replacement, to sm-signal.
On Mon, 2020-05-18 at 00:05 +0200, Mark Wielaard wrote:
> Hi,
>
> While trying out -fanalyzer on the bzip2 source code I noticed that
> it
> did warn about some unsafe calls in the signal handler,
Great.
> but din't warn
> about the exit call:
> https://sourceware.org/pipermail/bzip2-devel/2020q2/000107.html
> It was easy to add exit to the async_signal_unsafe_fns, but since
> there is a signal safe _exit call (which is what you should use in a
> signal handler, so no unsafe calls are made, like to the at_exit
> handlers) I also wanted to add a fixit hint.
I like this idea.
> The fixit hint is emitted, but it is somewhat hard to see. Is there a
> better way to do this for analyzer warnings so that it is a bit more
> prominent?
>
> This is how it currently looks:
>
> /opt/local/gcc/bin/gcc -g -O2 -fanalyzer -c bzip2.c
> bzip2.c: In function ‘mySIGSEGVorSIGBUScatcher’:
> bzip2.c:874:4: warning: call to ‘exit’ from within signal handler
> [CWE-479] [-Wanalyzer-unsafe-call-within-signal-handler]
> 874 |exit(exitValue);
> |^~~
> |_exit
> ‘main’: events 1-2
> |
> | 1784 | IntNative main ( IntNative argc, Char *argv[] )
> | | ^~~~
> | | |
> | | (1) entry to ‘main’
> |..
> | 1800 |smallMode = False;
> | |~
> | ||
> | |(2) registering ‘mySIGSEGVorSIGBUScatcher’ as signal
> handler
> |
> event 3
> |
> |cc1:
> | (3): later on, when the signal is delivered to the process
> |
> +--> ‘mySIGSEGVorSIGBUScatcher’: events 4-5
>|
>| 816 | void mySIGSEGVorSIGBUScatcher ( IntNative n )
>| | ^~~~
>| | |
>| | (4) entry to ‘mySIGSEGVorSIGBUScatcher’
>|..
>| 874 |exit(exitValue);
>| |~~~
>| ||
>| |(5) call to ‘exit’ from within signal handler
>|
>
> Thanks,
I think this ought to be implemented as a followup "note" diagnostic
via the "inform" API, so that the followup message is separate from the
initial warning, and thus separate from the path...
[...]
> diagnostic_metadata m;
> /* CWE-479: Signal Handler Use of a Non-reentrant Function. */
> m.add_cwe (479);
> +if (m_replacement != NULL)
> + rich_loc->add_fixit_replace (m_replacement);
> return warning_meta (rich_loc, m,
>OPT_Wanalyzer_unsafe_call_within_signal_handle
> r,
>"call to %qD from within signal handler",
How about something like this (though I even haven't checked if it
compiles, and am not 100% sure what the wording should be):
bool emit (rich_location *rich_loc) FINAL OVERRIDE
{
diagnostic_metadata m;
/* CWE-479: Signal Handler Use of a Non-reentrant Function. */
m.add_cwe (479);
if (warning_meta (rich_loc, m,
OPT_Wanalyzer_unsafe_call_within_signal_handler,
"call to %qD from within signal handler",
m_unsafe_fndecl))
{
if (m_replacement)
{
gcc_rich_location note_rich_loc (gimple_location (m_unsafe_call));
note_rich_loc.add_fixit_replace (m_replacement);
inform (¬e_rich_loc, "%qs is a signal-safe replacement for %qD",
m_replacement, unsafe_fndecl);
}
return true;
}
return false;
}
Dave
Re: [RFC] analyzer: Add exit, and _exit replacement, to sm-signal.
On Sun, 2020-05-17 at 18:39 -0400, David Malcolm via Gcc-patches wrote:
> On Mon, 2020-05-18 at 00:05 +0200, Mark Wielaard wrote:
[...snip...]
> How about something like this (though I even haven't checked if it
> compiles, and am not 100% sure what the wording should be):
>
> bool emit (rich_location *rich_loc) FINAL OVERRIDE
> {
> diagnostic_metadata m;
> /* CWE-479: Signal Handler Use of a Non-reentrant Function. */
> m.add_cwe (479);
...and there should be this here:
auto_diagnostic_group d;
to associate the note with the warning.
> if (warning_meta (rich_loc, m,
> OPT_Wanalyzer_unsafe_call_within_signal_handler,
> "call to %qD from within signal handler",
> m_unsafe_fndecl))
> {
> if (m_replacement)
> {
> gcc_rich_location note_rich_loc (gimple_location
> (m_unsafe_call));
> note_rich_loc.add_fixit_replace (m_replacement);
> inform (¬e_rich_loc, "%qs is a signal-safe replacement
> for %qD",
> m_replacement, unsafe_fndecl);
> }
> return true;
> }
> return false;
> }
Re: [RFC] analyzer: Add exit, and _exit replacement, to sm-signal.
On Mon, 2020-05-18 at 00:05 +0200, Mark Wielaard wrote: > Hi, > > While trying out -fanalyzer on the bzip2 source code I noticed that > it > did warn about some unsafe calls in the signal handler, but din't > warn > about the exit call: > https://sourceware.org/pipermail/bzip2-devel/2020q2/000107.html > > It was easy to add exit to the async_signal_unsafe_fns, but since > there is a signal safe _exit call (which is what you should use in a > signal handler, so no unsafe calls are made, like to the at_exit > handlers) I also wanted to add a fixit hint. > > The fixit hint is emitted, but it is somewhat hard to see. Is there a > better way to do this for analyzer warnings so that it is a bit more > prominent? > > This is how it currently looks: > > /opt/local/gcc/bin/gcc -g -O2 -fanalyzer -c bzip2.c > bzip2.c: In function ‘mySIGSEGVorSIGBUScatcher’: > bzip2.c:874:4: warning: call to ‘exit’ from within signal handler > [CWE-479] [-Wanalyzer-unsafe-call-within-signal-handler] > 874 |exit(exitValue); > |^~~ > |_exit > ‘main’: events 1-2 > | > | 1784 | IntNative main ( IntNative argc, Char *argv[] ) > | | ^~~~ > | | | > | | (1) entry to ‘main’ > |.. > | 1800 |smallMode = False; > | |~ > | || > | |(2) registering ‘mySIGSEGVorSIGBUScatcher’ as signal > handler > | BTW, it looks like it's using the wrong location for event (2). It ought to be showing a call to "signal", not an assignment. Please can you file a bug about this, and attach the source in question so I can take a look at some point. Thanks Dave > event 3 > | > |cc1: > | (3): later on, when the signal is delivered to the process > | > +--> ‘mySIGSEGVorSIGBUScatcher’: events 4-5 >| >| 816 | void mySIGSEGVorSIGBUScatcher ( IntNative n ) >| | ^~~~ >| | | >| | (4) entry to ‘mySIGSEGVorSIGBUScatcher’ >|.. >| 874 |exit(exitValue); >| |~~~ >| || >| |(5) call to ‘exit’ from within signal handler >| > > Thanks, > > Mark >
[PATCH] aarch64: Change the definition of Pmode [pr95182]
Hi, This changes the definition of Pmode for aarch64 port. Unlike x86, S390 etc., Pmode is always set to DImode for aarch64 port even under ILP32. Because of that definition, machine mode of symbol_ref which is supposed to be SImode becomes DImode under target ILP32. Definition of Pmode should depend on the current ABI, i.e., SImode for ILP32 and DImode for LP64. Attached please find the proposed patch . Bootstrap and tested on aarch64 Linux platform. No new regression witnessed. Any suggestion? Thanks, Duanbo pr95182-v0.patch Description: pr95182-v0.patch
Re: [PATCH] aarch64: Change the definition of Pmode [pr95182]
On Sun, May 17, 2020 at 8:23 PM duanbo (C) wrote:
>
> Hi,
>
> This changes the definition of Pmode for aarch64 port.
> Unlike x86, S390 etc., Pmode is always set to DImode for aarch64 port even
> under ILP32.
> Because of that definition, machine mode of symbol_ref which is supposed to
> be SImode becomes DImode under target ILP32.
> Definition of Pmode should depend on the current ABI, i.e., SImode for ILP32
> and DImode for LP64.
> Attached please find the proposed patch .
> Bootstrap and tested on aarch64 Linux platform. No new regression witnessed.
> Any suggestion?
THIS DOES NOT WORK correctly and will never work correctly. When I
was originally writing AARCH64 ILP32 (back in 2013), I went this route
first (as it was the fastest way to get it working; I could not wait
on ARM's implementation at the time) but I had regressions.
The place where it fails was something like:
int f(char *g, int t)
{
return g[t];
}
Which you pass -1 for t as there would be no zero-extend any more.
I remember at least one testcase in the testsuite failing due to
implementation this way even; I don't remember which one as I did not
write it down and it was over 6 years ago.
If there was an arch mode which would VAs to be truncated to 32bits,
this would be the correct way to implement this.
The reason why the other ABIs/targets define Pmode as SImode is
because the underlying hardware will extend the VA correctly as Linux
will set the arch bit correctly (NOTE MIPS is an example where
index'ed load/stores which has a similar issue even on MIPS32 but that
is a different story).
Also there are other ABIs where Pmode != PTRmode (e.g. IA64-HPUX32).
x32 has an option which can select either way. The ISA on x86_64
supports both cases which is why it can be selected that way; this is
unlike AARCH64 which cannot.
Thanks,
Andrew Pinski
>
> Thanks,
> Duanbo
[PATCH] Fortran : ProcPtr function results: 'ppr@' in error message PR39695
Please find attache a patch for PR39695 Commit message: Fortran : ProcPtr function results: 'ppr@' in error message PR39695 The value 'ppr@' is set in the name of result symbol, the actual name of the symbol is in the procedure name symbol pointed to by the result symbol's namespace (ns). When reporting errors for symbols that have the proc_pointer attribute check whether the result attribute is set and set the name accordingly. 2020-05-18 Mark Eggleston gcc/fortran/ PR fortran/39695 * resolve.c (resolve_fl_procedure): Set name depending on whether the result attribute is set. For PROCEDURE/RESULT conflict use the name in sym->ns->proc_name->name. * symbol.c (gfc_add_type): Add check for function and result attributes use sym->ns->proc_name->name if both are set. Where the symbol cannot have a type use the name in sym->ns->proc_name->name. 2020-05-18 Mark Eggleston gcc/testsuite/ PR fortran/39695 * gfortran.dg/pr39695_1.f90: New test. * gfortran.dg/pr39695_2.f90: New test. * gfortran.dg/pr39695_3.f90: New test. * gfortran.dg/pr39695_4.f90: New test. Tested on x86_64 using make check-fortran for master, gcc-8, gcc-9 and gcc-10. OK to to commit and backport? -- https://www.codethink.co.uk/privacy.html
