620561 - nanojit: rename opcodes that specifically involve signed integers

Status: RESOLVED WONTFIX

(Core Graveyard :: Nanojit, defect)

Description

[Nicholas Nethercote [inactive]]

Currently we only distinguish signed and unsigned ints in a few places:  ui2d, rshui, lduc2ui, ldus2ui, and ltui/gtui/leui/geui.  These are the operations where signed and unsigned ints give different results;  other operations like immi, addi give the same results (ie. the same bit pattern).

In TM we just hit bug 620532, where we were bogusly comparing a signed value with an unsigned value using a signed comparison.  This got me thinking about adding unsigned variants of all the undistinguished operations.  This would increase type safety, but would be a lot more opcodes.  Most of the extra code would probably just be extra fall-through case labels in switches, eg:

   case LIR_addi:
   case LIR_addui:

We'd probably have to do likewise with quad operations.

We could do a half-assed approximation of this checking without adding the extra opcodes... eg. detect obviously wrong things like this:

  a = ui2d x
  b = i2d y
  c = lti x, y

But it wouldn't be as good.

Maybe I'll try prototyping this and see if the extra effort seems too painful.  We might be add the unsigned variants lazily;  I imagine only a small selection of the integer ops are used in an unsigned fashion currently by TM and TR.

Comment 1

[Nicholas Nethercote [inactive]]

Ugh, it doesn't take long for this to become painful.

I wonder if renaming the opcodes that involve signed integers would be helpful.  I think the full set of renamings would be this:

- {lt,gt,le,gei}i --> {lt,gt,le,ge}si    (mirrors {lt,gt,le,ge}ui)
- {lt,gt,le,gei}q --> {lt,gt,le,ge}sq    (mirrors {lt,gt,le,ge}uq)
- i2q --> si2sq      (mirrors ui2uq)
- rshi --> rshsi     (mirrors rshui)
- rshq --> rshsq     (mirrors rshuq)
- lds2i --> ldss2si  (mirrors lsus2ui)
- ldc2i --> ldsc2si  (mirrors lduc2ui)
- i2q -> si2sq       (mirrors ui2uq)
- i2d -> si2d        (mirrors ui2d)
- d2i -> d2si (is this one necessary? if d2ui existed would it give a different 
               result?)