Skip to content

Commit

Permalink
mir: add built-in float arithmetic ops (nim-works#1172)
Browse files Browse the repository at this point in the history
## Summary

Introduce built-in MIR operators for basic floating-point arithmetic.
Exceptional control-flow arising from float overflow checks
(`infChecks`) is now also visible to data-flow analysis and destructor
injection, fixing locals not being cleaned up when a scope is exited as
the result of a failed float overflow check and there's no other
unstructured scope exit.

## Details

### MIR

* the existing signed integer arithmetic operators (`mnkNegI`,
  `mnkAddI`, `mnkSubI`, etc.) are extended to also cover float
  operands. The "I" suffix is removed from the names
* the built-in arithmetic operators for floating-point operands are
  also without run-time checks

### AST-to-MIR translation

* if float overflow checks are enabled, the float arithmetic magic
  calls are kept as calls (a checked call is used if panics are
  disabled)
* if the checks are disabled, the float arithmetic magic calls are
  translated to the MIR's built-in operators
* `mUnaryMinusF64` is always translated to `mnkNeg`, since overflow is
  not possible for floating-point negation

### C code generator

* the built-in float arithmetic operation are translated in the same
  way that the originating-from magics were
* for translating the built-in arithmetic operations (`cnkNeg`,
  `cnkAdd`, etc.), the expression's type is used for picking the
  correct translation
* `binaryFloatArith`, which is only used for translating the magic
  calls, always emits float overflow checks

### JS code generator

* except for division, the code so far generated for the built-in
  arithmetic operators is also valid for floats
* overflow checks for floats aren't implemented for the JS backend at
  this time, so nothing changes for the magics

### VM code generator

* code generation for the built-in arithmetic operators pick the
  correct opcode based on the type
* emission of the `NarrowS` instructions (via `genNarrow`) is removed
  for the built-in arithmetic op code generation. They're unnecessary,
  and they'd also be invalid for float operands
* overflow checks for floats aren't implemented for the VM backend at
  this time, so nothing changes for the magics

There were also two issues with `cnkNeg` code generation that are now
fixed:
* the temporary register is freed properly (caused a minor register
  leak)
* the destination register is set up properly (could cause the VM to
  crash)

### Tests

* to act as a reminder that the float overflow check implementation
  is missing, the `float/tfloats1.nim` test is enabled for all
  targets. The expected output is changed such that it doesn't rely
  on backend-specific uncaught-exception rendering
* a test case for ensuring that locals are cleaned up when their scope
  is left via exceptional control-flow
  • Loading branch information
zerbina authored Feb 7, 2024
1 parent 6864e18 commit 34a9654
Show file tree
Hide file tree
Showing 11 changed files with 112 additions and 63 deletions.
16 changes: 7 additions & 9 deletions compiler/backend/ccgexprs.nim
Original file line number Diff line number Diff line change
Expand Up @@ -1558,7 +1558,7 @@ proc genStrEquals(p: BProc, e: CgNode, d: var TLoc) =
binaryExpr(p, e, d, "#eqStrings($1, $2)")

proc binaryFloatArith(p: BProc, e: CgNode, d: var TLoc, m: TMagic) =
if optInfCheck in p.options:
if true:
const opr: array[mAddF64..mDivF64, string] = ["+", "-", "*", "/"]
var a, b: TLoc
assert(e[1].typ != nil)
Expand All @@ -1569,8 +1569,6 @@ proc binaryFloatArith(p: BProc, e: CgNode, d: var TLoc, m: TMagic) =
[opr[m], rdLoc(a), rdLoc(b),
getSimpleTypeDesc(p.module, e[1].typ)]))
linefmt(p, cpsStmts, "if ($1 != 0.0 && $1*0.5 == $1) { #raiseFloatOverflow($1); $2}$n", [rdLoc(d), raiseInstr(p)])
else:
binaryArith(p, e, e[1], e[2], d, m)

proc skipAddr(n: CgNode): CgNode =
if n.kind == cnkHiddenAddr: n.operand
Expand Down Expand Up @@ -1648,7 +1646,7 @@ proc genBreakState(p: BProc, n: CgNode, d: var TLoc) =

proc genMagicExpr(p: BProc, e: CgNode, d: var TLoc, op: TMagic) =
case op
of mNot..mUnaryMinusF64: unaryArith(p, e, e[1], d, op)
of mNot..mUnaryPlusF64: unaryArith(p, e, e[1], d, op)
of mUnaryMinusI, mUnaryMinusI64: unaryArithOverflow(p, e, d, op)
of mAddF64..mDivF64: binaryFloatArith(p, e, d, op)
of mShrI..mXor: binaryArith(p, e, e[1], e[2], d, op)
Expand Down Expand Up @@ -2056,11 +2054,11 @@ proc expr(p: BProc, n: CgNode, d: var TLoc) =
else:
genCall(p, n, d)
# unchecked arithmetic operations:
of cnkNegI: unaryArith(p, n, n[0], d, mUnaryMinusI)
of cnkAddI: binaryArith(p, n, n[0], n[1], d, mAddI)
of cnkSubI: binaryArith(p, n, n[0], n[1], d, mSubI)
of cnkMulI: binaryArith(p, n, n[0], n[1], d, mMulI)
of cnkDivI: binaryArith(p, n, n[0], n[1], d, mDivI)
of cnkNeg: unaryArith(p, n, n[0], d, pick(n, mUnaryMinusI, mUnaryMinusF64))
of cnkAdd: binaryArith(p, n, n[0], n[1], d, pick(n, mAddI, mAddF64))
of cnkSub: binaryArith(p, n, n[0], n[1], d, pick(n, mSubI, mSubF64))
of cnkMul: binaryArith(p, n, n[0], n[1], d, pick(n, mMulI, mMulF64))
of cnkDiv: binaryArith(p, n, n[0], n[1], d, pick(n, mDivI, mDivF64))
of cnkModI: binaryArith(p, n, n[0], n[1], d, mModI)
of cnkSetConstr:
genSetConstr(p, n, d)
Expand Down
10 changes: 5 additions & 5 deletions compiler/backend/cgir.nim
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,11 @@ type
cnkCheckedCall ## like ``cnkCall``, but the call might raise an exception

# arithmetic operations:
cnkNegI
cnkAddI
cnkSubI
cnkMulI
cnkDivI
cnkNeg
cnkAdd
cnkSub
cnkMul
cnkDiv
cnkModI

# constructors:
Expand Down
6 changes: 3 additions & 3 deletions compiler/backend/cgirgen.nim
Original file line number Diff line number Diff line change
Expand Up @@ -802,12 +802,12 @@ proc exprToIr(tree: MirBody, cl: var TranslateCl,
of mnkCall, mnkCheckedCall:
callToIr(tree, cl, n, cr)
of UnaryOps:
const Map = [mnkNegI: cnkNegI]
const Map = [mnkNeg: cnkNeg]
treeOp Map[n.kind]:
res.add valueToIr(tree, cl, cr)
of BinaryOps:
const Map = [mnkAddI: cnkAddI, mnkSubI: cnkSubI,
mnkMulI: cnkMulI, mnkDivI: cnkDivI, mnkModI: cnkModI]
const Map = [mnkAdd: cnkAdd, mnkSub: cnkSub,
mnkMul: cnkMul, mnkDiv: cnkDiv, mnkModI: cnkModI]
treeOp Map[n.kind]:
res.kids = @[valueToIr(tree, cl, cr), valueToIr(tree, cl, cr)]
of AllNodeKinds - ExprKinds - {mnkNone}:
Expand Down
9 changes: 9 additions & 0 deletions compiler/backend/compat.nim
Original file line number Diff line number Diff line change
Expand Up @@ -211,3 +211,12 @@ proc translate*(t: MirTree): CgNode =

var i = 0
translateAux(t, i)

proc pick*[T](n: CgNode, forInt, forFloat: T): T =
## Returns either `forInt` or `forFloat` depending on the type of `n`.
case n.typ.skipTypes(abstractRange + tyUserTypeClasses + {tyEnum}).kind
of tyInt..tyInt64, tyBool: forInt
of tyUInt..tyUInt64, tyChar: forInt
of tyFloat..tyFloat64: forFloat
else:
unreachable("not an integer or float type")
24 changes: 13 additions & 11 deletions compiler/backend/jsgen.nim
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ proc getTemp(p: PProc, defineInLocals: bool = true): Rope =
p.defs.add(p.indentLine("var $1;$n" % [result]))

type
TMagicOps = array[mAddI..mUnaryMinusF64, string]
TMagicOps = array[mAddI..mUnaryPlusF64, string]

const # magic checked op
jsMagics: TMagicOps = [
Expand Down Expand Up @@ -479,8 +479,7 @@ const # magic checked op
mNot: "",
mUnaryPlusI: "",
mBitnotI: "",
mUnaryPlusF64: "",
mUnaryMinusF64: ""]
mUnaryPlusF64: ""]

