[Lldb-commits] [PATCH] D32137: [Utility] Placate another GCC warning
This revision was automatically updated to reflect the committed changes. Closed by commit rL300845: [Utility] Placate another GCC warning. (authored by davide). Changed prior to commit: https://reviews.llvm.org/D32137?vs=95485=95961#toc Repository: rL LLVM https://reviews.llvm.org/D32137 Files: lldb/trunk/source/Plugins/Process/Utility/RegisterContextPOSIX_mips64.cpp Index: lldb/trunk/source/Plugins/Process/Utility/RegisterContextPOSIX_mips64.cpp === --- lldb/trunk/source/Plugins/Process/Utility/RegisterContextPOSIX_mips64.cpp +++ lldb/trunk/source/Plugins/Process/Utility/RegisterContextPOSIX_mips64.cpp @@ -55,9 +55,10 @@ m_registers_count[i] = reg_set_ptr->num_registers; } - assert(m_num_registers == m_registers_count[gpr_registers_count] + -m_registers_count[fpr_registers_count] + -m_registers_count[msa_registers_count]); + assert(m_num_registers == + static_cast(m_registers_count[gpr_registers_count] + + m_registers_count[fpr_registers_count] + + m_registers_count[msa_registers_count])); // elf-core yet to support ReadFPR() ProcessSP base = CalculateProcess(); Index: lldb/trunk/source/Plugins/Process/Utility/RegisterContextPOSIX_mips64.cpp === --- lldb/trunk/source/Plugins/Process/Utility/RegisterContextPOSIX_mips64.cpp +++ lldb/trunk/source/Plugins/Process/Utility/RegisterContextPOSIX_mips64.cpp @@ -55,9 +55,10 @@ m_registers_count[i] = reg_set_ptr->num_registers; } - assert(m_num_registers == m_registers_count[gpr_registers_count] + -m_registers_count[fpr_registers_count] + -m_registers_count[msa_registers_count]); + assert(m_num_registers == + static_cast(m_registers_count[gpr_registers_count] + + m_registers_count[fpr_registers_count] + + m_registers_count[msa_registers_count])); // elf-core yet to support ReadFPR() ProcessSP base = CalculateProcess(); ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32137: [Utility] Placate another GCC warning
davide added a comment. In https://reviews.llvm.org/D32137#731727, @labath wrote: > Thanks for digging into this, I've learned something new today. Fixing this > with a cast seems reasonable then. Me too, apparently C++ integer promotion is less obvious than I thought ;) https://reviews.llvm.org/D32137 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32137: [Utility] Placate another GCC warning
labath added a comment. Thanks for digging into this, I've learned something new today. Fixing this with a cast seems reasonable then. https://reviews.llvm.org/D32137 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32137: [Utility] Placate another GCC warning
sas accepted this revision. sas added inline comments. This revision is now accepted and ready to land. Comment at: source/Plugins/Process/Utility/RegisterContextPOSIX_mips64.cpp:59 + assert(m_num_registers == + static_cast(m_registers_count[gpr_registers_count] + + m_registers_count[fpr_registers_count] + If you're gonna go with casting, I think using the same type as `m_num_registers` makes more sense. assert(m_num_registers == static_cast(...)); https://reviews.llvm.org/D32137 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32137: [Utility] Placate another GCC warning
davide added a comment. Reference for the future (http://www.open-std.org/jtc1/sc22/open/n2356/conv.html [conv.prom1]) https://reviews.llvm.org/D32137 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32137: [Utility] Placate another GCC warning
davide added a subscriber: nlewycky. davide added a comment. And I was wrong. @nlewycky explained on IRC: 14:23 < nlewycky> gcc is correct, in 'char + char' each char gets promoted to 'int' first then summed, then you've got an unsigned int == int comparison 14:23 < nlewycky> uint8_t and unsigned char are also promoted to int (not unsigned int) before the addition https://reviews.llvm.org/D32137 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32137: [Utility] Placate another GCC warning
davide added a comment. I reduced it: https://gist.github.com/dcci/01c10b405041fa8d139a4f71aec102f7 https://reviews.llvm.org/D32137 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32137: [Utility] Placate another GCC warning
labath added a comment. I am confused. `m_registers_count` is declared as `uint8_t` at `RegisterContextPOSIX_mips64.h:67`... https://reviews.llvm.org/D32137 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D32137: [Utility] Placate another GCC warning
davide created this revision. For reference/discussion. GCC complains about signed-vs-unsigned comparison. I'm actually surprised that `m_registers_count` is a `signed` integer, as I can hardly imagine a negative register count. I'm under the impression that we could change `m_register_count` fields to be unsigned, but that would be a larger change. Thoughts? https://reviews.llvm.org/D32137 Files: source/Plugins/Process/Utility/RegisterContextPOSIX_mips64.cpp Index: source/Plugins/Process/Utility/RegisterContextPOSIX_mips64.cpp === --- source/Plugins/Process/Utility/RegisterContextPOSIX_mips64.cpp +++ source/Plugins/Process/Utility/RegisterContextPOSIX_mips64.cpp @@ -55,9 +55,10 @@ m_registers_count[i] = reg_set_ptr->num_registers; } - assert(m_num_registers == m_registers_count[gpr_registers_count] + -m_registers_count[fpr_registers_count] + -m_registers_count[msa_registers_count]); + assert(m_num_registers == + static_cast(m_registers_count[gpr_registers_count] + + m_registers_count[fpr_registers_count] + + m_registers_count[msa_registers_count])); // elf-core yet to support ReadFPR() ProcessSP base = CalculateProcess(); Index: source/Plugins/Process/Utility/RegisterContextPOSIX_mips64.cpp === --- source/Plugins/Process/Utility/RegisterContextPOSIX_mips64.cpp +++ source/Plugins/Process/Utility/RegisterContextPOSIX_mips64.cpp @@ -55,9 +55,10 @@ m_registers_count[i] = reg_set_ptr->num_registers; } - assert(m_num_registers == m_registers_count[gpr_registers_count] + -m_registers_count[fpr_registers_count] + -m_registers_count[msa_registers_count]); + assert(m_num_registers == + static_cast(m_registers_count[gpr_registers_count] + + m_registers_count[fpr_registers_count] + + m_registers_count[msa_registers_count])); // elf-core yet to support ReadFPR() ProcessSP base = CalculateProcess(); ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
