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

Issue 1154253004: Use R0/R1 instead of R4/R5 for ARM eh_return return values (Closed)

Created:
5 years, 6 months ago by Derek Schuff
Modified:
5 years, 6 months ago
CC:
native-client-reviews_googlegroups.com
Base URL:
https://chromium.googlesource.com/native_client/pnacl-llvm.git@master
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Use R0/R1 instead of R4/R5 for ARM eh_return return values Returning things in callee-save registers can clobber live variables in functions with catch blocks. I'm not 100% sure how this ever worked. The side effect of this means I had to fix the return behavior of functions which return via eh_return so that they save and restore R0/R1 in addition to the callee-save registers. This is all part of our ARM dwarf EH localmod, but doing this same thing on x86 should fix PR8541. BUG= https://code.google.com/p/nativeclient/issues/detail?id=4171 R=jvoung@chromium.org Committed: https://chromium.googlesource.com/native_client/pnacl-llvm/+/43d482ad1c96633d1bfb5ed7d1e50c97928ea8fa

Patch Set 1 #

Total comments: 8

Patch Set 2 : review #

Unified diffs Side-by-side diffs Delta from patch set Stats (+81 lines, -16 lines) Patch
M lib/Target/ARM/ARMBaseRegisterInfo.cpp View 1 2 chunks +9 lines, -1 line 0 comments Download
M lib/Target/ARM/ARMCallingConv.td View 1 1 chunk +1 line, -0 lines 0 comments Download
M lib/Target/ARM/ARMFrameLowering.cpp View 1 4 chunks +38 lines, -0 lines 0 comments Download
M lib/Target/ARM/ARMISelLowering.cpp View 1 chunk +8 lines, -15 lines 0 comments Download
A test/CodeGen/ARM/ehreturn.ll View 1 1 chunk +25 lines, -0 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
Derek Schuff
5 years, 6 months ago (2015-06-03 00:28:33 UTC) #1
jvoung (off chromium)
otherwise LGTM were you thinking of adding a .ll test, e.g., that checks the push ...
5 years, 6 months ago (2015-06-03 17:19:38 UTC) #2
Derek Schuff
also added test. https://codereview.chromium.org/1154253004/diff/1/lib/Target/ARM/ARMBaseRegisterInfo.cpp File lib/Target/ARM/ARMBaseRegisterInfo.cpp (right): https://codereview.chromium.org/1154253004/diff/1/lib/Target/ARM/ARMBaseRegisterInfo.cpp#newcode67 lib/Target/ARM/ARMBaseRegisterInfo.cpp:67: // @LOCALMOD-START On 2015/06/03 17:19:38, jvoung ...
5 years, 6 months ago (2015-06-03 20:50:57 UTC) #3
Derek Schuff
5 years, 6 months ago (2015-06-03 20:57:00 UTC) #4
Message was sent while issue was closed.
Committed patchset #2 (id:20001) manually as
43d482ad1c96633d1bfb5ed7d1e50c97928ea8fa.

Powered by Google App Engine
This is Rietveld 408576698