Emmmer added inline comments.
================ Comment at: lldb/source/Target/Process.cpp:2373 + reported_after = false; + return reported_after; +} ---------------- DavidSpickett wrote: > DavidSpickett wrote: > > Would this be any clearer with multiple returns? Or one giant return, but > > the logic is a bit cryptic then. > > > > ``` > > const ArchSpec &arch = GetTarget().GetArchitecture(); > > if (!arch.IsValid()) > > return true; > > > > llvm::Triple triple = arch.GetTriple(); > > return !(triple.isMIPS() || triple.isPPC64() || triple.isAArch64() || > > triple.isArmMClass() || triple.isARM()); > > ``` > > > > Better as: > > ``` > > const ArchSpec &arch = GetTarget().GetArchitecture(); > > if (!arch.IsValid()) > > return false; > > > > llvm::Triple triple = arch.GetTriple(); > > if (triple.isMIPS() || triple.isPPC64() || triple.isAArch64() || > > triple.isArmMClass() || triple.isARM()) > > return false; > > > > return true; > > ``` > > > > Also, do we know what RISC-V does? > @Emmmer any idea? It seems standard RISC-V uses the `before` mode, that is > The action for this trigger will be taken just before the instruction that > triggered it is committed, but after all preceding instructions are committed. If I understand this code correctly (correct me if I was wrong), we should return `false` for RISC-V. But in the debug-spec [1], RISC-V seems to allow both `before` and `after` modes, which can be detected through CSRs. When debug-spec lands, we may change the code accordingly. [1]: https://github.com/riscv/riscv-debug-spec/blob/6309972901417a0fa72d812d2ffe89e432f00dff/xml/hwbp_registers.xml#L365 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143215/new/ https://reviews.llvm.org/D143215 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits