Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2762993002:
Fix test Regression6046b. (Closed)
|
Created:
3 years, 9 months ago by Ilija.Pavlovic1 Modified:
3 years, 9 months ago CC:
v8-reviews_googlegroups.com Target Ref:
refs/heads/master Project:
v8 Visibility:
Public. |
DescriptionMIPS: Do not use ubfx for shr+and combination for mask=0
Port for https://codereview.chromium.org/2737493002
TEST=cctest/test-run-machops/Regression6046b
BUG=
Review-Url: https://codereview.chromium.org/2762993002
Cr-Commit-Position: refs/heads/master@{#44019}
Committed: https://chromium.googlesource.com/v8/v8/+/1cedeb39837accd5b7800d5caf515de51cb74704
Patch Set 1 #Patch Set 2 : Update according comments. #Messages
Total messages: 18 (7 generated)
PTAL
On 2017/03/21 at 12:25:57, Ilija.Pavlovic wrote: > PTAL Why should this test only 64 bit architectures? It does not contain any 64-bit specific code. It passes on ia32 just fine.
On 2017/03/21 at 12:32:02, ahaas wrote: > On 2017/03/21 at 12:25:57, Ilija.Pavlovic wrote: > > PTAL > > Why should this test only 64 bit architectures? It does not contain any 64-bit specific code. It passes on ia32 just fine. I think this test reveals a bug in the instruction selector of mips. I fixed the same bug for arm in https://codereview.chromium.org/2737493002.
On 2017/03/21 12:32:02, ahaas wrote: > On 2017/03/21 at 12:25:57, Ilija.Pavlovic wrote: > > PTAL > > Why should this test only 64 bit architectures? It does not contain any 64-bit > specific code. It passes on ia32 just fine. With CL https://codereview.chromium.org/2755373002 the test Regression6046b failed on MIPS32. I noted that this test is now enabled for 32-bit architectures, and then the question was: is it typo or intentionally changed? This is actually the reason for this CL.
On 2017/03/21 12:40:24, ahaas wrote: > On 2017/03/21 at 12:32:02, ahaas wrote: > > On 2017/03/21 at 12:25:57, Ilija.Pavlovic wrote: > > > PTAL > > > > Why should this test only 64 bit architectures? It does not contain any 64-bit > specific code. It passes on ia32 just fine. > > I think this test reveals a bug in the instruction selector of mips. I fixed the > same bug for arm in https://codereview.chromium.org/2737493002. Thx! I will check it for MIPS.
Description was changed from ========== Fix test Regression6046b. Test Regression6046b should run only on 64-bit architectures. TEST=cctest/test-run-machops/Regression6046b BUG= ========== to ========== MIPS: Do not use ubfx for shr+and combination for mask=0 Port for https://codereview.chromium.org/2737493002 TEST=cctest/test-run-machops/Regression6046b BUG= ==========
On 2017/03/21 at 13:58:05, Ilija.Pavlovic wrote: > On 2017/03/21 12:32:02, ahaas wrote: > > On 2017/03/21 at 12:25:57, Ilija.Pavlovic wrote: > > > PTAL > > > > Why should this test only 64 bit architectures? It does not contain any 64-bit > > specific code. It passes on ia32 just fine. > > With CL https://codereview.chromium.org/2755373002 the test Regression6046b failed on MIPS32. > I noted that this test is now enabled for 32-bit architectures, and then the question was: is it typo or intentionally changed? This is actually the reason for this CL. Ah, yes, originally I made the 64-bit version of the test, and when I added the 32-bit version, I accidentally added it in the "V8_TARGET_ARCH_64_BIT" scope. In my last CL I saw this mistake and moved the 32-bit test out of the scope. It's true that thereby the change looked like a typo. I'm sorry.
miran.karic@imgtec.com changed reviewers: + Miran.Karic@imgtec.com
lgtm
On 2017/03/22 at 08:59:36, Miran.Karic wrote: > lgtm lgtm
The CQ bit was checked by ilija.pavlovic@imgtec.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
ivica.bogosavljevic@imgtec.com changed reviewers: + ilija.pavlovic@imgtec.com - Ilija.Pavlovic@imgtec.com, Miran.Karic@imgtec.com
lgtm
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1490184288335790,
"parent_rev": "6f800b32ad381e302e80cf378447192927bf6287", "commit_rev":
"1cedeb39837accd5b7800d5caf515de51cb74704"}
Message was sent while issue was closed.
Description was changed from ========== MIPS: Do not use ubfx for shr+and combination for mask=0 Port for https://codereview.chromium.org/2737493002 TEST=cctest/test-run-machops/Regression6046b BUG= ========== to ========== MIPS: Do not use ubfx for shr+and combination for mask=0 Port for https://codereview.chromium.org/2737493002 TEST=cctest/test-run-machops/Regression6046b BUG= Review-Url: https://codereview.chromium.org/2762993002 Cr-Commit-Position: refs/heads/master@{#44019} Committed: https://chromium.googlesource.com/v8/v8/+/1cedeb39837accd5b7800d5caf515de51cb... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/v8/v8/+/1cedeb39837accd5b7800d5caf515de51cb... |
