Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

write system.abs in nim, not magic #476

Open
timotheecour opened this issue Dec 22, 2020 · 7 comments
Open

write system.abs in nim, not magic #476

timotheecour opened this issue Dec 22, 2020 · 7 comments

Comments

@timotheecour
Copy link
Owner

refs nim-lang#16432 (comment)

func abs*[T: SomeSignedInt](x: T): T {.inline.} =
  ## Returns the absolute value of `x`.
  ##
  ## If `x` is ``low(x)`` (that is -MININT for its type),
  ## an overflow exception is thrown (if overflow checking is turned on).
  if x < 0: -x else: x

and remove mAbsI magic, and then, in subsequent work, investigate whether the branchless bit hack https://graphics.stanford.edu/~seander/bithacks.html#IntegerAbs can improve performance in at least some cases ?

@ringabout ringabout self-assigned this Dec 25, 2020
@ringabout
Copy link
Collaborator

ringabout commented Dec 26, 2020

Implementing them as magics makes them could be folded in compile time

  of mAbsI: result = foldAbs(getInt(a), n, g)

@timotheecour
Copy link
Owner Author

does it matter though? why would abs be singled-out as being special? I have a feeling this is for historical reasons that aren't relevant anymore; I have a WIP branch but no PR yet.

@ringabout
Copy link
Collaborator

ringabout commented Dec 26, 2020

abs(-4) is folded into 4?
Besides this micro optimization, mAbs could be removed safely.

@ringabout
Copy link
Collaborator

system.min and system.max are also magics, do I need to clean them?

@timotheecour
Copy link
Owner Author

timotheecour commented Dec 27, 2020

indeed, const folding does happen for max/min as can be seen by looking at codegen for:

when true:
  func max2[T](a, b: T): T {.inline.} =
    if a<b: b else: a
  proc main()=
    let a1 = max(11+1,12+2)
    let a2 = max2(11+1,12+2)
    echo (a1, a2)
  main()

=> codegen shows (with nim r -d:danger main)

	a1 = ((NI) 14);
	a2 = max2__g9bYy9c5l5m9b6EyjAm9ac4Y9bQt11587(((NI) 12), ((NI) 14));

It feels very odd to have these hard-coded cases in the compiler.

Even if const folding is desired (which should be justified; C compiler should already take care of this; I can't see how this can possibly generate different optimized binary code), why should a magic be used for that? Instead of, say, a flag (maybe enabled by --opt:speed|size or preferably a dedicated one eg --constfolding) which would control whether expressions can safely be const folded without changing semantics, as should be the case with max2.

I think for abs the cleanup doesn't require an RFC (abs is relatively rare) but maybe it's needed for other magics-that-shouldn't-be-magics like max,min etc. I'm curious in fact how many of them there are.

@timotheecour
Copy link
Owner Author

regarding symbols that trigger const folding, IIUC it's handled by ctfeWhitelist in compiler

# things that we can evaluate safely at compile time, even if not asked for it:
const
  ctfeWhitelist* = {mNone, mSucc, ...

@ringabout
Copy link
Collaborator

what's the progress?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants