Page MenuHomePhabricator

Big shift test
ClosedPublic

Authored by chfast on Feb 19 2015, 4:48 AM.

Details

Reviewers
loladiro
Summary

A test for patch D4978

Diff Detail

Repository
rL LLVM

Event Timeline

chfast updated this revision to Diff 20193.Feb 19 2015, 4:48 AM
chfast retitled this revision from to Big shift test.
chfast updated this object.
chfast edited the test plan for this revision. (Show Details)
chfast set the repository for this revision to rL LLVM.
chfast added a subscriber: Unknown Object (MLST).
chfast updated this object.Mar 10 2015, 2:08 AM
chfast added a reviewer: loladiro.
lhames added a subscriber: lhames.Mar 10 2015, 10:00 PM

Hi Paweł,

Is this JIT specific? What platforms are you seeing problems on? I suspect it should just be a CodeGen test for those platforms.

  • Lang.
loladiro edited edge metadata.Mar 10 2015, 10:03 PM

This is a non-JIT-specific codegen bug (see the linked patch). I only noticed it on X86, but I suspect other platforms may have similar semantics, so it may be an issue there anyway. The fix certainly isn't architecture specific, so it might make sense to have this run on all platforms.

I agree with @loladiro, it looks like generic CodeGen bug. Placing it in ExecutionEngine/MCJIT wasn't the best idea, but I've found that lli based test are there. Should I move it to some better place? Like test/CodeGen/Generic?

It's tricky. This may be a generic codegen bug, but it will manifest differently on different platforms. I'd be inclined to pick a popular target where it's currently broken (X86?), fix it there, then add the test to test/CodeGen/<Target>.

It's tricky. This may be a generic codegen bug, but it will manifest differently on different platforms. I'd be inclined to pick a popular target where it's currently broken (X86?), fix it there, then add the test to test/CodeGen/<Target>.

This test is a quite different from other Target tests because it checks runtime result. Maybe we should come back to that when the actual bug is fixed.

I'd like to include this test case with the bugfix. Can we decide on where it should live?

I'd like to include this test case with the bugfix. Can we decide on where it should live?

I would call it a "runtime" test. But I haven't found similar tests in test/ directory.

chfast updated this revision to Diff 24038.Apr 20 2015, 11:45 AM
chfast edited edge metadata.

A new test that checks X86_64 assembly

loladiro accepted this revision.Apr 20 2015, 11:48 AM
loladiro edited edge metadata.

LGTM. If you want, you can combine this with the fix and commit. Otherwise I'll get to it later today or tomorrow.

This revision is now accepted and ready to land.Apr 20 2015, 11:48 AM