[all-commits] [llvm/llvm-project] 24e075: [lldb] Remove all the RegisterInfo name constifica...

Raphael Isemann via All-commits all-commits at lists.llvm.org
Tue Oct 13 08:11:08 PDT 2020


  Branch: refs/heads/master
  Home:   https://github.com/llvm/llvm-project
  Commit: 24e07570cc928b75e894b81639cabe96c660ccef
      https://github.com/llvm/llvm-project/commit/24e07570cc928b75e894b81639cabe96c660ccef
  Author: Raphael Isemann <teemperor at gmail.com>
  Date:   2020-10-13 (Tue, 13 Oct 2020)

  Changed paths:
    M lldb/include/lldb/Target/ABI.h
    M lldb/source/Plugins/ABI/ARM/ABIMacOSX_arm.cpp
    M lldb/source/Plugins/ABI/ARM/ABISysV_arm.cpp
    M lldb/source/Plugins/ABI/Hexagon/ABISysV_hexagon.cpp
    M lldb/source/Plugins/ABI/Mips/ABISysV_mips.cpp
    M lldb/source/Plugins/ABI/Mips/ABISysV_mips64.cpp
    M lldb/source/Plugins/ABI/SystemZ/ABISysV_s390x.cpp
    M lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.cpp
    M lldb/source/Plugins/Process/Utility/DynamicRegisterInfo.h
    M lldb/source/Target/ABI.cpp

  Log Message:
  -----------
  [lldb] Remove all the RegisterInfo name constification code

RegisterInfo's `reg_name`/`reg_alt_name` fields are C-Strings and are supposed
to only be generated from a ConstString. The reason for that is that
`DynamicRegisterInfo::GetRegisterInfo` and
`RegInfoBasedABI::GetRegisterInfoByName` try to optimise finding registers by
name by only comparing the C string pointer values instead of the underlying
strings. This only works if both C strings involved in the comparison come from
a ConstString. If one of the two C strings doesn't come from a ConstString the
comparison won't work (and most likely will silently fail).

I added an assert in b0060c3a7868ef026d95d0cf8a076791ef74f474 which checks that
both strings come from a ConstString. Apparently not all ABI plugins are
generating their register names via ConstString, so this code is now not just
silently failing but also asserting.

In D88375 we did a shady fix for the MIPS plugins by just copying the
ConstString setup code to that plugin, but we still need to fix ABISysV_arc,
ABISysV_ppc and ABISysV_ppc64 plugins.

I would say we just fix the remaining plugins by removing the whole requirement
to have the register names coming from ConstStrings. I really doubt that we
actually save any time with the whole ConstString search trick (searching ~50
strings that have <4 characters doesn't sound more expensive than calling the
really expensive ConstString constructor + comparing the same amount of pointer
values). Also whatever small percentage of LLDB's runtime is actually spend in
this function is anyway not worth the complexity of this approach.

This patch just removes all this and just does a normal string comparison.

Reviewed By: JDevlieghere, labath

Differential Revision: https://reviews.llvm.org/D88490




More information about the All-commits mailing list