nanojit: formalize the bool type in LIR

RESOLVED WONTFIX

Status

RESOLVED WONTFIX
9 years ago
5 years ago

People

(Reporter: njn, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

9 years ago
This came up in bug 558814 comment 12.  Currently bools are treated mostly the same as ints, and in cases where the difference matters, checking is done in an ad hoc fashion (eg. ValidateWriter calls checkLInsIsACondOrConst() for conditions).

It might be better to formally distinguish bools and ints.  This would require adding some opcodes, at least:  LIR_true, LIR_false, and boolean AND and OR operations.  I'm not sure what the type-indicator suffix should be since 'b' is taken for byte-sized integers.  We might also need opcodes for converting between bools and ints.

One thing to watch out for -- we don't want codegen to be affected.  But our codegen for conditions that aren't immediately used in a jump/guard is pretty sucky, so that shouldn't be hard :)
(Reporter)

Comment 1

9 years ago
(In reply to comment #0)
> 
> I'm not sure what the type-indicator suffix should be since 'b' is
> taken for byte-sized integers.

My best idea so far is 't', short for "truth value".  Hence LIR_truet, LIR_falset, LIR_andt, LIR_ort.

Also, instead of LIR_truet/LIR_falset we could have LIR_immt, and have it take a 'bool' arg.  This would make it more like other immediates.

Comment 2

9 years ago
still brainstorming, right? i was thinking 'c' for condition value:

  LIR_c2l                   // same as cmov(cond,1,0)
  LIR_immc 1 or 0
  LIR_andc, orc, xorc, notc // if we need them

compared to what we have now, we'd only need to add LIR_c2l for the case when a conditional is used in an int value context.  if its rare enough, cmov might even be fine, backends could recognize it as an idiom for (eg) SETcc, and would fold the comparison instruction like we already do in asm_branch().

LIR_immc(1|0) would be a nice improvement to bug 558814.

Comment 3

9 years ago
Another bonus from adding boolean to the type system:  now we can call functions returning C++ 'bool', which on x86 will be in al, not eax, at least on MSVC.  (ie, the upper 24 bits are garbage).  ARGTYPE_C (or T) and LIR_callc (or callt, etc) come into play. 

pure speculation:
I dont know of an existing ABI that lets a function return actual condition codes, but we've mused about it somewhat for low level helpers.  it would fit this new typesystem and would only need a new ABI constant.  branching on the result of a function like that wouldn't need any extra ALU ops.
(Reporter)

Comment 4

9 years ago
Another good thing about this:  it'll make LIR more compact.  This kind of thing is common in TM at least:

  y = andl x, k
  0 = imml 0
  z = eql y, 0
  xf z

Where the 'eq y, 0' converts a bool-like integer into a bool (and also flips the condition, hence the 'xf' instead of 'xt').

This will become:

  y = andl x, k
  z = l2c y
  xt z

First, there's no need for the 'imml 0'... often that would be removed by CseFilter, but not always, plus it reduces stress on CseFilter anyway (plus the pipeline stages preceding CseFilter).

Second, 'l2c' is a unary operand, in contrast to 'eql' being a binary operand, so that'll save a word.

Also, the new version is more readable.

Another thing I realized:  'immt' will be very rare in practice, there's no reason you'd use it deliberately from a LIR generator, it'll only turn up due to constant folding.
(Reporter)

Updated

9 years ago
Assignee: nobody → nnethercote
Status: NEW → ASSIGNED
(Reporter)

Updated

8 years ago
Depends on: 573416
(Reporter)

Comment 5

8 years ago
Thanks to the Great Opcode Renaming, 'b' is now available as the suffix for the bool type.
(Reporter)

Updated

7 years ago
Assignee: nnethercote → nobody
Status: ASSIGNED → NEW
(Assignee)

Updated

5 years ago
Component: Nanojit → Nanojit
Product: Core → Core Graveyard
(Reporter)

Comment 6

5 years ago
Nanojit has been dead for several years. Its Bugzilla component has been moved to the graveyard (bug 984276).

I checked all the open bugs. They're all uninteresting, so I'm WONTFIXing them all. Apologies for the bugspam.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.