This is an archive of the discontinued LLVM Phabricator instance.

Start setting dso_local in clang
ClosedPublic

Authored by espindola on Dec 15 2017, 5:18 PM.

Details

Summary

This starts adding dso_local to clang.

The hope is to eventually have TargetMachine::shouldAssumeDsoLocal go away. My objective for now is to move enough of it to clang to remove the need for the TargetMachine one to handle PIE copy relocations and -fno-plt. With that it should then be easy to implement a -fno-copy-reloc in clang.

This patch just adds the cases where we assume a symbol to be local based on the file being compiled for an executable or a shared library.

Future patches will add the rest of the ELF logic (visibility mostly) and the other file formats.

Diff Detail

Event Timeline

rafael created this revision.Dec 15 2017, 5:18 PM
rnk added inline comments.Dec 18 2017, 2:07 PM
lib/CodeGen/CodeGenModule.cpp
690–692 ↗(On Diff #127217)

Handling COFF here is probably trivial. Everything is dso_local unless it's dllimport. Does that matter, or is dso_local intended to be an ELF-specific annotation?

rafael edited the summary of this revision. (Show Details)Dec 19 2017, 7:43 AM
rnk accepted this revision.Dec 22 2017, 1:02 PM

Looks good, sorry for the holiday delay.

This revision is now accepted and ready to land.Dec 22 2017, 1:02 PM
espindola commandeered this revision.Jan 17 2018, 7:20 PM
espindola added a reviewer: rafael.

All tests have been updated.

A few missing cases in c++ codegen are handled.
We now use CodeGenOpts.RelocationModel, which is the same value that llvm uses. This also has the advantage that clang_cc1 default to pic, so there is a lot less tests to update.

Sorry for the noise, but sending for review again since there is more code.

Rebased. Ping

espindola requested review of this revision.Jan 29 2018, 2:32 PM

Sorry, I missed that you wanted this reviewed again, I'll make sure to review it today.

sfertile added inline comments.Jan 31 2018, 7:47 AM
clang/lib/CodeGen/CodeGenModule.cpp
740

I don't think this is the case. I think this would break ppc, where we need to restore the toc pointer after the plt stubs returns to the original call site.

sfertile accepted this revision.Feb 2 2018, 8:00 AM

LGTM

This revision is now accepted and ready to land.Feb 2 2018, 8:00 AM