Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

improve callWithStackShell #426

Merged
merged 1 commit into from
Mar 17, 2013
Merged

Conversation

MartinNowak
Copy link
Member

  • compatible with LDC's inline asm
  • only push callee save registers

@MartinNowak
Copy link
Member Author

This supersedes #414.
@klickverbot can you have a look whether this correctly fixes the LDC issue?

@dnadlinger
Copy link
Member

My main dev laptop (well, actually my only one) is being repaired right now, so I unfortunately won't be able to test it until the end of the week or so. Looks like it should work fine, though.

Is there a reason to load the stored values back into the registers? They are guaranteed to be preserved by the ABI anyway, and e.g. the GDC implementation doesn't bother with restoring them.

You might also want to add a short comment mentioning that it is the callee-save regs that are pushed, and as a possible micro-optimization add a void initializer to the static arrays.

@MartinNowak
Copy link
Member Author

Is there a reason to load the stored values back into the registers? They are guaranteed to be preserved by the ABI anyway, and e.g. the GDC implementation doesn't bother with restoring them.

Right, the compiler will restore them now that we're no longer using naked asm blocks.

You might also want to add a short comment mentioning that it is the callee-save regs that are pushed, and as a possible micro-optimization add a void initializer to the static arrays.

Thanks for the review and finding these bugs.

@ibuclaw
Copy link
Member

ibuclaw commented Mar 6, 2013

With gdc, the built-in function called may pop the registers back at the end of the function, but that detail is different for each supported target backend in the codegen.

@MartinNowak
Copy link
Member Author

With gdc, the built-in function called may pop the registers back at the end of the function, but that detail is different for each supported target backend in the codegen.

But it will correctly leave the stack frame?

@alexrp
Copy link
Member

alexrp commented Mar 6, 2013

Yes. Keep in mind that for GDC, the __builtin_unwind_init function is used rather than inline assembly, so the compiler can deal with it all.

@ghost ghost assigned dnadlinger Mar 6, 2013
@MartinNowak
Copy link
Member Author

Ping.

@dnadlinger
Copy link
Member

Urgh, I just discovered that regs[1] offsets one byte into regs, not one element. This is completely counterintuitive.

@WalterBright: Can you confirm whether this is a bug or the offset syntax works as designed?

@dnadlinger
Copy link
Member

(I'd expect to see something like 1[regs] for the 1 byte offset case)

@MartinNowak
Copy link
Member Author

Oh, I thought about it for one second but didn't recheck. How does this even pass the test suite?
And I think it's a bug btw.

- compatible with LDC's inline asm
- only push callee save registers
@MartinNowak
Copy link
Member Author

Fixed and Bugzilla 9738.

dnadlinger added a commit that referenced this pull request Mar 17, 2013
@dnadlinger dnadlinger merged commit a856aa2 into dlang:master Mar 17, 2013
@MartinNowak MartinNowak deleted the callWithStackShell branch March 17, 2013 15:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants