Details
Diff Detail
Event Timeline
(please submit simple build fixes first and have them reviewed post commit, so that the bots don't stay red for long.)
Sorry, I didn't get "have them reviewed post commit", I'm new to llvm so can you give me some instruction on how to do it? Thank you.
Just commit the change, and if there are comments on the commit later, address then in a follow-up.
(please submit simple build fixes first and have them reviewed post commit, so that the bots don't stay red for long.)
And in this case the way I'd review it is to check it all out and build it so it amounts to the same thing (and saves me firing up a Windows machine).
LGTM regardless.
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.cpp | ||
---|---|---|
54 | On the assumption that riscv6 linux doesn't have a 32 bit compatibility mode, this is fine. (that's what this is trying to detect, for example running arm32 on arm64). |
After hours of trying to set up a working windows dev machine, I failed to reproduce the build failure you provided here (http://45.33.8.238/win/64255/step_4.txt). I am using a Windows 10 21H2 19044.1889 with VS 2022 and MSVC 143.
I guess it is the mocked dwarf PC register number that was converted to signed int32 and then implicitly converted to unsigned int32 where msvc reject this kind of conversion. You may need to check manually if this was the fix you want ( sorry for that :( ). If so, we land it immediately.
That bot uses clang-cl as compiler.
I can't easily check it either at the moment. It's easiest to land this and see if it helps.
Changing enum to enum : unsigned might (or might not) help as well.
We take a lot risk to land it without test. What if it does not help? Adding more commits until it is fixed?
It's _already broken_. There's zero risk to make it worse. (Assuming you checked it still builds on Linux.)
Yes, if it doesn't help you'd land another fix attempt.
(This is what the "if it takes a while" was for, though.)
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_riscv64.cpp | ||
---|---|---|
54 | (even if it does, I don't think we have to support that mode right away) |
On the assumption that riscv6 linux doesn't have a 32 bit compatibility mode, this is fine.
(that's what this is trying to detect, for example running arm32 on arm64).