Re: [PATCH] [Stage1] Refactor tree-ssa-operands.c

[Martin Sebor via Gcc-patches]

On 4/22/20 12:39 PM, Giuliano Belinassi via Gcc-patches wrote:

This patch refactors tree-ssa-operands.c by wrapping the global
variables into a class, and also removes unused code.


Just a few comments on this nice cleanup:

It looks to me like class build_virtual_operands isn't safe to copy
or assign due its vec member.  If that's correct, can you please
either disable copying and assignments for such classes or make sure
they can be?  (The macro DISABLE_COPY_AND_ASSIGN will do the former.)

I would also suggest to avoid declaring member functions inline.
It serves no purpose if the function is also defined inline later
on and  causes problems when it isn't (e.g., when the definition
is moved to a .cc file).

Finally, as a purely cosmetic improvement, although it's benign
the void parameter in a parameter list can be omitted in C++.

Martin



Just sending this for when Stage1 is back again.

I ran the testsuite and bootstraped in a x86_64 linux machine and
found no issues.

gcc/ChangeLog:
2020-04-22  Giuliano Belinassi  

* tree-ssa-operands.c (build_virtual_operands): New class.
(operands_bitmap_obstack): Remove.
(n_initialized): Remove.
(build_uses): Move to build_virtual_operands class.
(build_vuse): Same as above.
(build_vdef): Same as above.
(verify_ssa_operands): Same as above.
(finalize_ssa_uses): Same as above.
(cleanup_build_arrays): Same as above.
(finalize_ssa_stmt_operands): Same as above.
(start_ssa_stmt_operands): Same as above.
(append_use): Same as above.
(append_vdef): Same as above.
(add_virtual_operand): Same as above.
(add_stmt_operand): Same as above.
(get_mem_ref_operands): Same as above.
(get_tmr_operands): Same as above.
(maybe_add_call_vops): Same as above.
(get_asm_stmt_operands): Same as above.
(get_expr_operands): Same as above.
(parse_ssa_operands): Same as above.
(finalize_ssa_defs): Same as above.
(build_ssa_operands): Same as above, plus create a C-like wrapper.
(update_stmt_operands): Create an instance of build_virtual_operands.

Re: [PATCH] [Stage1] Refactor tree-ssa-operands.c

[Richard Biener via Gcc-patches]

On Wed, Apr 22, 2020 at 8:40 PM Giuliano Belinassi
 wrote:
>
> This patch refactors tree-ssa-operands.c by wrapping the global
> variables into a class, and also removes unused code.
>
> Just sending this for when Stage1 is back again.
>
> I ran the testsuite and bootstraped in a x86_64 linux machine and
> found no issues.

First of all thanks for doing this.  I have a few editorial suggestions
about the class setup - first the name build_virtual_operands is
badly chosen, I prefer operand_scanner.  Second I suggest to
have the CTOR take the invariants as arguments which is the
stmt we operate on and its containing function.  Thus,

  operand_scanner (function *, gimple *);

which makes passing those down functions unnecessary.

Since build_vuses is now a member and allocated for each stmt
which would be a regression I'd suggest to use an auto_vec
with some pre-allocated storage, thus change it to

  auto_vec build_uses;

that also makes the destructor trivial (please simply remove
cleanup_build_arrays).  I guess there's further possibilities
for streamlining the initialization/teardown process but that's
better done as followup.

Otherwise the change looks OK to me.

Thanks,
Richard.

> gcc/ChangeLog:
> 2020-04-22  Giuliano Belinassi  
>
> * tree-ssa-operands.c (build_virtual_operands): New class.
> (operands_bitmap_obstack): Remove.
> (n_initialized): Remove.
> (build_uses): Move to build_virtual_operands class.
> (build_vuse): Same as above.
> (build_vdef): Same as above.
> (verify_ssa_operands): Same as above.
> (finalize_ssa_uses): Same as above.
> (cleanup_build_arrays): Same as above.
> (finalize_ssa_stmt_operands): Same as above.
> (start_ssa_stmt_operands): Same as above.
> (append_use): Same as above.
> (append_vdef): Same as above.
> (add_virtual_operand): Same as above.
> (add_stmt_operand): Same as above.
> (get_mem_ref_operands): Same as above.
> (get_tmr_operands): Same as above.
> (maybe_add_call_vops): Same as above.
> (get_asm_stmt_operands): Same as above.
> (get_expr_operands): Same as above.
> (parse_ssa_operands): Same as above.
> (finalize_ssa_defs): Same as above.
> (build_ssa_operands): Same as above, plus create a C-like wrapper.
> (update_stmt_operands): Create an instance of build_virtual_operands.

[PATCH] [Stage1] Refactor tree-ssa-operands.c

[Giuliano Belinassi via Gcc-patches]

This patch refactors tree-ssa-operands.c by wrapping the global
variables into a class, and also removes unused code.

Just sending this for when Stage1 is back again.

I ran the testsuite and bootstraped in a x86_64 linux machine and
found no issues.

gcc/ChangeLog:
2020-04-22  Giuliano Belinassi  

