Unify wrapper paths

[compnerd]

Merge the block and non-blocking sys call wrappers

Motivation:

The two paths are nearly identical with the core difference being that blocking calls ignore EWOULDBLOCK. This simplifies the handling for the different types by adding a parameter to the wrapSyscall method.

Modifications:

Added a new parameter nonblocking to the wrapSyscall and wrapSyscallWouldBlock. Those paths are then merged into a single call method.

Result:

No external changes.

[weissi]

I'm okay with this if we can show that it leads to the same assembly. Back when we first wrote this, the optimiser couldn't optimise those two cases into the best code which is why we had to split it. I'm happy to undo this now if we can show that the code isn't affected by this.

[weissi]

nit: we usually put newlines after every {

[weissi]

I think we should still name this wrapSyscall or callSyscall or something. This wrapper only works if the called function behaves like a unix syscall (ie. -1 == error)

[compnerd]

I think addressing the generated code issue first is more important, fixing the style stuff can come later. I checked on an ubuntu build, it depends on how you write the code and what you want the result to be.

Looking at the code for:

while true {
  let res = try body()
  if res == -1 {
    let err = errno
    if err == EINTR { continue }
    if blocking && err == EWOULDBLOCK { return .wouldBlock(0) }
    preconditionIsNotBlacklistedErrno(err: err, where: function)
    throw IOError(errnoCode: err, reason: function)
  }
  return .processed(res)
}

generates the same assembly (modulo offsets) - I've attached the before and after results.

However, it generates two functions, the static function and the merged function.

Writing it as:

while true {
  let res = try body()
  if res == -1 {
    let err = errno
    switch err {
    case EINTR: continue
    case EWOULDBLOCK:
      if blocking { return .wouldBlock(0) }
      fallthrough
    default:
      preconditionIsNotBlacklistedErrno(err: err, where: function)
      throw IOError(errnoCode: err, reason: function)
    }
  }
  return .processed(res)
}

produces a single function instead.

The single function is a little bit heavier on the frame size, but overall reduces the amount of code. There is a minor amount of extra overhead (2 extra cmp+jne, 4 total instructions) due to the unnecessary precondition check in the case of EWOULDBLOCK, but that is not going to happen anyways - and you can see that with -Ounchecked results.

[compnerd]

after.txt
before.txt
variant.txt

[weissi]

Thank you! That looks good to me

[Lukasa]

@compnerd Are you still ok with us going ahead and merging this separate from #1440?

[compnerd]

Yes, that would be wonderful. Small steps make it easier :)