[Lldb-commits] [PATCH] D140386: [LLDB][LoongArch] Add FP branch instructions for EmulateInstructionLoongArch

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

Please split this patch into 2:

- the cleanup of the existing branch instructions
- the addition of the new ones

The changes look good but let's keep each commit to doing 1 thing.

You are missing tests though, and this is my mistake for not asking for them in the previous patches. See https://reviews.llvm.org/D139294 for examples of how riscv is doing it. It doesn't need to be totally comprehensive. For example we can assume basic register operands are extracted correctly. So for a branch the tests would be one where it is supposed to branch and one where it isn't. The infrastructure for testing the emulation varies between arch but as before, riscv probably the best reference point.

Please add testing for the existing instructions as parent patches of this one (these 2 once you've split them), then these new ones should have tests too.

Note: I will be off for Christmas break from tomorrow until the 2nd week of January. So you can add any of the other common reviewers in lldb in the meantime. Or wait for me to be back, feel free to pile up reviews :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140386/new/

https://reviews.llvm.org/D140386