stabilize_reference question

[Mike Stump]

> To: mrs@wrs.com (Mike Stump)
> Date: Tue, 09 Mar 1999 18:14:31 -0800
> From: Per Bothner <bothner@cygnus.com>

> The PR was submitted Apr 22. 1996.  It does discuss volatile
> and the comma operator.  It is not very conclusive.

I have the feeling it will turn out to be unrelated.


Let me try another hand at resynthing the bug report:

char * volatile p;
char * n;
void foo() {
  delete (--p, n);
}

The , should put in a compound, and delete should stabilize it...

Now, we try this with and without my patch, and we get:

	sethi	%hi(p), %o1
	or	%o1, %lo(p), %o0
	sethi	%hi(p), %o1
	or	%o1, %lo(p), %o0
	sethi	%hi(p), %o2
	or	%o2, %lo(p), %o1
	ld	[%o1], %o2
	add	%o2, -1, %o1
	st	%o1, [%o0]
	sethi	%hi(n), %o1
	or	%o1, %lo(n), %o0
	ld	[%o0], %l0
	mov	%l0, %o0
	call	__builtin_delete, 0

and

	sethi	%hi(p), %o1
	or	%o1, %lo(p), %o0
	sethi	%hi(p), %o1
	or	%o1, %lo(p), %o0
	sethi	%hi(p), %o2
	or	%o2, %lo(p), %o1
	ld	[%o1], %o2
	add	%o2, -1, %o1
	st	%o1, [%o0]
	ld	[%o0], %o0
	mov	%o0, %l0
	sethi	%hi(n), %o0
	or	%o0, %lo(n), %o1
	ld	[%o1], %o0
	call	__builtin_delete, 0

Notice the extra read, bad!  The code just puts a save_expr on it,
can't work.  Now, if you want to build a VOID_mode save, that should
work:

Doing diffs in tree.c.~1~:
*** tree.c.~1~	Mon Mar  1 19:02:22 1999
--- tree.c	Tue Mar  9 20:30:58 1999
*************** stabilize_reference (ref)
*** 2859,2868 ****
        break;
  
      case COMPOUND_EXPR:
!       /* We cannot wrap the first expression in a SAVE_EXPR, as then
! 	 it wouldn't be ignored.  This matters when dealing with
! 	 volatiles.  */
!       return stabilize_reference_1 (ref);
  
      case RTL_EXPR:
        result = build1 (INDIRECT_REF, TREE_TYPE (ref),
--- 2859,2878 ----
        break;
  
      case COMPOUND_EXPR:
! 
!       /* We MUST wind up ignoring the first value.  */
! 
!       result = stabilize_reference_1 (TREE_OPERAND (ref, 0));
!       if (TREE_CODE (result) == SAVE_EXPR)
! 	TREE_TYPE (result) = void_type_node;
!       else
! 	if (result != TREE_OPERAND (ref, 0))
! 	  abort ();
! 
!       result = build_nt (COMPOUND_EXPR,
! 			 result,
! 			 stabilize_reference (TREE_OPERAND (ref, 1)));
!       break;
  
      case RTL_EXPR:
        result = build1 (INDIRECT_REF, TREE_TYPE (ref),
--------------

I tried it out, and it does work.  I'll leave the cleanup to you...  I
don't like the abort, but I like it more than incorrect code.  The
problem is if someone optimizes save_expr to do something far more
interesting (reasonable), then we can't undo the operation.  We can
fall back to what it was before, but if Java wants this change for
correctness and not optimization, that would be bad.  If it is only
for opimization, then the fall back would be good.  Else, you can just
add an argument to save_expr to ignore the value (ick, hate to change
all the callers), or convert the argument to void_type_node before
hand...

Hum, maybe the last is the best...