Page MenuHomePhabricator

[LLDB] Added support for PHI nodes to IR interpreter
ClosedPublic

Authored by cameron314 on Apr 14 2016, 10:01 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

cameron314 retitled this revision from to [LLDB] Added support for PHI nodes to IR interpreter.
cameron314 updated this object.
cameron314 added a reviewer: spyffe.
cameron314 set the repository for this revision to rL LLVM.
cameron314 added subscribers: mamai, lldb-commits.

I'll let Sean comment on the content of the patch, but please add a test
that runs such an expression and demonstrates the correct output.

Ah, that's a bit tricky at the moment. The LLVM tip no longer compiles with VS2015 (specifically lib\Support\SourceMgr.cpp), and my Python setup for VS2013 is all wonky.
This will have to wait a bit.

http://lab.llvm.org:8011/builders/lldb-x86-windows-msvc2015

My build bot which uses MSVC 2015 is workign fine. What kind of error are
you seeing?

Whoops, my fault. I was accidentally using the VS2013 command prompt with the 2015 cmake files. Compiling happily as we speak, test coming up soon after.

cameron314 updated this revision to Diff 54134.Apr 18 2016, 3:53 PM
cameron314 removed rL LLVM as the repository for this revision.

I've added a test for this patch.

I'd appreciate if someone could take a look at this when they have a moment.

This is going to have to be Sean, but his delay can be unpredictable. Keep
pinging every so often until you get a response

All right, no worries :-) I didn't want to bother anyone.

@spyffe, do you have time to look at this sometime this week? It's a very simple patch (the test code took longer to write than the patch itself). Thanks!

spyffe edited edge metadata.May 9 2016, 12:57 PM
spyffe added a subscriber: spyffe.

Ah… you know me too well.
Looking at this now.

spyffe accepted this revision.May 9 2016, 1:01 PM
spyffe edited edge metadata.

This patch is fine. I especially appreciate the time you took making the IRInterpreter properly testable. I will use this myself.

source/Commands/CommandObjectExpression.cpp
67 ↗(On Diff #54134)

This is a great feature which I will use in test cases.

source/Expression/IRInterpreter.cpp
1102 ↗(On Diff #54134)

Looks all right to me. Thank you!

This revision is now accepted and ready to land.May 9 2016, 1:01 PM

Excellent, thank you!

This revision was automatically updated to reflect the committed changes.

Looks like this CL broke CMake build bot - http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-cmake/builds/14634, could you take a look?