bulbazord added inline comments.

================
Comment at: lldb/source/Expression/LLVMUserExpression.cpp:339
+      if (stack_frame_size == 0)
+        stack_frame_size = arch == llvm::Triple::msp430 ? 512 : 512 * 1024;
 
----------------
kuilpd wrote:
> bulbazord wrote:
> > jingham wrote:
> > > This doesn't seem appropriate in generic code.  You should ask the Arch 
> > > plugin for this.
> > Oh, I missed this in the previous patch. Yeah, we shouldn't be hardcoding 
> > `llvm::Triple::msp430` in generic code. You're introducing even more uses 
> > of this in this patch, we should abstract this out into a plugin like Jim 
> > suggested.
> Do you mean creating a new MSP430 plugin in 
> lldb/source/Plugins/Architecture/, or accessing the existing ABI plugin? 
> Should I create new methods for getting stack frame size in each existing 
> plugin?
You could probably put this in the ABI plugin? The base ABI class could have a 
virtual method like `GetStackFrameSize` that returns `512 * 1024` and the 
MSP430 subclass can override it and return `512` instead. That should work, I 
think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149262

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to