Re: [16/23] recog: Add a way of temporarily undoing changes

2020-12-16 Thread Richard Sandiford via Gcc-patches
Jeff Law via Gcc-patches  writes:
> On 11/13/20 1:19 AM, Richard Sandiford via Gcc-patches wrote:
>> In some cases, it can be convenient to roll back the changes that
>> have been made by validate_change to see how things looked before,
>> then reroll the changes.  For example, this makes it possible
>> to defer calculating the cost of an instruction until we know that
>> the result is actually needed.  It can also make dumps easier to read.
>>
>> This patch adds a couple of helper functions for doing that.
>>
>> gcc/
>>  * recog.h (temporarily_undo_changes, redo_changes): Declare.
>>  * recog.c (swap_change, temporarily_undo_changes): New functions.
>>  (redo_changes): Likewise.
> OK...  But...
> +
>> +/* Temporarily undo all the changes numbered NUM and up, with a view
>> +   to reapplying them later.  The next call to the changes machinery
>> +   must be:
>> +
>> +  redo_changes (NUM)
>> +
>> +   otherwise things will end up in an invalid state.  */
> It'd be nice if we had state validation in the other routines. Somebody
> is likely to mess this up at some point...

Yeah, good point, can definitely see myself doing that. :-)

Here's the version I committed, with temporarily_undone_changes tracking
what has been undone.

Thanks,
Richard


gcc/
* recog.h (temporarily_undo_changes, redo_changes): Declare.
* recog.c (temporarily_undone_changes): New variable.
(validate_change_1, confirm_change_group): Check that it's zero.
(cancel_changes): Likewise.
(swap_change, temporarily_undo_changes): New functions.
(redo_changes): Likewise.
---
 gcc/recog.c | 48 
 gcc/recog.h |  2 ++
 2 files changed, 50 insertions(+)

diff --git a/gcc/recog.c b/gcc/recog.c
index 65125b8f0d1..cee481f4fa0 100644
--- a/gcc/recog.c
+++ b/gcc/recog.c
@@ -193,6 +193,7 @@ static change_t *changes;
 static int changes_allocated;
 
 static int num_changes = 0;
+static int temporarily_undone_changes = 0;
 
 /* Validate a proposed change to OBJECT.  LOC is the location in the rtl
at which NEW_RTX will be placed.  If NEW_LEN is >= 0, XVECLEN (NEW_RTX, 0)
@@ -218,6 +219,7 @@ static bool
 validate_change_1 (rtx object, rtx *loc, rtx new_rtx, bool in_group,
   bool unshare, int new_len = -1)
 {
+  gcc_assert (temporarily_undone_changes == 0);
   rtx old = *loc;
 
   /* Single-element parallels aren't valid and won't match anything.
@@ -506,6 +508,7 @@ confirm_change_group (void)
   int i;
   rtx last_object = NULL;
 
+  gcc_assert (temporarily_undone_changes == 0);
   for (i = 0; i < num_changes; i++)
 {
   rtx object = changes[i].object;
@@ -561,6 +564,7 @@ num_validated_changes (void)
 void
 cancel_changes (int num)
 {
+  gcc_assert (temporarily_undone_changes == 0);
   int i;
 
   /* Back out all the changes.  Do this in the opposite order in which
@@ -577,6 +581,50 @@ cancel_changes (int num)
   num_changes = num;
 }
 
+/* Swap the status of change NUM from being applied to not being applied,
+   or vice versa.  */
+
+static void
+swap_change (int num)
+{
+  if (changes[num].old_len >= 0)
+std::swap (XVECLEN (*changes[num].loc, 0), changes[num].old_len);
+  else
+std::swap (*changes[num].loc, changes[num].old);
+  if (changes[num].object && !MEM_P (changes[num].object))
+std::swap (INSN_CODE (changes[num].object), changes[num].old_code);
+}
+
+/* Temporarily undo all the changes numbered NUM and up, with a view
+   to reapplying them later.  The next call to the changes machinery
+   must be:
+
+  redo_changes (NUM)
+
+   otherwise things will end up in an invalid state.  */
+
+void
+temporarily_undo_changes (int num)
+{
+  gcc_assert (temporarily_undone_changes == 0 && num <= num_changes);
+  for (int i = num_changes - 1; i >= num; i--)
+swap_change (i);
+  temporarily_undone_changes = num_changes - num;
+}
+
+/* Redo the changes that were temporarily undone by:
+
+  temporarily_undo_changes (NUM).  */
+
+void
+redo_changes (int num)
+{
+  gcc_assert (temporarily_undone_changes == num_changes - num);
+  for (int i = num; i < num_changes; ++i)
+swap_change (i);
+  temporarily_undone_changes = 0;
+}
+
 /* Reduce conditional compilation elsewhere.  */
 /* A subroutine of validate_replace_rtx_1 that tries to simplify the resulting
rtx.  */
diff --git a/gcc/recog.h b/gcc/recog.h
index e152e2bb591..facf36e7c08 100644
--- a/gcc/recog.h
+++ b/gcc/recog.h
@@ -96,6 +96,8 @@ extern void confirm_change_group (void);
 extern int apply_change_group (void);
 extern int num_validated_changes (void);
 extern void cancel_changes (int);
