<html><body><p><tt>Hello Pavel,</tt><br><br><tt>> It's exciting to see support for new platforms being added to LLDB.<br>> I've had a brief glance at the patch set and I was surprised at how<br>> few changes you had actually needed to make to lldb core to support<br>> this. The patches look good at a first glance, but they will need to<br>> be reviewed by respective code owners.<br>> <br>> Going forward, the best way to handle this would be to create a<br>> phabricator account and submit the patches there. This will make the<br>> review much easier. For reviewers, you can use the CODE_OWNERS.txt<br>> file (or just git history). If you don't know the right person, just<br>> pick Greg Clayton for general stuff and me or Tamas Berghammer for<br>> Linux-specific code, and we will route it further if needed.<br></tt><br><tt>OK, sure, I will do that. Thanks for the pointers!</tt><br><br><tt>> Also, please run clang-format on your patches before submission. Let<br>> me know if you need any help setting that up.<br></tt><br><tt>Well, in some cases existing / surrounding code already differs from</tt><br><tt>the format enforced by clang-format (e.g. space vs. no space before</tt><br><tt>the '(' of a function call). I've now used clang-format on all the</tt><br><tt>new files, and most other changes, except where the style would</tt><br><tt>directly contradict surrounding code.</tt><br><br><tt>> > This should provide complete support for debugging the base SystemZ<br>> > platform. Not yet supported are optional features like transaction support<br>> > (zEC12) or SIMD vector support (z13). Note that there is no instruction<br>> > emulation, since our ABI requires that all code provides correct DWARF CFI<br>> > in .eh_frame to support unwinding.<br>> <br>> Is the CFI info correct at all PC locations, or only at call sites<br>> (synchronous vs. asynchronous exceptions). If it's the latter, you may<br>> still need to do some instruction emulation to "augment" the CFI info<br>> for it to be usable at non-call-sites. However, that's definitely not<br>> a blocker now...<br></tt><br><tt>It should be correct at all PC locations (-fasynchronous-unwind-tables).</tt><br><tt><br>> > Looking forward to working with you on getting SystemZ support upstream! I'd<br>> > also be happy to take on ongoing maintenance (setting up a build bot, fixing<br>> > issues as they come up, etc ...).<br>> <br>> Setting up a buildbot is definitely recommended and it will make<br>> further significantly easier.<br></tt><br><tt>I'll look into setting this up once the code is in.</tt><br><br><tt>Thanks for your comments!</tt><br><br><tt>Bye,<br>Ulrich</tt><br><BR>
</body></html>