Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(38)

Issue 6304009: [gcc] Replace rsp/rbp mov source with esp/ebp (Closed)

Created:
9 years, 11 months ago by eaeltsin
Modified:
9 years, 7 months ago
Reviewers:
khim
CC:
native-client-reviews_googlegroups.com
Visibility:
Public.

Description

[gcc] Replace rsp/rbp mov source with esp/ebp The problem is that with POINTERS_EXTEND_UNSIGNED > 0 gcc considers high part of rsp and rbp to be zero, which is not true. This seems to be backend design flaw. Fixing this by changing POINTERS_EXTEND_UNSIGNED to -1 and defining ptr_extend solves the problem but is likely to hit the performance of pointer arithmetics that does not involve rsp and rbp. This fix fights the particular and most common case when rsp or rbp are sources of the mov instruction, by forcing this instructions to operate on low parts of the registers only. This is not the complete solution, and more changes will follow. Test (-O1) must return 0 but returns high part of rsp without the fix: struct StringRef { const char *Data; unsigned Length; }; // Prevent inlining with 'weak' attribute int print_size(struct StringRef Data) __attribute__((weak)); int print_size(struct StringRef Data) { return Data.Length; } int main() { char str[6]; // NEEDED! struct StringRef s; s.Data = str; // NEEDED! s.Length = 0; return print_size(s); } BUG=http://code.google.com/p/nativeclient/issues/detail?id=1304 TEST=see above Committed: http://git.chromium.org/gitweb/?p=nacl-toolchain.git;a=commit;h=3416f7e

Patch Set 1 #

Patch Set 2 : ready for code review #

Unified diffs Side-by-side diffs Delta from patch set Stats (+27 lines, -16 lines) Patch
M gcc/gcc/config/i386/i386.md View 1 chunk +27 lines, -16 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
eaeltsin
9 years, 11 months ago (2011-01-18 18:25:20 UTC) #1
khim
9 years, 11 months ago (2011-01-18 18:29:48 UTC) #2
Well, if you consider that it was my idea initially...

LGTM

Powered by Google App Engine
This is Rietveld 408576698