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

Issue 1478923002: Revert of binary-operator-reducer: reduce mul+div(shift) (Closed)

Created:
5 years ago by Benedikt Meurer
Modified:
5 years ago
CC:
danno, v8-reviews_googlegroups.com
Base URL:
https://chromium.googlesource.com/v8/v8.git@master
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

Revert of binary-operator-reducer: reduce mul+div(shift) (patchset #11 id:200001 of https://codereview.chromium.org/1350223006/ ) Reason for revert: This is also unsound for the reasons outlined in https://codereview.chromium.org/1473073004/ Will help Fedor to implement a solution based on simplified operators. Original issue's description: > binary-operator-reducer: reduce mul+div(shift) > > Reduction Input: > > ChangeInt32ToFloat64=> TruncateFloat64ToInt32 > Float64Mul=> > ChangeInt32ToFloat64=> Float64Div=>TruncateFloat64ToInt32 > > Output: > > => TruncateInt64ToInt32 > Int64Mul > => Int64Shr => TruncateInt64ToInt32 > > Test code: > > function mul(a, b) { > var l = a & 0x3ffffff; > var h = b & 0x3ffffff; > var m = l * h; > > var rl = m & 0x3ffffff; > var rh = (m / 0x4000000) | 0; > > return rl | rh; > } > > mul(1, 2); > var a0 = mul(0x3ffffff, 0x3ffffff); > mul(0x0, 0x0); > %OptimizeFunctionOnNextCall(mul); > var a1 = mul(0x3ffffff, 0x3ffffff); > > print(a0 + ' == ' + a1); > > BUG= > R=mstarzinger@chromium.org > > Committed: https://crrev.com/461e5b49d022335a7fc4e9d172397a4bd48b93d4 > Cr-Commit-Position: refs/heads/master@{#31899} TBR=mstarzinger@chromium.org,danno@chromium.org,titzer@chromium.org,fedor@indutny.com NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= Committed: https://crrev.com/5d18e93bd61dd0492578002f2eefd56402bd56af Cr-Commit-Position: refs/heads/master@{#32313}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+0 lines, -304 lines) Patch
M BUILD.gn View 1 chunk +0 lines, -2 lines 0 comments Download
D src/compiler/binary-operator-reducer.h View 1 chunk +0 lines, -52 lines 0 comments Download
D src/compiler/binary-operator-reducer.cc View 1 chunk +0 lines, -128 lines 0 comments Download
M src/compiler/machine-operator-reducer.cc View 1 chunk +0 lines, -1 line 0 comments Download
M src/compiler/pipeline.cc View 2 chunks +0 lines, -4 lines 0 comments Download
D test/unittests/compiler/binary-operator-reducer-unittest.cc View 1 chunk +0 lines, -94 lines 0 comments Download
M test/unittests/compiler/machine-operator-reducer-unittest.cc View 1 chunk +0 lines, -12 lines 0 comments Download
M test/unittests/compiler/node-test-utils.h View 3 chunks +0 lines, -5 lines 0 comments Download
M test/unittests/compiler/node-test-utils.cc View 3 chunks +0 lines, -3 lines 0 comments Download
M test/unittests/unittests.gyp View 1 chunk +0 lines, -1 line 0 comments Download
M tools/gyp/v8.gyp View 1 chunk +0 lines, -2 lines 0 comments Download

Messages

Total messages: 6 (2 generated)
Benedikt Meurer
Created Revert of binary-operator-reducer: reduce mul+div(shift)
5 years ago (2015-11-26 06:14:36 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1478923002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1478923002/1
5 years ago (2015-11-26 06:14:46 UTC) #2
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years ago (2015-11-26 06:15:02 UTC) #4
commit-bot: I haz the power
5 years ago (2015-11-26 06:16:08 UTC) #6
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/5d18e93bd61dd0492578002f2eefd56402bd56af
Cr-Commit-Position: refs/heads/master@{#32313}

Powered by Google App Engine
This is Rietveld 408576698