This is an archive of the discontinued LLVM Phabricator instance.

[X86] Handle MachineBasicBlock as a memory displacement operand in the LEA optimization pass
ClosedPublic

Authored by aturetsk on Apr 22 2016, 5:15 AM.

Details

Summary

A memory displacement operand in a LEA instruction can be a MachineBasicBlock. Currently if that happens the LEA optimization pass crashes with 'UNREACHABLE'.
This patch fixes that.

Diff Detail

Repository
rL LLVM

Event Timeline

aturetsk updated this revision to Diff 54634.Apr 22 2016, 5:15 AM
aturetsk retitled this revision from to [X86] Handle MachineBasicBlock as a memory displacement operand in the LEA optimization pass.
aturetsk updated this object.
aturetsk added reviewers: nadav, qcolombet.
aturetsk added subscribers: llvm-commits, zinovy.nis.
aturetsk added inline comments.
test/CodeGen/X86/lea-opt-memop-check-1.ll
1 ↗(On Diff #54634)

This is just the renamed 'test/CodeGen/X86/lea-opt-memop-check.ll'.

aturetsk added a subscriber: dim.Apr 25 2016, 3:59 AM

The corresponding bug was filed yesterday (https://llvm.org/bugs/show_bug.cgi?id=27502).

aturetsk updated this revision to Diff 54832.Apr 25 2016, 4:43 AM

Mention the PR# in the test.

rnk accepted this revision.Apr 25 2016, 9:45 AM
rnk added a reviewer: rnk.
rnk added a subscriber: rnk.

Fix looks good, thanks!

test/CodeGen/X86/lea-opt-memop-check-1.ll
1 ↗(On Diff #54832)

Yes, this is just a file move, but I think we should remove this REQUIRES line so that we run this test in more configurations.

test/CodeGen/X86/lea-opt-memop-check-2.ll
1 ↗(On Diff #54832)

I don't think we need this REQUIRES asserts. The test will pass as is without assertions, right?

This revision is now accepted and ready to land.Apr 25 2016, 9:45 AM
This revision was automatically updated to reflect the committed changes.

Thanks for the review.
You are right, the REQUIRES line is redundant for tests to pass, so I removed them.

llvm/trunk/test/CodeGen/X86/lea-opt-memop-check-1.ll