[Lldb-commits] [PATCH] D143215: Separate Process::GetWatchpointSupportInfo into two methods to get the two separate pieces of information

[Emmmer S via Phabricator via lldb-commits]

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