* tree-ssa-operands.c (build_virtual_operands): New class.
(operands_bitmap_obstack): Remove.
(n_initialized): Remove.
(build_uses): Move to build_virtual_operands class.
(build_vuse): Same as above.
(build_vdef): Same as above.
(verify_ssa_operands): Same as above.
(finalize_ssa_uses): Same as above.
(cleanup_build_arrays): Same as above.
(finalize_ssa_stmt_operands): Same as above.
(start_ssa_stmt_operands): Same as above.
(append_use): Same as above.
(append_vdef): Same as above.
(add_virtual_operand): Same as above.
(add_stmt_operand): Same as above.
(get_mem_ref_operands): Same as above.
(get_tmr_operands): Same as above.
(maybe_add_call_vops): Same as above.
(get_asm_stmt_operands): Same as above.
(get_expr_operands): Same as above.
(parse_ssa_operands): Same as above.
(finalize_ssa_defs): Same as above.
(build_ssa_operands): Same as above, plus create a C-like wrapper.
(update_stmt_operands): Create an instance of build_virtual_operands.
>From b860b39045e1b90319caa7c75ad189514e4a5641 Mon Sep 17 00:00:00 2001
From: Giuliano Belinassi 
Date: Tue, 21 Apr 2020 19:38:37 -0300
Subject: [PATCH] [Stage1] Refactor tree-ssa-operands.c

Refactor tree-ssa-operands.c by wrapping the global
variables into a class, and also removes unused code.

gcc/ChangeLog:
2020-04-22  Giuliano Belinassi  

	* tree-ssa-operands.c (build_virtual_operands): New class.
	(operands_bitmap_obstack): Remove.
	(n_initialized): Remove.
	(build_uses): Move to build_virtual_operands class.
	(build_vuse): Same as above.
	(build_vdef): Same as above.
	(verify_ssa_operands): Same as above.
	(finalize_ssa_uses): Same as above.
	(cleanup_build_arrays): Same as above.
	(finalize_ssa_stmt_operands): Same as above.
	(start_ssa_stmt_operands): Same as above.
	(append_use): Same as above.
	(append_vdef): Same as above.
	(add_virtual_operand): Same as above.
	(add_stmt_operand): Same as above.
	(get_mem_ref_operands): Same as above.
	(get_tmr_operands): Same as above.
	(maybe_add_call_vops): Same as above.
	(get_asm_stmt_operands): Same as above.
	(get_expr_operands): Same as above.
	(parse_ssa_operands): Same as above.
	(finalize_ssa_defs): Same as above.
	(build_ssa_operands): Same as above, plus create a C-like wrapper.
	(update_stmt_operands): Create an instance of build_virtual_operands.
---
 gcc/tree-ssa-operands.c | 218 +++-
 1 file changed, 150 insertions(+), 68 deletions(-)

diff --git a/gcc/tree-ssa-operands.c b/gcc/tree-ssa-operands.c
index b525ee318a4..ae8734d17e2 100644
--- a/gcc/tree-ssa-operands.c
+++ b/gcc/tree-ssa-operands.c
@@ -99,23 +99,111 @@ along with GCC; see the file COPYING3.  If not see
 /* Operand is having its address taken.  */
 #define opf_address_taken (1 << 5)
 
-/* Array for building all the use operands.  */
-static vec build_uses;
+/* Class containing temporary per-stmt state.  */
 
-/* The built VDEF operand.  */
-static tree build_vdef;
+class build_virtual_operands
+{
+  public:
+build_virtual_operands ()
+  {
+	build_uses.create (10);
+	build_vuse = NULL_TREE;
+	build_vdef = NULL_TREE;
+  }
+
+~build_virtual_operands ()
+  {
+	cleanup_build_arrays ();
+	build_uses.release ();
+  }
+
+/* Create an operands cache for STMT.  */
+void build_ssa_operands (struct function *fn, gimple *stmt);
+
+/* Verifies SSA statement operands.  */
+DEBUG_FUNCTION bool verify_ssa_operands (struct function *fn, gimple *stmt);
+
+  private:
+/* Array for building all the use operands.  */
+vec build_uses;
+
+/* The built VDEF operand.  */
+tree build_vdef;
+
+/* The built VUSE operand.  */
+tree build_vuse;
+
+/* Takes elements from build_uses and turns them into use operands of STMT.  */
+inline void finalize_ssa_uses (struct function *fn, gimple *stmt);
+
+/* Clear the in_list bits and empty the build array for VDEFs and
+   VUSEs.  */
+inline void cleanup_build_arrays (void);
+
+/* Finalize all the build vectors, fill the new ones into INFO.  */
+inline void finalize_ssa_stmt_operands (struct function *fn, gimple *stmt);
+
+/* Start the process of building up operands vectors in INFO.  */
+inline void start_ssa_stmt_operands (void);
+
+/* Add USE_P to the list of pointers to operands.  */
+inline void append_use (tree *use_p);
 
-/* The built VUSE operand.  */
-static tree build_vuse;
+/* Add VAR to the set of variables that require a VDEF operator.  */
+inline void append_vdef