[patch][rfa] Add ltu,leu,gtu,geu support for unsigned modes

[patch][rfa] Add ltu,leu,gtu,geu support for unsigned modes

[Dave Brolley]

[-- Attachment #1: Type: text/plain, Size: 254 bytes --]

Hi,

The attached patch adds support for the unsigned modes (UQI, UHI, USI, 
UDI, UINT) for the cgen rtl ops ltu, leu, gtu and geu. These calls can 
be generated by code like

(if (ltu (zext UDI op1) (zext UDI op2 ))(set op3 op1 ))

ok to commit?

Dave


[-- Attachment #2: compu.patch.txt --]
[-- Type: text/plain, Size: 2725 bytes --]

Index: sid/component/cgen-cpu/cgen-ops.h
===================================================================
RCS file: /cvs/src/src/sid/component/cgen-cpu/cgen-ops.h,v
retrieving revision 1.9
diff -c -p -r1.9 cgen-ops.h
*** sid/component/cgen-cpu/cgen-ops.h	1 Feb 2002 01:19:43 -0000	1.9
--- sid/component/cgen-cpu/cgen-ops.h	6 May 2003 19:27:42 -0000
*************** namespace cgen {
*** 55,60 ****
--- 55,64 ----
  #define LEUQI(x, y) ((UQI) (x) <= (UQI) (y))
  #define GTUQI(x, y) ((UQI) (x) > (UQI) (y))
  #define GEUQI(x, y) ((UQI) (x) >= (UQI) (y))
+ #define LTUUQI(x, y) ((UQI) (x) < (UQI) (y))
+ #define LEUUQI(x, y) ((UQI) (x) <= (UQI) (y))
+ #define GTUUQI(x, y) ((UQI) (x) > (UQI) (y))
+ #define GEUUQI(x, y) ((UQI) (x) >= (UQI) (y))
  #define ABSQI(x)  ((x) > 0 ? (x) : -(x))
  
  #define ADDHI(x, y) ((x) + (y))
*************** namespace cgen {
*** 83,88 ****
--- 87,96 ----
  #define LEUHI(x, y) ((UHI) (x) <= (UHI) (y))
  #define GTUHI(x, y) ((UHI) (x) > (UHI) (y))
  #define GEUHI(x, y) ((UHI) (x) >= (UHI) (y))
+ #define LTUUHI(x, y) ((UHI) (x) < (UHI) (y))
+ #define LEUUHI(x, y) ((UHI) (x) <= (UHI) (y))
+ #define GTUUHI(x, y) ((UHI) (x) > (UHI) (y))
+ #define GEUUHI(x, y) ((UHI) (x) >= (UHI) (y))
  #define ABSHI(x)  ((x) > 0 ? (x) : -(x))
  
  #define ADDSI(x, y) ((x) + (y))
*************** namespace cgen {
*** 111,116 ****
--- 119,128 ----
  #define LEUSI(x, y) ((USI) (x) <= (USI) (y))
  #define GTUSI(x, y) ((USI) (x) > (USI) (y))
  #define GEUSI(x, y) ((USI) (x) >= (USI) (y))
+ #define LTUUSI(x, y) ((USI) (x) < (USI) (y))
+ #define LEUUSI(x, y) ((USI) (x) <= (USI) (y))
+ #define GTUUSI(x, y) ((USI) (x) > (USI) (y))
+ #define GEUUSI(x, y) ((USI) (x) >= (USI) (y))
  #define ABSSI(x)  ((x) > 0 ? (x) : -(x))
  
  #define ADDDI(x, y) ((x) + (y))
*************** namespace cgen {
*** 139,150 ****
--- 151,171 ----
  #define LEUDI(x, y) ((UDI) (x) <= (UDI) (y))
  #define GTUDI(x, y) ((UDI) (x) > (UDI) (y))
  #define GEUDI(x, y) ((UDI) (x) >= (UDI) (y))
+ #define LTUUDI(x, y) ((UDI) (x) < (UDI) (y))
+ #define LEUUDI(x, y) ((UDI) (x) <= (UDI) (y))
+ #define GTUUDI(x, y) ((UDI) (x) > (UDI) (y))
+ #define GEUUDI(x, y) ((UDI) (x) >= (UDI) (y))
  #define ABSDI(x)  ((x) > 0 ? (x) : -(x))
  
  #define LTUINT(x, y) ((UINT) (x) < (UINT) (y))
  #define LEUINT(x, y) ((UINT) (x) <= (UINT) (y))
  #define GTUINT(x, y) ((UINT) (x) > (UINT) (y))
  #define GEUINT(x, y) ((UINT) (x) >= (UINT) (y))
+ 
+ #define LTUUINT(x, y) ((UINT) (x) < (UINT) (y))
+ #define LEUUINT(x, y) ((UINT) (x) <= (UINT) (y))
+ #define GTUUINT(x, y) ((UINT) (x) > (UINT) (y))
+ #define GEUUINT(x, y) ((UINT) (x) >= (UINT) (y))
  
  // XXX: fill out families of functions
  #define RORQI(x, y) (SRLQI((x), (y)) | SLLQI((x), 8-(y)))

Re: [patch][rfa] Add ltu,leu,gtu,geu support for unsigned modes

[Dave Brolley]

Approved by fche and committed.

Dave Brolley wrote:

> Hi,
>
> The attached patch adds support for the unsigned modes (UQI, UHI, USI, 
> UDI, UINT) for the cgen rtl ops ltu, leu, gtu and geu. These calls can 
> be generated by code like
>
> (if (ltu (zext UDI op1) (zext UDI op2 ))(set op3 op1 ))
>
> ok to commit?
>
> Dave
>
>
>------------------------------------------------------------------------
>
>Index: sid/component/cgen-cpu/cgen-ops.h
>===================================================================
>RCS file: /cvs/src/src/sid/component/cgen-cpu/cgen-ops.h,v
>retrieving revision 1.9
>diff -c -p -r1.9 cgen-ops.h
>*** sid/component/cgen-cpu/cgen-ops.h	1 Feb 2002 01:19:43 -0000	1.9
>--- sid/component/cgen-cpu/cgen-ops.h	6 May 2003 19:27:42 -0000
>*************** namespace cgen {
>*** 55,60 ****
>--- 55,64 ----
>  #define LEUQI(x, y) ((UQI) (x) <= (UQI) (y))
>  #define GTUQI(x, y) ((UQI) (x) > (UQI) (y))
>  #define GEUQI(x, y) ((UQI) (x) >= (UQI) (y))
>+ #define LTUUQI(x, y) ((UQI) (x) < (UQI) (y))
>+ #define LEUUQI(x, y) ((UQI) (x) <= (UQI) (y))
>+ #define GTUUQI(x, y) ((UQI) (x) > (UQI) (y))
>+ #define GEUUQI(x, y) ((UQI) (x) >= (UQI) (y))
>  #define ABSQI(x)  ((x) > 0 ? (x) : -(x))
>  
>  #define ADDHI(x, y) ((x) + (y))
>*************** namespace cgen {
>*** 83,88 ****
>--- 87,96 ----
>  #define LEUHI(x, y) ((UHI) (x) <= (UHI) (y))
>  #define GTUHI(x, y) ((UHI) (x) > (UHI) (y))
>  #define GEUHI(x, y) ((UHI) (x) >= (UHI) (y))
>+ #define LTUUHI(x, y) ((UHI) (x) < (UHI) (y))
>+ #define LEUUHI(x, y) ((UHI) (x) <= (UHI) (y))
>+ #define GTUUHI(x, y) ((UHI) (x) > (UHI) (y))
>+ #define GEUUHI(x, y) ((UHI) (x) >= (UHI) (y))
>  #define ABSHI(x)  ((x) > 0 ? (x) : -(x))
>  
>  #define ADDSI(x, y) ((x) + (y))
>*************** namespace cgen {
>*** 111,116 ****
>--- 119,128 ----
>  #define LEUSI(x, y) ((USI) (x) <= (USI) (y))
>  #define GTUSI(x, y) ((USI) (x) > (USI) (y))
>  #define GEUSI(x, y) ((USI) (x) >= (USI) (y))
>+ #define LTUUSI(x, y) ((USI) (x) < (USI) (y))
>+ #define LEUUSI(x, y) ((USI) (x) <= (USI) (y))
>+ #define GTUUSI(x, y) ((USI) (x) > (USI) (y))
>+ #define GEUUSI(x, y) ((USI) (x) >= (USI) (y))
>  #define ABSSI(x)  ((x) > 0 ? (x) : -(x))
>  
>  #define ADDDI(x, y) ((x) + (y))
>*************** namespace cgen {
>*** 139,150 ****
>--- 151,171 ----
>  #define LEUDI(x, y) ((UDI) (x) <= (UDI) (y))
>  #define GTUDI(x, y) ((UDI) (x) > (UDI) (y))
>  #define GEUDI(x, y) ((UDI) (x) >= (UDI) (y))
>+ #define LTUUDI(x, y) ((UDI) (x) < (UDI) (y))
>+ #define LEUUDI(x, y) ((UDI) (x) <= (UDI) (y))
>+ #define GTUUDI(x, y) ((UDI) (x) > (UDI) (y))
>+ #define GEUUDI(x, y) ((UDI) (x) >= (UDI) (y))
>  #define ABSDI(x)  ((x) > 0 ? (x) : -(x))
>  
>  #define LTUINT(x, y) ((UINT) (x) < (UINT) (y))
>  #define LEUINT(x, y) ((UINT) (x) <= (UINT) (y))
>  #define GTUINT(x, y) ((UINT) (x) > (UINT) (y))
>  #define GEUINT(x, y) ((UINT) (x) >= (UINT) (y))
>+ 
>+ #define LTUUINT(x, y) ((UINT) (x) < (UINT) (y))
>+ #define LEUUINT(x, y) ((UINT) (x) <= (UINT) (y))
>+ #define GTUUINT(x, y) ((UINT) (x) > (UINT) (y))
>+ #define GEUUINT(x, y) ((UINT) (x) >= (UINT) (y))
>  
>  // XXX: fill out families of functions
>  #define RORQI(x, y) (SRLQI((x), (y)) | SLLQI((x), 8-(y)))
>

Re: [patch][rfa] Add ltu,leu,gtu,geu support for unsigned modes

[Ben Elliston]

Dave Brolley <brolley@redhat.com> writes:

> ok to commit?

Looks good to me.

Ben

[patch][rfa] Add ltu,leu,gtu,geu support for unsigned modes

[Doug Evans]

Dave Brolley writes:
 > Hi,
 > 
 > The attached patch adds support for the unsigned modes (UQI, UHI, USI, 
 > UDI, UINT) for the cgen rtl ops ltu, leu, gtu and geu. These calls can 
 > be generated by code like
 > 
 > (if (ltu (zext UDI op1) (zext UDI op2 ))(set op3 op1 ))
 > 
 > ok to commit?

Well ....

The first indication that you're barking up the wrong tree is that
cgen-ops.h generally doesn't have Umode support (e.g. ADDUSI).
There's a reason.  It's excessive and unnecessary and I don't want
to unnecessarily propagate them.  GCC doesn't have Umodes.
I added Umodes mostly as an escape hatch, but a controlled one.
If this patch goes in and the general direction it takes cgen-ops.h in
is allowed to proceed, I think that's a bad thing.

Do you get the desired result if you replace the above with

  (if (ltu (zext DI op1) (zext DI op2 ))(set op3 op1 ))

?

Note this: from doc/rtl.texi:

@item UQI,UHI,USI,UDI
Unsigned versions of QI,HI,SI,DI.

These modes do not appear in semantic RTL.  Instead, the RTL function
specifies the signedness of its operands where necessary.

??? I'm not entirely sure these unsigned modes are needed.
They are useful in removing any ambiguity in how to sign extend constants
which has been a source of problems in GCC.

??? Some existing ports use these modes.

Re: [patch][rfa] Add ltu,leu,gtu,geu support for unsigned modes

[Dave Brolley]

Thanks for your comments. Some questions and further comments below....

Doug Evans wrote:

>
>The first indication that you're barking up the wrong tree is that
>cgen-ops.h generally doesn't have Umode support (e.g. ADDUSI).
>
Yet types.scm does support them and cgen happily consumes them with the 
expected result. Also, just about every .cpu file in cgen/cpu contains 
an instance of the mode USI. That's why I was led to believe that this 
support was appropriate.

>
>There's a reason.  It's excessive and unnecessary and I don't want
>to unnecessarily propagate them.  GCC doesn't have Umodes.
>I added Umodes mostly as an escape hatch, but a controlled one.
>
Can you explain this? An escape hatch for what? And what limits do you 
have in mind? It may be excessive, but one could also argue that it's 
more complete to support Umodes.

>
>If this patch goes in and the general direction it takes cgen-ops.h in
>is allowed to proceed, I think that's a bad thing.
>
>Do you get the desired result if you replace the above with
>
>  (if (ltu (zext DI op1) (zext DI op2 ))(set op3 op1 ))
>
Yes, however this example is from machine generated code (a customer's 
tool) and the customer thought (and convinced me too) that the use of 
the Umode UDI is reasonable, since Umodes are frequently used and 
accepted in many instances. FWIW, by coincidence

(if (lt (zext UDI op1) (zext UDI op2 ))(set op3 op1 ))

works and it's hard to explain to the customer why ltu doesn't accept 
umodes while lt does.

>
>Note this: from doc/rtl.texi:
>
>@item UQI,UHI,USI,UDI
>Unsigned versions of QI,HI,SI,DI.
>
>These modes do not appear in semantic RTL.  Instead, the RTL function
>specifies the signedness of its operands where necessary.
>
>??? I'm not entirely sure these unsigned modes are needed.
>They are useful in removing any ambiguity in how to sign extend constants
>which has been a source of problems in GCC.
>
>??? Some existing ports use these modes.
>
Perhaps this can help me convince the customer.

Dave

Re: [patch][rfa] Add ltu,leu,gtu,geu support for unsigned modes

[Doug Evans]

Dave Brolley writes:
 > That's why I was led to believe that this 
 > support was appropriate.

I'm not faulting you for thinking that.
We need to reach a decision about Umodes and more rigidly enforce it.
Mea culpa.

I propose we set a direction of removing them entirely.

One catch, to me anyway, is that I much prefer thinking of
registers (esp. pc) as unsigned quantities.
Thus an additional step might be to define SI as unsigned
when it appears in C.  At the semantic level it is neither signed
nor unsigned, but C doesn't really have a way of understanding that,
and that might cause confusion.  Whether it'd be a more palatable
confusion than having both SI and USI I dunno.

 > It may be excessive, but one could also argue that it's 
 > more complete to support Umodes.

Depends on how you look at things.
To a cpu, a 32 bit register is a 32 bit register.
Ditto for when the 32 bit quantity lives in memory.
It's only in how it is subsequently used or interpreted that
signedness might come into play.
When signedness comes into play on the chip, it's explicitly
specified in the operation, _not_ in the data.
Ergo from this perspective Umodes don't belong in .cpu files.
And I think this is the perspective we must use (when writing
.cpu files anyway).

 > (if (lt (zext UDI op1) (zext UDI op2 ))(set op3 op1 ))
 > 
 > works and it's hard to explain to the customer why ltu doesn't accept 
 > umodes while lt does.

Well, it's easy to explain.  No claim is made that one's eyes won't roll.
Is LTUDI LT concatenated with UDI or LTU concatenated with DI?

 > >@item UQI,UHI,USI,UDI
 > >Unsigned versions of QI,HI,SI,DI.
 > >
 > >These modes do not appear in semantic RTL.  Instead, the RTL function
 > >specifies the signedness of its operands where necessary.
 > > [...]
 >
 > Perhaps this can help me convince the customer.

I think if you explain the above reasoning that will also help.

Re: [patch][rfa] Add ltu,leu,gtu,geu support for unsigned modes

[Dave Brolley]

[-- Attachment #1: Type: text/plain, Size: 161 bytes --]

We have explained this to our customer and convinced them to fix their 
tool. As a result I have committed the attached patch which reverts this 
change.

Dave


[-- Attachment #2: cgen-ops.patch.txt --]
[-- Type: text/plain, Size: 2755 bytes --]

Index: sid/component/cgen-cpu/cgen-ops.h
===================================================================
RCS file: /cvs/src/src/sid/component/cgen-cpu/cgen-ops.h,v
retrieving revision 1.10
retrieving revision 1.11
diff -c -p -r1.10 -r1.11
*** sid/component/cgen-cpu/cgen-ops.h	6 May 2003 21:52:53 -0000	1.10
--- sid/component/cgen-cpu/cgen-ops.h	13 Jun 2003 19:58:08 -0000	1.11
*************** namespace cgen {
*** 55,64 ****
  #define LEUQI(x, y) ((UQI) (x) <= (UQI) (y))
  #define GTUQI(x, y) ((UQI) (x) > (UQI) (y))
  #define GEUQI(x, y) ((UQI) (x) >= (UQI) (y))
- #define LTUUQI(x, y) ((UQI) (x) < (UQI) (y))
- #define LEUUQI(x, y) ((UQI) (x) <= (UQI) (y))
- #define GTUUQI(x, y) ((UQI) (x) > (UQI) (y))
- #define GEUUQI(x, y) ((UQI) (x) >= (UQI) (y))
  #define ABSQI(x)  ((x) > 0 ? (x) : -(x))
  
  #define ADDHI(x, y) ((x) + (y))
--- 55,60 ----
*************** namespace cgen {
*** 87,96 ****
  #define LEUHI(x, y) ((UHI) (x) <= (UHI) (y))
  #define GTUHI(x, y) ((UHI) (x) > (UHI) (y))
  #define GEUHI(x, y) ((UHI) (x) >= (UHI) (y))
- #define LTUUHI(x, y) ((UHI) (x) < (UHI) (y))
- #define LEUUHI(x, y) ((UHI) (x) <= (UHI) (y))
- #define GTUUHI(x, y) ((UHI) (x) > (UHI) (y))
- #define GEUUHI(x, y) ((UHI) (x) >= (UHI) (y))
  #define ABSHI(x)  ((x) > 0 ? (x) : -(x))
  
  #define ADDSI(x, y) ((x) + (y))
--- 83,88 ----
*************** namespace cgen {
*** 119,128 ****
  #define LEUSI(x, y) ((USI) (x) <= (USI) (y))
  #define GTUSI(x, y) ((USI) (x) > (USI) (y))
  #define GEUSI(x, y) ((USI) (x) >= (USI) (y))
- #define LTUUSI(x, y) ((USI) (x) < (USI) (y))
- #define LEUUSI(x, y) ((USI) (x) <= (USI) (y))
- #define GTUUSI(x, y) ((USI) (x) > (USI) (y))
- #define GEUUSI(x, y) ((USI) (x) >= (USI) (y))
  #define ABSSI(x)  ((x) > 0 ? (x) : -(x))
  
  #define ADDDI(x, y) ((x) + (y))
--- 111,116 ----
*************** namespace cgen {
*** 151,171 ****
  #define LEUDI(x, y) ((UDI) (x) <= (UDI) (y))
  #define GTUDI(x, y) ((UDI) (x) > (UDI) (y))
  #define GEUDI(x, y) ((UDI) (x) >= (UDI) (y))
- #define LTUUDI(x, y) ((UDI) (x) < (UDI) (y))
- #define LEUUDI(x, y) ((UDI) (x) <= (UDI) (y))
- #define GTUUDI(x, y) ((UDI) (x) > (UDI) (y))
- #define GEUUDI(x, y) ((UDI) (x) >= (UDI) (y))
  #define ABSDI(x)  ((x) > 0 ? (x) : -(x))
  
  #define LTUINT(x, y) ((UINT) (x) < (UINT) (y))
  #define LEUINT(x, y) ((UINT) (x) <= (UINT) (y))
  #define GTUINT(x, y) ((UINT) (x) > (UINT) (y))
  #define GEUINT(x, y) ((UINT) (x) >= (UINT) (y))
- 
- #define LTUUINT(x, y) ((UINT) (x) < (UINT) (y))
- #define LEUUINT(x, y) ((UINT) (x) <= (UINT) (y))
- #define GTUUINT(x, y) ((UINT) (x) > (UINT) (y))
- #define GEUUINT(x, y) ((UINT) (x) >= (UINT) (y))
  
  // XXX: fill out families of functions
  #define RORQI(x, y) (SRLQI((x), (y)) | SLLQI((x), 8-(y)))
--- 139,150 ----