egcs 2.91.14: packed-1 failure on openvms/alpha, more info

egcs 2.91.14: packed-1 failure on openvms/alpha, more info

[kkaempf]

Looks like there is a bug in simplify_shift_const() (combine.c)
for mixed 32/64 bit targets.

It is called from alpha_expand_unaligned_load() in alpha/alpha.c
for the signed word case (see comment there).

Adding a bit of debug output reveals a wrong simplification

simplify_shift_const (code 81(ashiftrt), mode 6(DI), count 48)

x:	(ashiftrt:DI (and:DI (reg:DI 82)
	        (const_int -281474976710656 0xffff000000000000))
	    (const_int 48 0x30))

varop:	(and:DI (reg:DI 82)
	    (const_int -281474976710656 0xffff000000000000))

returns
 
	(ashiftrt:DI (reg:DI 82)
	    (const_int 63 0x3f))


So instead of removing the (unneeded) 'and' operation from the
ashiftrt operand, the shift count is set to the wrong value.

The code in question is at line 8338 in combine.c:

      /* If we are doing an arithmetic right shift and discarding all but
         the sign bit copies, this is equivalent to doing a shift by the
         bitsize minus one.  Convert it into that shift because it will often
         allow other simplifications.  */
      
      if (code == ASHIFTRT
          && (count + num_sign_bit_copies (varop, shift_mode)
              >= GET_MODE_BITSIZE (shift_mode)))
        count = GET_MODE_BITSIZE (shift_mode) - 1;
         
since num_sign_bit_copies() returns 16 and count is set to 63.

Klaus
-- 
proGIS Software                 E-Mail: kkaempf@progis.de
Dipl.-Inform. Klaus K"ampf      Fax:    0241-47067-29
Jakobstr. 117                   Voice:  0241-47067-11
D-52064 Aachen                  WWW:	http://www.progis.de

Re: egcs 2.91.14: packed-1 failure on openvms/alpha, more info

[Jim Wilson]

The problem here is that pseudo 82 was marked as a pointer, because it holds
the result of pointer arithmetic at one point, and num_sign_bit_copies in
combine `knows' that pointers are always 32 bit sign extended values for
alpha-dec-vms.  However, the pseudo is later re-assigned a non-pointer 64 bit
value, and then num_sign_bit_copies returns the wrong value because gcc still
thinks it holds a pointer value.

We can fix this by not mixing pointer and non-pointer values in the same
pseudo.  This patch seems to work.

*** patches-egcs-980321/gcc/config/alpha/alpha.c	Tue Feb 24 16:00:27 1998
--- kaempf-egcs-980321/gcc/config/alpha/alpha.c	Wed Mar 25 17:06:08 1998
*************** alpha_expand_unaligned_load (tgt, mem, s
*** 1337,1343 ****
        emit_insn (gen_extxl (extl, meml, GEN_INT (64), addr));
        emit_insn (gen_extqh (exth, memh, addr));
  
!       addr = expand_binop (DImode, ior_optab, extl, exth, addr, 1, OPTAB_WIDEN);
        addr = expand_binop (DImode, ashr_optab, addr, GEN_INT (48), 
  			   addr, 1, OPTAB_WIDEN);
      }
--- 1337,1346 ----
        emit_insn (gen_extxl (extl, meml, GEN_INT (64), addr));
        emit_insn (gen_extqh (exth, memh, addr));
  
!       /* We must use tgt here for the target.  Alpha-vms port fails if we use
! 	 addr for the target, because addr is marked as a pointer and combine
! 	 knows that pointers are always sign-extended 32 bit values.  */
!       addr = expand_binop (DImode, ior_optab, extl, exth, tgt, 1, OPTAB_WIDEN);
        addr = expand_binop (DImode, ashr_optab, addr, GEN_INT (48), 
  			   addr, 1, OPTAB_WIDEN);
      }

Re: egcs 2.91.14: packed-1 failure on openvms/alpha, more info

[Jim Wilson]

	Is this happening in reg_scan_mark_refs?  We're going to need to
	throttle that code to be less aggressive for gcse anyway.

yes.

Jim

Re: egcs 2.91.14: packed-1 failure on openvms/alpha, more info

[Jeffrey A Law]

  In message < 199803262235.OAA09101@rtl.cygnus.com >you write:
  > 	Is this happening in reg_scan_mark_refs?  We're going to need to
  > 	throttle that code to be less aggressive for gcse anyway.
  > 
  > yes.
OK.  I'll bring over my change from devo.

In theory the whole marking of pointer registers by reg_scan_mark_refs
should be completely reworked to not lose in the presense of global
optimizations.  But I haven't come up with the right conceptual model
yet.

Who knows, that code might even be totally useless now that we
propagate alias information better.

jeff

Re: egcs 2.91.14: packed-1 failure on openvms/alpha, more info

[Jeffrey A Law]

  In message < 199803260122.RAA15789@rtl.cygnus.com >you write:
  > The problem here is that pseudo 82 was marked as a pointer,
  > because it holds the result of pointer arithmetic at one point,
Is this happening in reg_scan_mark_refs?  We're going to need to
throttle that code to be less aggressive for gcse anyway.

You patch is probably still useful though.

jeff

Re: egcs 2.91.14: packed-1 failure on openvms/alpha, more info

[Jeffrey A Law]

  In message < 19980330145218.21981@dot.cygnus.com >you write:
  > On Thu, Mar 26, 1998 at 04:33:54PM -0700, Jeffrey A Law wrote:
  > > Who knows, that code might even be totally useless now that we
  > > propagate alias information better.
  > 
  > Is that the same code that records "known alignment" of pointers
  > in registers?  That information is still useful, though the way
  > we keep track of it seems truely ugly.
I don't think it records known alignment at that particular time;
just the fact that if A is a pointer, then A + C is a pointer
(not strictly true, but for most machines that's good enough).

It may be the case that we propagate alignment stuff after we
set REGNO_POINTER_FLAG on these "derived" pointers, but I haven't
looked.

That is something we might want to do as part of the alias analysis
code though....

jeff

Re: egcs 2.91.14: packed-1 failure on openvms/alpha, more info

[Richard Henderson]

On Thu, Mar 26, 1998 at 04:33:54PM -0700, Jeffrey A Law wrote:
> Who knows, that code might even be totally useless now that we
> propagate alias information better.

Is that the same code that records "known alignment" of pointers
in registers?  That information is still useful, though the way
we keep track of it seems truely ugly.


r~

Re: egcs 2.91.14: packed-1 failure on openvms/alpha, more info

[Jim Wilson]

I went ahead and installed the alpha.c patch I wrote for this problem.

Jim