Re: Rationale for an old TRUNCATE patch

2009-06-17 Thread Adam Nemet
Jeff Law writes:
 Ian Lance Taylor wrote:
  Adam Nemet ane...@caviumnetworks.com writes:
 

  I am trying to understand the checkin by Jeff Law from about 11 years ago:
 
 r19204 | law | 1998-04-14 01:04:21 -0700 (Tue, 14 Apr 1998) | 4 lines
 
 
 * combine.c (simplify_rtx, case TRUNCATE): Respect value of
 TRULY_NOOP_TRUNCATION.
 
 
 Index: combine.c
 ===
 --- combine.c   (revision 19018)
 +++ combine.c   (revision 19204)
 @@ -3736,7 +3736,9 @@ simplify_rtx (x, op0_mode, last, in_dest
if (GET_MODE_CLASS (mode) == MODE_PARTIAL_INT)
 break;
  
 -  if (GET_MODE_BITSIZE (mode) = HOST_BITS_PER_WIDE_INT)
 +  if (GET_MODE_BITSIZE (mode) = HOST_BITS_PER_WIDE_INT
 +  TRULY_NOOP_TRUNCATION (GET_MODE_BITSIZE (mode),
 +   GET_MODE_BITSIZE (GET_MODE (XEXP 
  (x, 0)
 SUBST (XEXP (x, 0),
force_to_mode (XEXP (x, 0), GET_MODE (XEXP (x, 0)),
   GET_MODE_MASK (mode), NULL_RTX, 0));
 
  This optimization simplifies the input to a truncate by only computing bits
  that won't be eliminated by the truncation.  Normally these are the bits in
  the output mode mask.  Note that the optimization does not change the 
  truncate
  into a low-part subreg, which would pretty automatically warrant the
  TRULY_NOOP_TRUNCATION check.
  
 
  I agree that this patch looks wrong in todays compiler.  There should be
  no need to call TRULY_NOOP_TRUNCATION if you are in a TRUNCATE anyhow.

 Based on reviewing my old notes, we do have to ensure that combine 
 doesn't replace a TRUNCATE with a SUBREG as that can result in having a 
 32bit value that isn't sign-extended, which clearly causes MIPS64 ports 
 grief.

Thanks for the long information in your other reply.

As I said in the original email, we are not turning a TRUNCATE into a SUBREG
in this transformation.  We're just simplifying the input expression to
truncate with the knowledge that only the truncated mode bits are relevant
from the input.  At the end we are still left with a truncate expression but
possibly with a simpler operand.

Adam


Re: Rationale for an old TRUNCATE patch

2009-06-17 Thread Adam Nemet
Ian Lance Taylor writes:
 truncate has a machine independent meaning.

Yes, I guess with your definition below it does.  It's interesting though that
Jim had said the opposite in the excerpts posted by Jeff:

  And a later message from Jim:
  
  Truncate converts a value from a larger to a smaller mode in a machine
  dependent way.
  
  A subreg just chops off the high bits.  A truncate does not.  The name might
  be a little confusing, but the whole point of truncate is to have something
  that operates differently than a subreg.
  
  Combine is clearly making an invalid transformation.
 
 Yes.  The bits in Nmode's mask are determined by the truncate.  The
 other bits are don't-care.  If the result of the truncate happens to
 wind up in a register, then in some cases PROMOTE_MODE will apply.

And IIUC this don't-care nature of the other bits that allows backends to
define the upper bits.  For example to have sign-bit copies there in registers
to enforce the MIPS64 SI mode representation.  And treating the don't care
bits outside SI mode in this way is true for any other SI-mode operations
performed on registers not just truncate, right?  Hmm, nice.

Adam


Re: Rationale for an old TRUNCATE patch

2009-06-17 Thread Ian Lance Taylor
Adam Nemet ane...@caviumnetworks.com writes:

 Ian Lance Taylor writes:
 truncate has a machine independent meaning.

 Yes, I guess with your definition below it does.  It's interesting though that
 Jim had said the opposite in the excerpts posted by Jeff:

   And a later message from Jim:
   
   Truncate converts a value from a larger to a smaller mode in a machine
   dependent way.
   
   A subreg just chops off the high bits.  A truncate does not.  The name might
   be a little confusing, but the whole point of truncate is to have something
   that operates differently than a subreg.
   
   Combine is clearly making an invalid transformation.

I'm not entirely sure, but I don't think Jim said the opposite.  He said
that the way truncate works is machine dependent.  I said that the
output of truncate is machine independent.  Since truncate is only
defined for fixed-point modes, I think both statements are true.

Ian


Re: Rationale for an old TRUNCATE patch

2009-06-17 Thread Jeff Law

Adam Nemet wrote:

Jeff Law writes:
  

Ian Lance Taylor wrote:


Adam Nemet ane...@caviumnetworks.com writes:

  
  

I am trying to understand the checkin by Jeff Law from about 11 years ago:

   r19204 | law | 1998-04-14 01:04:21 -0700 (Tue, 14 Apr 1998) | 4 lines
   
   
   * combine.c (simplify_rtx, case TRUNCATE): Respect value of

   TRULY_NOOP_TRUNCATION.
   
   
   Index: combine.c

   ===
   --- combine.c(revision 19018)
   +++ combine.c(revision 19204)
   @@ -3736,7 +3736,9 @@ simplify_rtx (x, op0_mode, last, in_dest
  if (GET_MODE_CLASS (mode) == MODE_PARTIAL_INT)
break;

   -  if (GET_MODE_BITSIZE (mode) = HOST_BITS_PER_WIDE_INT)

   +  if (GET_MODE_BITSIZE (mode) = HOST_BITS_PER_WIDE_INT
   +   TRULY_NOOP_TRUNCATION (GET_MODE_BITSIZE (mode),
   +GET_MODE_BITSIZE (GET_MODE (XEXP (x, 0)
SUBST (XEXP (x, 0),
   force_to_mode (XEXP (x, 0), GET_MODE (XEXP (x, 0)),
  GET_MODE_MASK (mode), NULL_RTX, 0));

This optimization simplifies the input to a truncate by only computing bits
that won't be eliminated by the truncation.  Normally these are the bits in
the output mode mask.  Note that the optimization does not change the truncate
into a low-part subreg, which would pretty automatically warrant the
TRULY_NOOP_TRUNCATION check.



I agree that this patch looks wrong in todays compiler.  There should be
no need to call TRULY_NOOP_TRUNCATION if you are in a TRUNCATE anyhow.
  
  
Based on reviewing my old notes, we do have to ensure that combine 
doesn't replace a TRUNCATE with a SUBREG as that can result in having a 
32bit value that isn't sign-extended, which clearly causes MIPS64 ports 
grief.



Thanks for the long information in your other reply.

As I said in the original email, we are not turning a TRUNCATE into a SUBREG
in this transformation.  We're just simplifying the input expression to
truncate with the knowledge that only the truncated mode bits are relevant
from the input.  At the end we are still left with a truncate expression but
possibly with a simpler operand.
  

Agreed.

I think it's a distinct possibility that the patch in question just 
papered over the problem and the Oct 1998 patch actually fixed the problem.


I've got no objections to removing the code you're referring to.


Jeff



Adam
  




Re: Rationale for an old TRUNCATE patch

2009-06-17 Thread Adam Nemet
Ian Lance Taylor writes:
 I'm not entirely sure, but I don't think Jim said the opposite.  He said
 that the way truncate works is machine dependent.  I said that the
 output of truncate is machine independent.  Since truncate is only
 defined for fixed-point modes, I think both statements are true.

OK but in that way every operation is machine dependent not just truncate.
BTW, why is being fixed-point relevant here?

From that little excerpt I just gathered that maybe my misunderstanding of
treating truncate as a blackbox was not completely without precedence.  But
anyway I think I understand now.  JTBS, can you agree with other statement in
my email?:

 And IIUC this don't-care nature of the other bits that allows backends to
 define the upper bits.  For example to have sign-bit copies there in registers
 to enforce the MIPS64 SI mode representation.  And treating the don't care
 bits outside SI mode in this way is true for any other SI-mode operations
 performed on registers not just truncate, right?  Hmm, nice.

Thanks for all the explanations.

Adam


Re: Rationale for an old TRUNCATE patch

2009-06-17 Thread Ian Lance Taylor
Adam Nemet ane...@caviumnetworks.com writes:

 Ian Lance Taylor writes:
 I'm not entirely sure, but I don't think Jim said the opposite.  He said
 that the way truncate works is machine dependent.  I said that the
 output of truncate is machine independent.  Since truncate is only
 defined for fixed-point modes, I think both statements are true.

 OK but in that way every operation is machine dependent not just truncate.

Yes.

 BTW, why is being fixed-point relevant here?

Because truncating from DFmode to SFmode really is machine dependent,
since it depends on the floating point representation being used.  We
use float_truncate for that.

Ian


Re: Rationale for an old TRUNCATE patch

2009-06-16 Thread Ian Lance Taylor
Adam Nemet ane...@caviumnetworks.com writes:

 I am trying to understand the checkin by Jeff Law from about 11 years ago:

r19204 | law | 1998-04-14 01:04:21 -0700 (Tue, 14 Apr 1998) | 4 lines


* combine.c (simplify_rtx, case TRUNCATE): Respect value of
TRULY_NOOP_TRUNCATION.


Index: combine.c
===
--- combine.c  (revision 19018)
+++ combine.c  (revision 19204)
@@ -3736,7 +3736,9 @@ simplify_rtx (x, op0_mode, last, in_dest
   if (GET_MODE_CLASS (mode) == MODE_PARTIAL_INT)
   break;
 
-  if (GET_MODE_BITSIZE (mode) = HOST_BITS_PER_WIDE_INT)
+  if (GET_MODE_BITSIZE (mode) = HOST_BITS_PER_WIDE_INT
+ TRULY_NOOP_TRUNCATION (GET_MODE_BITSIZE (mode),
+  GET_MODE_BITSIZE (GET_MODE (XEXP (x, 0)
   SUBST (XEXP (x, 0),
  force_to_mode (XEXP (x, 0), GET_MODE (XEXP (x, 0)),
 GET_MODE_MASK (mode), NULL_RTX, 0));

 This optimization simplifies the input to a truncate by only computing bits
 that won't be eliminated by the truncation.  Normally these are the bits in
 the output mode mask.  Note that the optimization does not change the truncate
 into a low-part subreg, which would pretty automatically warrant the
 TRULY_NOOP_TRUNCATION check.

I agree that this patch looks wrong in todays compiler.  There should be
no need to call TRULY_NOOP_TRUNCATION if you are in a TRUNCATE anyhow.


 Specifically I am curious if this was prompted by MIPS (or SH) or maybe some
 other target that have a notion of truncate different from MIPS.

It pretty much has to have been MIPS or SH since I don't think any other
target back then defined TRULY_NOOP_TRUNCATION as anything other than 1.

Ian


Re: Rationale for an old TRUNCATE patch

2009-06-16 Thread Adam Nemet
Ian Lance Taylor writes:
 I agree that this patch looks wrong in todays compiler.  There should be
 no need to call TRULY_NOOP_TRUNCATION if you are in a TRUNCATE anyhow.

Thanks.

Do you think we can assume this for TRUNCATEs in general or only for MIPS-like
TRUNCATEs?

I can't think of why it would be useful to represent a mode so that bits
outside the mode mask don't either depent on bits inside the mask or are
don't-care bits.  IOW, can we assume that for any TRUNCATE from wider mode W
to narrower mode N the following holds:

  (truncate:N expr:W) == (truncate:N (and:W expr:W GET_MODE_MASK(Nmode)))

?  Where == is not necessarily identical bit representation of the object
holding the value (e.g. QI HI values in MIPS) but that they are
indistinguishable in the operations that are defined on them.

Adam


Re: Rationale for an old TRUNCATE patch

2009-06-16 Thread Ian Lance Taylor
Adam Nemet ane...@caviumnetworks.com writes:

 Ian Lance Taylor writes:
 I agree that this patch looks wrong in todays compiler.  There should be
 no need to call TRULY_NOOP_TRUNCATION if you are in a TRUNCATE anyhow.

 Thanks.

 Do you think we can assume this for TRUNCATEs in general or only for MIPS-like
 TRUNCATEs?

truncate has a machine independent meaning.

 I can't think of why it would be useful to represent a mode so that bits
 outside the mode mask don't either depent on bits inside the mask or are
 don't-care bits.  IOW, can we assume that for any TRUNCATE from wider mode W
 to narrower mode N the following holds:

   (truncate:N expr:W) == (truncate:N (and:W expr:W GET_MODE_MASK(Nmode)))

 ?  Where == is not necessarily identical bit representation of the object
 holding the value (e.g. QI HI values in MIPS) but that they are
 indistinguishable in the operations that are defined on them.

Yes.  The bits in Nmode's mask are determined by the truncate.  The
other bits are don't-care.  If the result of the truncate happens to
wind up in a register, then in some cases PROMOTE_MODE will apply.

Ian


Re: Rationale for an old TRUNCATE patch

2009-06-16 Thread Jeff Law

Adam Nemet wrote:

I am trying to understand the checkin by Jeff Law from about 11 years ago:

   r19204 | law | 1998-04-14 01:04:21 -0700 (Tue, 14 Apr 1998) | 4 lines
   
   
   * combine.c (simplify_rtx, case TRUNCATE): Respect value of

   TRULY_NOOP_TRUNCATION.
   
   
   Index: combine.c

   ===
   --- combine.c(revision 19018)
   +++ combine.c(revision 19204)
   @@ -3736,7 +3736,9 @@ simplify_rtx (x, op0_mode, last, in_dest
  if (GET_MODE_CLASS (mode) == MODE_PARTIAL_INT)
break;

   -  if (GET_MODE_BITSIZE (mode) = HOST_BITS_PER_WIDE_INT)

   +  if (GET_MODE_BITSIZE (mode) = HOST_BITS_PER_WIDE_INT
   +   TRULY_NOOP_TRUNCATION (GET_MODE_BITSIZE (mode),
   +GET_MODE_BITSIZE (GET_MODE (XEXP (x, 0)
SUBST (XEXP (x, 0),
   force_to_mode (XEXP (x, 0), GET_MODE (XEXP (x, 0)),
  GET_MODE_MASK (mode), NULL_RTX, 0));

This optimization simplifies the input to a truncate by only computing bits
that won't be eliminated by the truncation.  Normally these are the bits in
the output mode mask.  Note that the optimization does not change the truncate
into a low-part subreg, which would pretty automatically warrant the
TRULY_NOOP_TRUNCATION check.

Specifically I am curious if this was prompted by MIPS (or SH) or maybe some
other target that have a notion of truncate different from MIPS.

force_to_mode was broken WRT truncate before my patch in 2006:

  http://gcc.gnu.org/ml/gcc-patches/2006-01/msg00553.html

so maybe the Jeff's patch was just trying to avoid the call to force_to_mode.

Anyhow, I don't think that the above transformation is illegal for MIPS64.

When truncating to SI mode the bits outside the resulting SI mode will all be
copies of the SI mode sign bit so their input values are obviously irrelevant.

For QI and HI modes the representation requires the upper 32 bits in the
64-bit word to be copies of the 31st bit.  The transformation might change the
31 bits so it can affect the the resulting bits in the 64-bit word.  This
however should have no visible effect.  Operations on QI- and HI-mode values
are performed in SI mode with proper extensions before the operation if bits
outside the mode would affect the result.

I also tried a few things after removing the above check.  Boostrapping and
regtesting on mips64octeon-linux was successful so was regtesting on
mipsisa64r2-elf.  The assembly output of gcc.c-torture/execute/*.c improved
as:

  13 files changed, 73 insertions(+), 99 deletions(-)

The changes typically remove zero and sign-extensions before a subsequent
truncation.

Any further information on this subject would be very much appreciated.
  
I can't be 100% sure, but I believe this was installed to deal with 
problems related to a 64bit mips chip. 

Assuming I've found the right thread in my personal mail archives (I've 
extracted the most relevant parts of the conversation)



I wrote (in 1998):

Imagine a 64bit mips part (like that's hard :-)

Integer comparisons in the hardware are always done with 64bits
of precision.


Consider this code:

(insn 21 18 22 (set (reg:DI 86)
   (ashiftrt:DI (reg/v:DI 83)
   (const_int 32))) 222 {ashrdi3_internal4} (insn_list 15 (nil))
   (nil))

(insn 22 21 24 (set (reg:SI 87)
   (truncate:SI (reg:DI 86))) 112 {truncdisi2} (insn_list 21 (nil))
   (expr_list:REG_DEAD (reg:DI 86)
   (nil)))

(jump_insn 24 22 26 (set (pc)
   (if_then_else (ge:SI (reg:SI 87)
   (const_int 0))
   (label_ref 36)
   (pc))) 241 {branch_zero} (insn_list 22 (nil))
   (expr_list:REG_DEAD (reg:SI 87)

Combine will turn that into (and I believe this is a valid 
tranformation): [ we now know this was invalid... ]



(insn 22 21 24 (set (subreg:DI (reg:SI 87) 0)
   (lshiftrt:DI (reg/v:DI 83)
   (const_int 32))) 232 {lshrdi3_internal4} (insn_list 15 (nil))
   (nil))

(jump_insn 24 22 26 (set (pc)
   (if_then_else (ge:SI (reg:SI 87)
   (const_int 0))
   (label_ref 36)
   (pc))) 241 {branch_zero} (insn_list 22 (nil))
   (expr_list:REG_DEAD (reg:SI 87)
   (nil)))

   (nil)))
;; End of basic block 0


Note that insn 22 uses a logical shift -- the net result will
be that the value placed into reg87 is no longer sign extended
from 32 to 64 bits.

This causes serious grief when we try to perform a signed comparison
with zero.  I suspect it would cause major headaches for arithmetic
operations too.


Ian wrote (in 1998):

  For the MIPS port, it is not valid to examine a reg:DI in reg:SI mode
  without using truncate.  I don't know why that is happening.  Perhaps
  something is failing to check TRULY_NOOP_TRUNCATION, which I believe
  is intended to prohibit this type of operation.

And a later message from Jim:

Truncate converts a value from a larger to a smaller mode in a machine

Re: Rationale for an old TRUNCATE patch

2009-06-16 Thread Jeff Law

Ian Lance Taylor wrote:

Adam Nemet ane...@caviumnetworks.com writes:

  

I am trying to understand the checkin by Jeff Law from about 11 years ago:

   r19204 | law | 1998-04-14 01:04:21 -0700 (Tue, 14 Apr 1998) | 4 lines
   
   
   * combine.c (simplify_rtx, case TRUNCATE): Respect value of

   TRULY_NOOP_TRUNCATION.
   
   
   Index: combine.c

   ===
   --- combine.c(revision 19018)
   +++ combine.c(revision 19204)
   @@ -3736,7 +3736,9 @@ simplify_rtx (x, op0_mode, last, in_dest
  if (GET_MODE_CLASS (mode) == MODE_PARTIAL_INT)
break;

   -  if (GET_MODE_BITSIZE (mode) = HOST_BITS_PER_WIDE_INT)

   +  if (GET_MODE_BITSIZE (mode) = HOST_BITS_PER_WIDE_INT
   +   TRULY_NOOP_TRUNCATION (GET_MODE_BITSIZE (mode),
   +GET_MODE_BITSIZE (GET_MODE (XEXP (x, 0)
SUBST (XEXP (x, 0),
   force_to_mode (XEXP (x, 0), GET_MODE (XEXP (x, 0)),
  GET_MODE_MASK (mode), NULL_RTX, 0));

This optimization simplifies the input to a truncate by only computing bits
that won't be eliminated by the truncation.  Normally these are the bits in
the output mode mask.  Note that the optimization does not change the truncate
into a low-part subreg, which would pretty automatically warrant the
TRULY_NOOP_TRUNCATION check.



I agree that this patch looks wrong in todays compiler.  There should be
no need to call TRULY_NOOP_TRUNCATION if you are in a TRUNCATE anyhow.
  
Based on reviewing my old notes, we do have to ensure that combine 
doesn't replace a TRUNCATE with a SUBREG as that can result in having a 
32bit value that isn't sign-extended, which clearly causes MIPS64 ports 
grief.


Jeff