template binaryExpr(p: PProc, n: CgNode, r: var TCompRes, magic, frmt: string) =
# $1 and $2 in the `frmt` string bind to lhs and rhs of the expr,
Expand Down Expand Up @@ -614,7 +613,6 @@ proc arithAux(p: PProc, n: CgNode, r: var TCompRes, op: TMagic) =
of mUnaryPlusI: applyFormat("+($1)")
of mBitnotI: applyFormat("~($1)")
of mUnaryPlusF64: applyFormat("+($1)")
of mUnaryMinusF64: applyFormat("-($1)")
else:
unreachable(op)

Expand Down Expand Up @@ -911,7 +909,7 @@ proc generateHeader(params: openArray[Loc]): string =
const
nodeKindsNeedNoCopy = cnkLiterals + {
cnkObjConstr, cnkTupleConstr, cnkArrayConstr, cnkCall, cnkCheckedCall,
cnkNegI, cnkAddI, cnkSubI, cnkMulI, cnkDivI, cnkModI }
cnkNeg, cnkAdd, cnkSub, cnkMul, cnkDiv, cnkModI }

proc needsNoCopy(p: PProc; y: CgNode): bool =
return y.kind in nodeKindsNeedNoCopy or
Expand Down Expand Up @@ -1739,7 +1737,7 @@ proc genRangeChck(p: PProc, n: CgNode, r: var TCompRes)
proc genMagic(p: PProc, n: CgNode, r: var TCompRes) =
let op = getCalleeMagic(p.g.env, n[0])
case op
of mAddI..mUnaryMinusI64, mNot..mUnaryMinusF64:
of mAddI..mUnaryMinusI64, mNot..mUnaryPlusF64:
arith(p, n, r, op)
of mCharToStr:
unaryExpr(p, n, r, "nimCharToStr", "nimCharToStr($1)")
Expand Down Expand Up @@ -2360,15 +2358,19 @@ proc gen(p: PProc, n: CgNode, r: var TCompRes) =
genInfixCall(p, n, r)
else:
genCall(p, n, r)
of cnkNegI:
of cnkNeg:
let x = gen(p, n[0])
r.res = "(-$1)" % rdLoc(x)
r.typ = mapType(n.typ)
r.kind = resExpr
of cnkAddI: binaryExpr(p, n[0], n[1], r, "($1 + $2)")
of cnkSubI: binaryExpr(p, n[0], n[1], r, "($1 - $2)")
of cnkMulI: binaryExpr(p, n[0], n[1], r, "($1 * $2)")
of cnkDivI: binaryExpr(p, n[0], n[1], r, "Math.trunc($1 / $2)")
of cnkAdd: binaryExpr(p, n[0], n[1], r, "($1 + $2)")
of cnkSub: binaryExpr(p, n[0], n[1], r, "($1 - $2)")
of cnkMul: binaryExpr(p, n[0], n[1], r, "($1 * $2)")
of cnkDiv:
if mapType(n.typ) == etyFloat:
binaryExpr(p, n[0], n[1], r, "($1 / $2)")
else:
binaryExpr(p, n[0], n[1], r, "Math.trunc($1 / $2)")
of cnkModI: binaryExpr(p, n[0], n[1], r, "Math.trunc($1 % $2)")
of cnkClosureConstr:
useMagic(p, "makeClosure")
Expand Down
27 changes: 19 additions & 8 deletions compiler/mir/mirgen.nim
Original file line number Diff line number Diff line change
Expand Up @@ -1213,9 +1213,9 @@ proc genMagic(c: var TCtx, n: PNode; m: TMagic) =
arg n[1]
arg n[2]
else:
const Map = [mAddI: mnkAddI, mSubI: mnkSubI,
mMulI: mnkMulI, mDivI: mnkDivI, mModI: mnkModI,
mSucc: mnkAddI, mPred: mnkSubI]
const Map = [mAddI: mnkAdd, mSubI: mnkSub,
mMulI: mnkMul, mDivI: mnkDiv, mModI: mnkModI,
mSucc: mnkAdd, mPred: mnkSub]
c.buildTree Map[m], n.typ:
genArgExpression(c, n[1], sink=false)
genArgExpression(c, n[2], sink=false)
Expand All @@ -1226,7 +1226,7 @@ proc genMagic(c: var TCtx, n: PNode; m: TMagic) =
c.buildDefectMagicCall m, n.typ:
arg n[1]
else:
c.genOp(mnkNegI, n.typ, n[1])
c.genOp(mnkNeg, n.typ, n[1])

