public inbox for gcc@gcc.gnu.org
 help / color / mirror / Atom feed
* Rationale for an old TRUNCATE patch
@ 2009-06-16  7:12 Adam Nemet
  2009-06-16 14:35 ` Ian Lance Taylor
  2009-06-17  2:10 ` Jeff Law
  0 siblings, 2 replies; 12+ messages in thread
From: Adam Nemet @ 2009-06-16  7:12 UTC (permalink / raw)
  To: gcc; +Cc: law

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.

Adam

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Rationale for an old TRUNCATE patch
  2009-06-16  7:12 Rationale for an old TRUNCATE patch Adam Nemet
@ 2009-06-16 14:35 ` Ian Lance Taylor
  2009-06-16 22:45   ` Adam Nemet
  2009-06-17  2:12   ` Jeff Law
  2009-06-17  2:10 ` Jeff Law
  1 sibling, 2 replies; 12+ messages in thread
From: Ian Lance Taylor @ 2009-06-16 14:35 UTC (permalink / raw)
  To: Adam Nemet; +Cc: gcc, law

Adam Nemet <anemet@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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Rationale for an old TRUNCATE patch
  2009-06-16 14:35 ` Ian Lance Taylor
@ 2009-06-16 22:45   ` Adam Nemet
  2009-06-17  0:28     ` Ian Lance Taylor
  2009-06-17  2:12   ` Jeff Law
  1 sibling, 1 reply; 12+ messages in thread
From: Adam Nemet @ 2009-06-16 22:45 UTC (permalink / raw)
  To: Ian Lance Taylor; +Cc: gcc, law

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Rationale for an old TRUNCATE patch
  2009-06-16 22:45   ` Adam Nemet
@ 2009-06-17  0:28     ` Ian Lance Taylor
  2009-06-17  6:42       ` Adam Nemet
  0 siblings, 1 reply; 12+ messages in thread
From: Ian Lance Taylor @ 2009-06-17  0:28 UTC (permalink / raw)
  To: Adam Nemet; +Cc: gcc, law

Adam Nemet <anemet@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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Rationale for an old TRUNCATE patch
  2009-06-16  7:12 Rationale for an old TRUNCATE patch Adam Nemet
  2009-06-16 14:35 ` Ian Lance Taylor
@ 2009-06-17  2:10 ` Jeff Law
  1 sibling, 0 replies; 12+ messages in thread
From: Jeff Law @ 2009-06-17  2:10 UTC (permalink / raw)
  To: Adam Nemet; +Cc: gcc

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
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.





[ back to the present ]

I think we finally agreed that combine was incorrectly changing the 
TRUNCATE (which the MIPS backend ensured always resulted in 32bit sign 
extended value) into a SUBREG (which was a 32bit non-sign-extended value).

Unfortunately, I can't find the message where I posted the patch to the 
egcs list for review.  I did find a later patch on Oct 5, 1998 which 
might be related, but I don't see any discussion.  It's also terribly 
unfortunate that I don't see testcases for either -- however, that would 
lead me to believe both patches were fixing a problem in the existing 
GCC testsuite at the time that were triggering on the MIPS part in question.

The later patch was:
r22835 | law | 1998-10-05 03:05:58 -0600 (Mon, 05 Oct 1998) | 4 lines


        * combine.c (simplify_rtx): Do not replace TRUNCATE with a SUBREG if
        truncation is not a no-op.



I wasn't trying to avoid the call to force_to_mode, but instead trying 
to avoid combine turning a TRUNCATE into a SUBREG.

jeff






^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Rationale for an old TRUNCATE patch
  2009-06-16 14:35 ` Ian Lance Taylor
  2009-06-16 22:45   ` Adam Nemet
@ 2009-06-17  2:12   ` Jeff Law
  2009-06-17  6:17     ` Adam Nemet
  1 sibling, 1 reply; 12+ messages in thread
From: Jeff Law @ 2009-06-17  2:12 UTC (permalink / raw)
  To: Ian Lance Taylor; +Cc: Adam Nemet, gcc

Ian Lance Taylor wrote:
> Adam Nemet <anemet@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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Rationale for an old TRUNCATE patch
  2009-06-17  2:12   ` Jeff Law
@ 2009-06-17  6:17     ` Adam Nemet
  2009-06-17 14:52       ` Jeff Law
  0 siblings, 1 reply; 12+ messages in thread
From: Adam Nemet @ 2009-06-17  6:17 UTC (permalink / raw)
  To: Jeff Law; +Cc: Ian Lance Taylor, gcc

Jeff Law writes:
> Ian Lance Taylor wrote:
> > Adam Nemet <anemet@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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Rationale for an old TRUNCATE patch
  2009-06-17  0:28     ` Ian Lance Taylor
@ 2009-06-17  6:42       ` Adam Nemet
  2009-06-17 14:24         ` Ian Lance Taylor
  0 siblings, 1 reply; 12+ messages in thread
From: Adam Nemet @ 2009-06-17  6:42 UTC (permalink / raw)
  To: Ian Lance Taylor; +Cc: gcc, law

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Rationale for an old TRUNCATE patch
  2009-06-17  6:42       ` Adam Nemet
@ 2009-06-17 14:24         ` Ian Lance Taylor
  2009-06-17 15:26           ` Adam Nemet
  0 siblings, 1 reply; 12+ messages in thread
From: Ian Lance Taylor @ 2009-06-17 14:24 UTC (permalink / raw)
  To: Adam Nemet; +Cc: gcc, law

Adam Nemet <anemet@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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Rationale for an old TRUNCATE patch
  2009-06-17  6:17     ` Adam Nemet
@ 2009-06-17 14:52       ` Jeff Law
  0 siblings, 0 replies; 12+ messages in thread
From: Jeff Law @ 2009-06-17 14:52 UTC (permalink / raw)
  To: Adam Nemet; +Cc: Ian Lance Taylor, gcc

Adam Nemet wrote:
> Jeff Law writes:
>   
>> Ian Lance Taylor wrote:
>>     
>>> Adam Nemet <anemet@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
>   

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Rationale for an old TRUNCATE patch
  2009-06-17 14:24         ` Ian Lance Taylor
@ 2009-06-17 15:26           ` Adam Nemet
  2009-06-17 15:54             ` Ian Lance Taylor
  0 siblings, 1 reply; 12+ messages in thread
From: Adam Nemet @ 2009-06-17 15:26 UTC (permalink / raw)
  To: Ian Lance Taylor; +Cc: gcc, law

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Rationale for an old TRUNCATE patch
  2009-06-17 15:26           ` Adam Nemet
@ 2009-06-17 15:54             ` Ian Lance Taylor
  0 siblings, 0 replies; 12+ messages in thread
From: Ian Lance Taylor @ 2009-06-17 15:54 UTC (permalink / raw)
  To: Adam Nemet; +Cc: gcc, law

Adam Nemet <anemet@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

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2009-06-17 15:54 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-06-16  7:12 Rationale for an old TRUNCATE patch Adam Nemet
2009-06-16 14:35 ` Ian Lance Taylor
2009-06-16 22:45   ` Adam Nemet
2009-06-17  0:28     ` Ian Lance Taylor
2009-06-17  6:42       ` Adam Nemet
2009-06-17 14:24         ` Ian Lance Taylor
2009-06-17 15:26           ` Adam Nemet
2009-06-17 15:54             ` Ian Lance Taylor
2009-06-17  2:12   ` Jeff Law
2009-06-17  6:17     ` Adam Nemet
2009-06-17 14:52       ` Jeff Law
2009-06-17  2:10 ` Jeff Law

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).