Page MenuHomePhabricator

[MC] Pass the section to onSymbolStart() for more context
Needs ReviewPublic

Authored by rochauha on Jul 14 2020, 11:34 AM.

Details

Summary

This allows the disassembler to have more context regarding how to treat a particular symbol during disassembly.

Diff Detail

Event Timeline

rochauha created this revision.Jul 14 2020, 11:34 AM

Can you demonstrate how you are going to use the new interface?

llvm/include/llvm/MC/MCDisassembler/MCDisassembler.h
156

Pass object::SectionRef by value. It has 2 words, which is cheaper to copy.

Can you demonstrate how you are going to use the new interface?

The note section in AMDGPU ELF objects contains metadata. But we need to know whether the section is note section to deal with it.

Can you demonstrate how you are going to use the new interface?

The note section in AMDGPU ELF objects contains metadata. But we need to know whether the section is note section to deal with it.

I'm not sure I follow. Are you saying you need information from the note section to disassemble other sections? Or are you saying you need it to disassemble the note section? If the former, there's already code in the ELFDumper of llvm-readobj for parsing AMDGPU note sections, which you could reuse (it would need moving somewhere outside the llvm-readobj tool into one of the libraries, maybe in the ELFObjectFile stuff, I'm not sure). If the latter, there's still opposition to adding the functionality to the disassembler (the previous concerns haven't been addressed), given that llvm-readobj already provides this functionality.

Can you demonstrate how you are going to use the new interface?

The note section in AMDGPU ELF objects contains metadata. But we need to know whether the section is note section to deal with it.

I'm not sure I follow. Are you saying you need information from the note section to disassemble other sections? Or are you saying you need it to disassemble the note section? If the former, there's already code in the ELFDumper of llvm-readobj for parsing AMDGPU note sections, which you could reuse (it would need moving somewhere outside the llvm-readobj tool into one of the libraries, maybe in the ELFObjectFile stuff, I'm not sure). If the latter, there's still opposition to adding the functionality to the disassembler (the previous concerns haven't been addressed), given that llvm-readobj already provides this functionality.

There is AMDGPU metadata stored in the note section, which needs to be disassembled. The SectionRef passed here is just to check whether the section is a note section. There is no iterating over the note section.

This work doesn't fundamentally alter the the infrastructure. It just provides additional context to onSymbolStart (which some other target might probably find useful in the future too?).

Like @MaskRay, I'm going to see more usage of this to be able to properly comment on this change itself. However, one major point that needs highlighting is that this introduces a dependency on the Object library from the MC Disassembler, which doesn't currently exist. As far as I can tell, there are no references from MC or MCDisassembler to Object, but there are the other way around. That probably means this approach is a no-go. On the other hand, there is a SectionKind.h header file in MC, which might be an appropriate thing to use/extend if you need to identify the type of a section.