This is an archive of the discontinued LLVM Phabricator instance.

MCAsmInfo: Allow targets to specify when the .section directive should be omitted
ClosedPublic

Authored by tstellarAMD on Aug 27 2015, 6:48 PM.

Details

Summary

The default behavior is to omit the .section directive for .text, .data,
and sometimes .bss, but some targets may want to omit this directive for
other sections too.

A future change to the AMDGPU backend will make use of this new feature
and have tests.

Diff Detail

Repository
rL LLVM

Event Timeline

tstellarAMD retitled this revision from to MCAsmInfo: Allow targets to specify when the .section directive should be omitted.
tstellarAMD updated this object.
tstellarAMD added a subscriber: llvm-commits.
rafael edited edge metadata.Aug 28 2015, 7:39 AM
rafael added a subscriber: rafael.

I don't think this is correct. If a target is not using
COFF/MachO/ELF, it should never create a
MCSectionCOFF/MCSectionMacho/MCSectionELF.

IMHO what is needed is something like r245047 (with the logic for
picking the triple from -march fixed). With that you can them make
sure that you never create a section if your object format doesn't
have one.

I don't think this is correct. If a target is not using
COFF/MachO/ELF, it should never create a
MCSectionCOFF/MCSectionMacho/MCSectionELF.

This isn't the issue here. What this patch does is give targets the opportunity to control how section switches are emitted. Using AMDGPU as an example, if the section directive is not omitted, section switches to .hsatext are printed like this:

.section .hsatext,#alloc,#execinstr,#write

If the section directive is emitted switches are printed like this:

.hsatext

The main reason I don't want the .section directive for .hsatext sections is because it has several custom STT_* flags, and I didn't really see a good way to add support for printing these custom flags.

Can you include that in the patch?

tstellarAMD edited edge metadata.

Improved explanation of how shouldOmitSectionDirective() affects assembly output.

Updated commit message to explain usage for AMDGPU backend. The new commit message looks like this:

The default behavior is to omit the .section directive for .text, .data,
and sometimes .bss, but some targets may want to omit this directive for
other sections too.

The AMDGPU backend will uses this to emit a simplified syntax for section
switches.  For example if the section directive is not omitted (current
behavior), section switches to .hsatext will be printed like this:

.section .hsatext,#alloc,#execinstr,#write

This is actually wrong, because .hsatext has some custom STT_* flags,
which MC doesn't know how to print or parse.

If the section directive is omitted (made possible by this commit),
section switches will be printed like this:

.hsatext

The motivation for this patch is to make it possible to emit sections
with custom STT_* flags without having to teach MC about all the target
specific STT_* flags.

No, what I meant is to include a change to lib/Target/AMDGPU showing
this in use (and a test).

This is the patch to make of use of this in the AMDPGU backend, and it also has tests: http://reviews.llvm.org/D12424

The AMDGPU patch is huge, but I can merge the two together if you want.

rafael accepted this revision.Sep 8 2015, 12:04 PM
rafael edited edge metadata.

Just noticed /D12424. Sorry for the confusion. LGTM for this one.

This revision is now accepted and ready to land.Sep 8 2015, 12:04 PM
This revision was automatically updated to reflect the committed changes.