This is an archive of the discontinued LLVM Phabricator instance.

[X86] Convert esp-relative movs of function arguments to pushes, step 1
ClosedPublic

Authored by mkuper on Dec 3 2014, 8:06 AM.

Details

Summary

I've started working on using pushes instead of esp-relative movs to pass function arguments.
The main motivation is code size (e.g. 2 bytes instead of 7 to pass an 8-bit immediate), but it some cases it is also faster.

This is just the first step, that handles the simplest case:

  1. x86-32 calling convention, everything is passed through the stack.
  2. There is no reserved frame (for x86 this happens is there is a dynamic stack allocation in the caller).

Does this look reasonably sane?

Diff Detail

Event Timeline

mkuper updated this revision to Diff 16864.Dec 3 2014, 8:06 AM
mkuper retitled this revision from to [X86] Convert esp-relative movs of function arguments to pushes, step 1.
mkuper updated this object.
mkuper edited the test plan for this revision. (Show Details)
mkuper added reviewers: nadav, delena, eliben.
mkuper added a subscriber: Unknown Object (MLST).
nadav edited edge metadata.Dec 3 2014, 8:23 AM

Hi Michael,

Overall it looks really good. Did you measure the performance of this patch on the llvm test suite? Do you know if this transformation is beneficial for all recent x86 processors? If it is not then we can enable it for specific processors.

Also, can you use DenseMap instead of std::map?

Thanks,
Nadav

mkuper added a comment.Dec 3 2014, 1:45 PM

Thanks for the review, Nadav!

  1. Yes, the mov -> push transformation should be beneficial (or, at least, not harmful) on all recent Intel architectures when the pushed value is an immediate or a register (not memory), which is what this patch handles.
  2. I don't really expect a performance change, in either direction - this step is mostly for code size.

You're absolutely right, though, I should verify that with the test-suite. If there are any anomalous results, I'll post them here.

  1. I used std::map because it guarantees ordered iteration, which is useful here. I thought DenseMap doesn't have that guarantee. Am I wrong?

Michael

mkuper added a comment.Dec 8 2014, 8:01 AM

As expected, no performance change.

Any other issues (there's actually a small style violation in the review, will fix before committing) ?

From: Nadav Rotem [mailto:nrotem@apple.com]
Sent: Wednesday, December 03, 2014 23:54
To: reviews+D6503+public+b702a7f66de6a943@reviews.llvm.org
Cc: Kuperstein, Michael M; Demikhovsky, Elena; eliben@google.com; llvm-commits@cs.uiuc.edu
Subject: Re: [PATCH] [X86] Convert esp-relative movs of function arguments to pushes, step 1

mkuper closed this revision.Dec 8 2014, 10:11 PM
mkuper updated this revision to Diff 17070.

Closed by commit rL223757 (authored by @mkuper).