of mInc, mDec:
# ``inc a, b`` -> ``a = a + b``
Expand All @@ -1249,7 +1249,7 @@ proc genMagic(c: var TCtx, n: PNode; m: TMagic) =
c.emitByVal dest
arg n[2]
else:
const kind = [mInc: mnkAddI, mDec: mnkSubI]
const kind = [mInc: mnkAdd, mDec: mnkSub]
# the unchecked arithmetic operators can be used directly
c.buildTree kind[m], dest.typ:
c.use dest
Expand Down Expand Up @@ -1281,9 +1281,18 @@ proc genMagic(c: var TCtx, n: PNode; m: TMagic) =
# float arithmetic operations:
of mAddF64, mSubF64, mMulF64, mDivF64:
proc op(c: var TCtx, m: TMagic, a, b: PNode) =
c.buildMagicCall m, n.typ:
arg a
arg b
if optInfCheck in c.userOptions:
# needs an overflow check
c.buildDefectMagicCall m, n.typ:
arg a
arg b
else:
# the unchecked version can be used
const Map = [mAddF64: mnkAdd, mSubF64: mnkSub,
mMulF64: mnkMul, mDivF64: mnkDiv]
c.buildTree Map[m], n.typ:
c.genArgExpression(a, sink=false)
c.genArgExpression(b, sink=false)

if optNaNCheck in c.userOptions:
let tmp = c.wrapTemp n.typ:
Expand All @@ -1295,6 +1304,8 @@ proc genMagic(c: var TCtx, n: PNode; m: TMagic) =
c.use tmp
else:
op(c, m, n[1], n[2])
of mUnaryMinusF64:
c.genOp mnkNeg, n.typ, n[1]

# magics that use incomplete symbols (most of them are generated by
# ``liftdestructors``):
Expand Down
20 changes: 11 additions & 9 deletions compiler/mir/mirtrees.nim
Original file line number Diff line number Diff line change
Expand Up @@ -155,16 +155,18 @@ type
mnkCheckedCall ## invoke a magic procedure and pass along the provided arguments

# unary arithmetic operations:
mnkNegI ## signed integer negation (overflow is UB)
mnkNeg ## signed integer and float negation (for ints, overflow is UB)
# binary arithmetic operations:
mnkAddI ## signed integer addition (overflow is UB)
mnkSubI ## signed integer substraction (overflow is UB)
mnkMulI ## signed integer multiplication (overflow is UB)
mnkDivI ## signed integer division (division by zero is UB)
mnkAdd ## signed integer and float addition (for ints, overflow is UB)
mnkSub ## signed integer and float subtraction (for ints, overflow is UB)
mnkMul ## signed integer and float multiplication (for ints, overflow is
## UB)
mnkDiv ## signed integer and float division (for ints, division by zero is
## UB)
mnkModI ## compute the remainder of an integer division (division by zero
## is UB)
# future direction: the arithmetic operations should be generalized to also
# apply to unsigned integers and floats
# future direction: the arithmetic operations should also apply to
# unsigned integers

mnkRaise ## if the operand is an ``mnkNone`` node, reraises the
## currently active exception. Otherwise, set the operand value
Expand Down Expand Up @@ -351,9 +353,9 @@ const
mnkAsgn, mnkSwitch, mnkFastAsgn, mnkVoid, mnkRaise, mnkEmit,
mnkAsm} + DefNodes

UnaryOps* = {mnkNegI}
UnaryOps* = {mnkNeg}
## All unary operators
BinaryOps* = {mnkAddI, mnkSubI, mnkMulI, mnkDivI, mnkModI}
BinaryOps* = {mnkAdd, mnkSub, mnkMul, mnkDiv, mnkModI}
## All binary operators

LvalueExprKinds* = {mnkPathPos, mnkPathNamed, mnkPathArray, mnkPathVariant,
Expand Down
6 changes: 3 additions & 3 deletions compiler/mir/utils.nim
Original file line number Diff line number Diff line change
Expand Up @@ -320,16 +320,16 @@ proc exprToStr(nodes: MirTree, i: var int, result: var string, env: EnvPtr) =
argToStr()
result.add ") (raises)"
of UnaryOps:
const Map = [mnkNegI: "-"]
const Map = [mnkNeg: "-"]
let kind = nodes[i].kind
tree Map[kind]:
valueToStr()
of BinaryOps:
let kind = nodes[i].kind
tree "":
valueToStr() # first operand
const Map = [mnkAddI: " + ", mnkSubI: " - ",
mnkMulI: " * ", mnkDivI: " div ", mnkModI: " mod "]
const Map = [mnkAdd: " + ", mnkSub: " - ",
mnkMul: " * ", mnkDiv: " div ", mnkModI: " mod "]
result.add Map[kind]
valueToStr() # second operand
else:
Expand Down
26 changes: 13 additions & 13 deletions compiler/vm/vmgen.nim
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ import
import std/options as std_options

from compiler/backend/compat import getInt, isOfBranch, skipConv, lastSon,
getMagic
getMagic, pick

from std/bitops import bitor

Expand Down Expand Up @@ -1954,7 +1954,6 @@ proc genMagic(c: var TCtx; n: CgNode; dest: var TDest; m: TMagic) =
of mUnaryMinusI, mUnaryMinusI64:
genUnaryABC(c, n, dest, opcUnaryMinusInt)
genNarrow(c, n, dest)
of mUnaryMinusF64: genUnaryABC(c, n, dest, opcUnaryMinusFloat)
of mUnaryPlusI, mUnaryPlusF64: gen(c, n[1], dest)
of mBitnotI:
genUnaryABC(c, n, dest, opcBitnotInt)
Expand Down Expand Up @@ -3057,15 +3056,15 @@ proc genClosureConstr(c: var TCtx, n: CgNode, dest: TRegister) =
c.freeTemp(tmp2)
c.freeTemp(envTmp)

proc binaryArith(c: var TCtx, e, x, y: CgNode, dest: var TDest, op: TOpcode) =
proc binaryArith(c: var TCtx, e, x, y: CgNode, dest: var TDest,
intOp, floatOp: TOpcode) =
## Emits the instruction sequence for the binary operation `e` with opcode
## `op`. `x` and `y` are the operand expressions.
prepare(c, dest, e.typ)
let
a = c.genx(x)
b = c.genx(y)
c.gABC(e, op, dest, a, b)
c.genNarrow(x, dest)
c.gABC(e, pick(e, intOp, floatOp), dest, a, b)
c.freeTemp(a)
c.freeTemp(b)

Expand All @@ -3089,15 +3088,16 @@ proc gen(c: var TCtx; n: CgNode; dest: var TDest) =
else:
genCall(c, n, dest)
clearDest(c, n, dest)
of cnkNegI:
of cnkNeg:
prepare(c, dest, n.typ)
let a = c.genx(n[0])
c.gABC(n, opcUnaryMinusInt, dest, a)
c.genNarrow(n[0], dest)
of cnkAddI: binaryArith(c, n, n[0], n[1], dest, opcAddu)
of cnkSubI: binaryArith(c, n, n[0], n[1], dest, opcSubu)
of cnkMulI: binaryArith(c, n, n[0], n[1], dest, opcMulu)
of cnkDivI: binaryArith(c, n, n[0], n[1], dest, opcDivInt)
of cnkModI: binaryArith(c, n, n[0], n[1], dest, opcModInt)
c.gABC(n, pick(n, opcUnaryMinusInt, opcUnaryMinusFloat), dest, a)
c.freeTemp(a)
of cnkAdd: binaryArith(c, n, n[0], n[1], dest, opcAddu, opcAddFloat)
of cnkSub: binaryArith(c, n, n[0], n[1], dest, opcSubu, opcSubFloat)
of cnkMul: binaryArith(c, n, n[0], n[1], dest, opcMulu, opcMulFloat)
of cnkDiv: binaryArith(c, n, n[0], n[1], dest, opcDivInt, opcDivFloat)
of cnkModI: binaryArith(c, n, n[0], n[1], dest, opcModInt, opcModInt)
of cnkIntLit, cnkUIntLit:
prepare(c, dest, n.typ)
c.loadInt(n, dest, getInt(n))
Expand Down
24 changes: 24 additions & 0 deletions tests/lang_objects/destructor/tdestruction_when_checks_failed.nim
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,30 @@ block abs_overflow_check:
runTest:
test(low(int32)) # provoke an overflow-check failure

block float_inf_check:
# enable infinity checks first; they're disabled by default
{.push infChecks: on.}

proc test(a: float) =
var obj = Object() # obj stores a value that needs to be destroyed
discard 1.0 / a

{.pop.}

numDestroy = 0
var raised = false
try:
test(0.0) # provoke an infinity-check failure
except:
raised = true

when defined(js) or defined(vm):
# XXX: infinity checks aren't yet implemented on these targets
doAssert not raised, "NaN checks are implemented"
else:
doAssert raised
doAssert numDestroy == 1

block float_nan_check:
# enable nan checks first; they're disabled by default
{.push nanChecks: on.}
Expand Down
7 changes: 5 additions & 2 deletions tests/lang_types/float/tfloat1.nim
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
discard """
matrix: "--exceptions:goto"
outputsub: "Error: unhandled exception: FPU operation caused an overflow [FloatOverflowDefect]"
targets: "c js vm"
outputsub: "FPU operation caused an overflow"
exitcode: "1"
knownIssue.js vm: '''
Overflow checks for floating-point arithmetic are not implemented
'''
"""
# Test new floating point exceptions

Expand Down

0 comments on commit 34a9654

Please sign in to comment.