DavidSpickett wrote: > I was (and still am) confused of RegisterContextFreeBSD being used in other > architectures but arm/arm64, so I created conversion logic between > RegisterInfoPOSIX and FreeBSD's reg.h.
Me too tbf, no criticism from me on that. > I understand PR should be created at least once build succeeds, but I wasn't > sure if using the conversion logic was correct approach so I wanted to hear > others' feedback on this Yeah that's a fine thing to do. And you put your specific concern in a comment for the reviewers to see - so many people forget to do this. > (And as expected, this seems to be a wrong approach). I'm not sure it is, it just needs justifying. So what I recommend is to look at one of the architectures that does not need this conversion and see what layers it has. Perhaps it would have needed this conversion if they'd taken your approach, perhaps they don't need the conversion because FreeBSD and Linux (and Darwin? seems unlikely) line up. If the other architectures also tackled this problem, but by adding more specialised classes, I think I'd prefer the extra compile time over the extra runtime work of conversion. But anyway let's see what you discover. If you do need help with Linux AArch64 details, let me know. > After finding a correct approach from feedback (presumably creating > RegisterContextFreeBSD for riscv64) Might be one of many ways of being correct, let's see what the evidence shows us. > I'll close this PR and reopen it once testing is done if leaving this open > during testing isn't allowed according to LLVM's policy. Leave the PR as is. It's clear to you, me, and anyone else who reads this what state it's in. You can do a draft if you like. Sometimes it is good to show new approaches as a separate PR though. Helps reviewers to be able to have the 2 side by side. https://github.com/llvm/llvm-project/pull/180549 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
