jaydeep added a comment. In http://reviews.llvm.org/D12079#244059, @jaydeep wrote:
> In http://reviews.llvm.org/D12079#243390, @clayborg wrote: > > > In http://reviews.llvm.org/D12079#242998, @jaydeep wrote: > > > > > In http://reviews.llvm.org/D12079#242742, @clayborg wrote: > > > > > > > So DumpAddress() in FormatEntity.cpp is a generic "dump any address by > > > > describing it". You can't just change the code to suit your needs for > > > > MIPS. This address could be any address: code or data. If you want > > > > something that can take an address like 0x1000 and you ask for its > > > > AddressClass and it sees that its address class is > > > > eAddressClassCodeAlternateISA, and then you change it to be "0x1001", > > > > this will need to be a new format type. > > > > > > > > DumpAddress in FormatEntity.cpp is called for the following entities: > > > > > > > > case Entry::Type::LineEntryStartAddress: > > > > case Entry::Type::LineEntryEndAddress: > > > > case Entry::Type::AddressFile: > > > > case Entry::Type::AddressLoad: > > > > case Entry::Type::AddressLoadOrFile: > > > > case Entry::Type::FrameRegisterPC > > > > > > > > > > > > So only the LineEntry ones should actually do what you did. > > > > > > > > > We need to display all these entities in compressed address format. How > > > about a new MIPS specific function in Address and Target class which > > > would do this. > > > > > > Address Address::GetCallableAddress(Target *target); > > > lldb::addr_t Target::GetCallableAddress (lldb::addr_t load_addr, > > > AddressClass addr_class); > > > > > > We already have this in Target: > > > > lldb::addr_t > > GetCallableLoadAddress (lldb::addr_t load_addr, lldb::AddressClass > > addr_class = lldb::eAddressClassInvalid) const; > > > > > > So the solution here will be to modify Address::Dump() such that it detects > > when an address is eAddressClassCodeAlternateISA and when that happens it > > checks if the ExecutionContext parameter is non NULL, and if so, extract > > the target, and check the target's architecture is MIPS, then add the extra > > bit when displaying this address. As it seems that we would always want to > > describe a section offset address (lldb_private::Address object) in this > > way to show the MicroMIPS address space bit, right? > > > > > In http://reviews.llvm.org/D12079#243390, @clayborg wrote: > > > In http://reviews.llvm.org/D12079#242998, @jaydeep wrote: > > > > > In http://reviews.llvm.org/D12079#242742, @clayborg wrote: > > > > > > > So DumpAddress() in FormatEntity.cpp is a generic "dump any address by > > > > describing it". You can't just change the code to suit your needs for > > > > MIPS. This address could be any address: code or data. If you want > > > > something that can take an address like 0x1000 and you ask for its > > > > AddressClass and it sees that its address class is > > > > eAddressClassCodeAlternateISA, and then you change it to be "0x1001", > > > > this will need to be a new format type. > > > > > > > > DumpAddress in FormatEntity.cpp is called for the following entities: > > > > > > > > case Entry::Type::LineEntryStartAddress: > > > > case Entry::Type::LineEntryEndAddress: > > > > case Entry::Type::AddressFile: > > > > case Entry::Type::AddressLoad: > > > > case Entry::Type::AddressLoadOrFile: > > > > case Entry::Type::FrameRegisterPC > > > > > > > > > > > > So only the LineEntry ones should actually do what you did. > > > > > > > > > We need to display all these entities in compressed address format. How > > > about a new MIPS specific function in Address and Target class which > > > would do this. > > > > > > Address Address::GetCallableAddress(Target *target); > > > lldb::addr_t Target::GetCallableAddress (lldb::addr_t load_addr, > > > AddressClass addr_class); > > > > > > We already have this in Target: > > > > lldb::addr_t > > GetCallableLoadAddress (lldb::addr_t load_addr, lldb::AddressClass > > addr_class = lldb::eAddressClassInvalid) const; > > > > > > So the solution here will be to modify Address::Dump() such that it detects > > when an address is eAddressClassCodeAlternateISA and when that happens it > > checks if the ExecutionContext parameter is non NULL, and if so, extract > > the target, and check the target's architecture is MIPS, then add the extra > > bit when displaying this address. As it seems that we would always want to > > describe a section offset address (lldb_private::Address object) in this > > way to show the MicroMIPS address space bit, right? > > > Yes. Instead of modifying Address::Dump() we should modify DumpAddress() so that In http://reviews.llvm.org/D12079#244059, @jaydeep wrote: > In http://reviews.llvm.org/D12079#243390, @clayborg wrote: > > > In http://reviews.llvm.org/D12079#242998, @jaydeep wrote: > > > > > In http://reviews.llvm.org/D12079#242742, @clayborg wrote: > > > > > > > So DumpAddress() in FormatEntity.cpp is a generic "dump any address by > > > > describing it". You can't just change the code to suit your needs for > > > > MIPS. This address could be any address: code or data. If you want > > > > something that can take an address like 0x1000 and you ask for its > > > > AddressClass and it sees that its address class is > > > > eAddressClassCodeAlternateISA, and then you change it to be "0x1001", > > > > this will need to be a new format type. > > > > > > > > DumpAddress in FormatEntity.cpp is called for the following entities: > > > > > > > > case Entry::Type::LineEntryStartAddress: > > > > case Entry::Type::LineEntryEndAddress: > > > > case Entry::Type::AddressFile: > > > > case Entry::Type::AddressLoad: > > > > case Entry::Type::AddressLoadOrFile: > > > > case Entry::Type::FrameRegisterPC > > > > > > > > > > > > So only the LineEntry ones should actually do what you did. > > > > > > > > > We need to display all these entities in compressed address format. How > > > about a new MIPS specific function in Address and Target class which > > > would do this. > > > > > > Address Address::GetCallableAddress(Target *target); > > > lldb::addr_t Target::GetCallableAddress (lldb::addr_t load_addr, > > > AddressClass addr_class); > > > > > > We already have this in Target: > > > > lldb::addr_t > > GetCallableLoadAddress (lldb::addr_t load_addr, lldb::AddressClass > > addr_class = lldb::eAddressClassInvalid) const; > > > > > > So the solution here will be to modify Address::Dump() such that it detects > > when an address is eAddressClassCodeAlternateISA and when that happens it > > checks if the ExecutionContext parameter is non NULL, and if so, extract > > the target, and check the target's architecture is MIPS, then add the extra > > bit when displaying this address. As it seems that we would always want to > > describe a section offset address (lldb_private::Address object) in this > > way to show the MicroMIPS address space bit, right? > > > > > In http://reviews.llvm.org/D12079#243390, @clayborg wrote: > > > In http://reviews.llvm.org/D12079#242998, @jaydeep wrote: > > > > > In http://reviews.llvm.org/D12079#242742, @clayborg wrote: > > > > > > > So DumpAddress() in FormatEntity.cpp is a generic "dump any address by > > > > describing it". You can't just change the code to suit your needs for > > > > MIPS. This address could be any address: code or data. If you want > > > > something that can take an address like 0x1000 and you ask for its > > > > AddressClass and it sees that its address class is > > > > eAddressClassCodeAlternateISA, and then you change it to be "0x1001", > > > > this will need to be a new format type. > > > > > > > > DumpAddress in FormatEntity.cpp is called for the following entities: > > > > > > > > case Entry::Type::LineEntryStartAddress: > > > > case Entry::Type::LineEntryEndAddress: > > > > case Entry::Type::AddressFile: > > > > case Entry::Type::AddressLoad: > > > > case Entry::Type::AddressLoadOrFile: > > > > case Entry::Type::FrameRegisterPC > > > > > > > > > > > > So only the LineEntry ones should actually do what you did. > > > > > > > > > We need to display all these entities in compressed address format. How > > > about a new MIPS specific function in Address and Target class which > > > would do this. > > > > > > Address Address::GetCallableAddress(Target *target); > > > lldb::addr_t Target::GetCallableAddress (lldb::addr_t load_addr, > > > AddressClass addr_class); > > > > > > We already have this in Target: > > > > lldb::addr_t > > GetCallableLoadAddress (lldb::addr_t load_addr, lldb::AddressClass > > addr_class = lldb::eAddressClassInvalid) const; > > > > > > So the solution here will be to modify Address::Dump() such that it detects > > when an address is eAddressClassCodeAlternateISA and when that happens it > > checks if the ExecutionContext parameter is non NULL, and if so, extract > > the target, and check the target's architecture is MIPS, then add the extra > > bit when displaying this address. As it seems that we would always want to > > describe a section offset address (lldb_private::Address object) in this > > way to show the MicroMIPS address space bit, right? > > > Yes. Change in Address::Dump() would display microMIPS address only for Entry::Type::AddressLoadOrFile entity. (when "print_file_addr_or_load_addr" is true in DumpAddress()). However we would like to display microMIPS addresses for Entry::Type::AddressFile Entry::Type::AddressLoad Entry::Type::FrameRegisterPC Entry::Type::LineEntryStartAddress Entry::Type::LineEntryEndAddress entities as well (when "print_file_addr_or_load_addr" is false). The suggested change should be moved to DumpAddress(). Repository: rL LLVM http://reviews.llvm.org/D12079 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