+extern void temporarily_undo_changes (int);
+extern void redo_changes (int);
 extern int constrain_operands (int, alternative_mask);
 extern int constrain_operands_cached (rtx_insn *, int);
 extern int memory_address_addr_space_p (machine_mode, rtx, addr_space_t);


Re: [16/23] recog: Add a way of temporarily undoing changes

2020-11-25 Thread Jeff Law via Gcc-patches



On 11/13/20 1:19 AM, Richard Sandiford via Gcc-patches wrote:
> In some cases, it can be convenient to roll back the changes that
> have been made by validate_change to see how things looked before,
> then reroll the changes.  For example, this makes it possible
> to defer calculating the cost of an instruction until we know that
> the result is actually needed.  It can also make dumps easier to read.
>
> This patch adds a couple of helper functions for doing that.
>
> gcc/
>   * recog.h (temporarily_undo_changes, redo_changes): Declare.
>   * recog.c (swap_change, temporarily_undo_changes): New functions.
>   (redo_changes): Likewise.
OK...  But...
+
> +/* Temporarily undo all the changes numbered NUM and up, with a view
> +   to reapplying them later.  The next call to the changes machinery
> +   must be:
> +
> +  redo_changes (NUM)
> +
> +   otherwise things will end up in an invalid state.  */
It'd be nice if we had state validation in the other routines. Somebody
is likely to mess this up at some point...


jeff




[16/23] recog: Add a way of temporarily undoing changes

2020-11-13 Thread Richard Sandiford via Gcc-patches
In some cases, it can be convenient to roll back the changes that
have been made by validate_change to see how things looked before,
then reroll the changes.  For example, this makes it possible
to defer calculating the cost of an instruction until we know that
the result is actually needed.  It can also make dumps easier to read.

This patch adds a couple of helper functions for doing that.

gcc/
* recog.h (temporarily_undo_changes, redo_changes): Declare.
* recog.c (swap_change, temporarily_undo_changes): New functions.
(redo_changes): Likewise.
---
 gcc/recog.c | 40 
 gcc/recog.h |  2 ++
 2 files changed, 42 insertions(+)

diff --git a/gcc/recog.c b/gcc/recog.c
index 65125b8f0d1..309a578a151 100644
--- a/gcc/recog.c
+++ b/gcc/recog.c
@@ -577,6 +577,46 @@ cancel_changes (int num)
   num_changes = num;
 }
 
+/* Swap the status of change NUM from being applied to not being applied,
+   or vice versa.  */
+
+static void
+swap_change (int num)
+{
+  if (changes[num].old_len >= 0)
+std::swap (XVECLEN (*changes[num].loc, 0), changes[num].old_len);
+  else
+std::swap (*changes[num].loc, changes[num].old);
+  if (changes[num].object && !MEM_P (changes[num].object))
+std::swap (INSN_CODE (changes[num].object), changes[num].old_code);
+}
+
+/* Temporarily undo all the changes numbered NUM and up, with a view
+   to reapplying them later.  The next call to the changes machinery
+   must be:
+
+  redo_changes (NUM)
+
+   otherwise things will end up in an invalid state.  */
+
+void
+temporarily_undo_changes (int num)
+{
+  for (int i = num_changes - 1; i >= num; i--)
+swap_change (i);
+}
+
+/* Redo the changes that were temporarily undone by:
+
+  temporarily_undo_changes (NUM).  */
+
+void
+redo_changes (int num)
+{
+  for (int i = num; i < num_changes; ++i)
+swap_change (i);
+}
+
 /* Reduce conditional compilation elsewhere.  */
 /* A subroutine of validate_replace_rtx_1 that tries to simplify the resulting
rtx.  */
diff --git a/gcc/recog.h b/gcc/recog.h
index e152e2bb591..facf36e7c08 100644
--- a/gcc/recog.h
+++ b/gcc/recog.h
@@ -96,6 +96,8 @@ extern void confirm_change_group (void);
 extern int apply_change_group (void);
 extern int num_validated_changes (void);
 extern void cancel_changes (int);
+extern void temporarily_undo_changes (int);
+extern void redo_changes (int);
 extern int constrain_operands (int, alternative_mask);
 extern int constrain_operands_cached (rtx_insn *, int);
 extern int memory_address_addr_space_p (machine_mode, rtx, addr_space_t);
-- 
2.17.1