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

Issue 1223133007: Factor out prelowerPhi for 32-bit targets. Disable adv phi lowering for ARM. (Closed)

Created:
5 years, 5 months ago by jvoung (off chromium)
Modified:
5 years, 5 months ago
Reviewers:
Jim Stichnoth, John
CC:
native-client-reviews_googlegroups.com
Base URL:
https://chromium.googlesource.com/native_client/pnacl-subzero.git@master
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Factor out prelowerPhi for 32-bit targets. Disable adv phi lowering for ARM. This way, prelowerPhi can be shared between 32-bit targets (split 64-bit values into 32-bit ones, and legalize undef). Suggestions from template experts on how to share prelowerPhi welcome. I'm not particularly happy with the first pass in that legalizeUndef has to be made public (though other methods used are also public). Also the methods required from the template type TargetT aren't clear without looking through the code. The current advanced phi lowering code depends on lowerPhiAssignments. That is a special case of lowerAssign that does some adhoc register allocation. The current adhoc register allocation doesn't work as well when a target may need to spill more than one register. Disable that optimization for ARM for now, until we have a better way that works for ARM, and enable O2 cross testing on ARM. BUG= https://code.google.com/p/nativeclient/issues/detail?id=4076 R=stichnot@chromium.org Committed: https://gerrit.chromium.org/gerrit/gitweb?p=native_client/pnacl-subzero.git;a=commit;h=53483691eba6e23de63afe0579b436002d06d187

Patch Set 1 #

Patch Set 2 : header #

Patch Set 3 : fill in wqthe random test case for more undef stuff #

Unified diffs Side-by-side diffs Delta from patch set Stats (+149 lines, -36 lines) Patch
M Makefile.standalone View 1 chunk +4 lines, -4 lines 0 comments Download
M src/IceClFlags.cpp View 1 chunk +9 lines, -1 line 0 comments Download
A src/IcePhiLoweringImpl.h View 1 1 chunk +61 lines, -0 lines 0 comments Download
M src/IceTargetLoweringARM32.h View 2 chunks +1 line, -1 line 0 comments Download
M src/IceTargetLoweringARM32.cpp View 2 chunks +2 lines, -5 lines 0 comments Download
M src/IceTargetLoweringX86Base.h View 2 chunks +1 line, -1 line 0 comments Download
M src/IceTargetLoweringX86BaseImpl.h View 2 chunks +3 lines, -24 lines 0 comments Download
M tests_lit/llvm2ice_tests/64bit.pnacl.ll View 1 2 2 chunks +68 lines, -0 lines 0 comments Download

Messages

Total messages: 4 (1 generated)
jvoung (off chromium)
5 years, 5 months ago (2015-07-15 21:21:45 UTC) #2
Jim Stichnoth
LGTM, but unfortunately I can't say much about template wizardry.
5 years, 5 months ago (2015-07-15 22:33:17 UTC) #3
jvoung (off chromium)
5 years, 5 months ago (2015-07-16 17:47:51 UTC) #4
Message was sent while issue was closed.
Committed patchset #3 (id:30009) manually as
53483691eba6e23de63afe0579b436002d06d187 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698