labath accepted this revision.
labath added inline comments.
================
Comment at:
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp:615
+ if (offset == LLDB_INVALID_XSAVE_OFFSET)
+ return false;
+
----------------
mgorny wrote:
> labath wrote:
> > When does this return false? When the ymm regs are not available?
> Yes. Note that this change brings us closer to being able to disable whole
> regsets — I just need to figure out how to do it ;-).
Ok, got it.
================
Comment at:
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h:35-38
+struct YMMSplitPtr {
+ void *xmm;
+ void *ymm_hi;
+};
----------------
Declare this inside the class (next to the function returning it), since it's
really a private thing.
================
Comment at:
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h:83
size_t GetDBROffset() const;
+ bool GetYMMSplitReg(uint32_t reg, void **xmm, void **ymm_hi);
};
----------------
labath wrote:
> mgorny wrote:
> > labath wrote:
> > > `void *&`
> > Are you sure about this? I feel like the whole `&` deal is quite confusing
> > (read: shouldn't have happened). You don't know whether the method modifies
> > the variables passed to it unless you look at the method prototype.
> well.. there certainly are code styles which prohibit non-const reference
> arguments. However, llvm is not one of them. Also lldb has a issue with
> compulsive null checks, so I think it's important to communicate the fact the
> two arguments cannot be null, which reference arguments do.
>
> Another possibility would be to just make this a real return value (pair<void
> *, void*> ?). The downside there is that it's not as self-documenting. Pick
> your poison...
Ok, that feels a bit over-engineered, but works too...
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91293/new/
https://reviews.llvm.org/D91293
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
