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

New syscall to read a byte from STDIN #8

Merged
merged 3 commits into from
Jul 22, 2017
Merged

New syscall to read a byte from STDIN #8

merged 3 commits into from
Jul 22, 2017

Conversation

akkartik
Copy link
Contributor

No description provided.

when Syscall::READ
char = STDIN.raw &.read_char
if char.is_a?(Char)
reg_write Register::R0.byte, char.ord
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

char.ord returns an Int32, so you'd have to write this into the Register::R0.dword register. Besides that, maybe the user should be able to provide a custom register where he wants the result to end up in?

I was thinking of something along the lines of the sys_puts syscall.

when Syscall::PUTS
  reg = Register.new stack_pop UInt8 # A register identifier is popped off the stack here
  value = reg_read Int32, reg
  STDOUT.puts "#{value}"

You could then change your sys_read implementation to:

when Syscall::READ
  reg = Register.new stack_pop UInt8
  char = STDIN.raw &.read_char
  if char.is_a?(Char)
    reg_write reg, char.ord
  end

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍. I was using the standard Unix approach, where syscall arguments are in standard registers. I can change it, though I still want to understand better what is going on.

Let's consider the call to puts in in examples/loops.asm. It looks like push t_register, counter pushes an identifier corresponding to r0 to the stack? Does this mean that the stack maintains types for its contents at runtime, to distinguish between numbers and register identifiers? This is very different from anything I've seen. Can you elaborate on the rationale for this design?

What does push do, decrement sp and write to the address in it?

I'm curious how my existing implementation is working. The following example doesn't provide a register identifier on the stack for puts, but it seems to work. sys_puts picks up the result of sys_read from R0.

.org 0x00
.label entry_addr
.label main

  push t_syscall, sys_read
  syscall
  push t_syscall, sys_puts
  syscall

  push t_syscall, sys_exit
  syscall

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean that the stack maintains types for its contents at runtime, to distinguish between numbers and register identifiers?

No :) The push instruction takes two arguments. The first one is a size specifier which tells the push instruction how long the immediate value being pushed onto the stack is. In this case, t_register stands for 1 and counter stands for r0. counter gets defined in this file on line 2: .def counter r0 ; counter register.

I'm curious how my existing implementation is working.

Your program only works because of a small coincidence. The sys_puts syscall actually takes an argument (that you didn't put on the stack before), which tells the machine from which register it should take the value from.

Because the stack is empty at the time this is being executed, it underflows and it reads a byte from the section in memory that is reserved for machine internals (currently all zero). This is documented inside the file design/execution.md.

Because that whole section is zero, the sys_puts syscall reads a byte from there, and interprets it as a register identifier, in this case for R0.

That's the reason this program seems to be working.

Also provide a configurable register identifier for the destination,
just like with `sys_puts`. Thanks Leonard for the suggestions:
  #8 (comment)
@KCreate
Copy link
Owner

KCreate commented Jul 22, 2017

Looks good! Can you document the new syscall in the design/semantics.md? Just a single table entry will suffice.

@akkartik
Copy link
Contributor Author

Ah, good idea!

@KCreate
Copy link
Owner

KCreate commented Jul 22, 2017

Cool thanks! Ready to merge?

@akkartik
Copy link
Contributor Author

I'm ready if you are! 🙂

@KCreate KCreate merged commit 7fc81bf into KCreate:master Jul 22, 2017
KCreate pushed a commit that referenced this pull request Jul 22, 2017
Also provide a configurable register identifier for the destination,
just like with `sys_puts`. Thanks Leonard for the suggestions:
  #8 (comment)
@tekknolagi
Copy link
Contributor

How does this relate to crystal-lang/crystal#2065 and crystal-lang/crystal#2713 ? (with respect to STDIN.raw)

Also, why read a single char the not-normal way (a la getchar)?

@KCreate
Copy link
Owner

KCreate commented Jul 24, 2017

I don't know if and how much stackvm is affected by this issue. I do remember that there was a rationale behind choosing STDIN.raw &.read_char over just STDIN.read_char altough I can't remember what it was. I'll have to look into that.

@tekknolagi
Copy link
Contributor

tekknolagi commented Jul 24, 2017 via email

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

Successfully merging this pull request may close these issues.

3 participants