This is an archive of the discontinued LLVM Phabricator instance.

Update API to add PIC-level support.
AbandonedPublic

Authored by jhibbits on Sep 12 2014, 9:50 AM.

Details

Summary

PowerPC and SPARC, possibly others, support multiple levels of PIC.
PowerPC specifically supports a 'small model' and 'large model', which changes
the behavior of the GOT. This introduces the necessary API changes to support
this distinction, and the PowerPC-specific bits will come in a future commit.

Diff Detail

Event Timeline

jhibbits updated this revision to Diff 13638.Sep 12 2014, 9:50 AM
jhibbits retitled this revision from to Update API to add PIC-level support..
jhibbits updated this object.
jhibbits edited the test plan for this revision. (Show Details)
jhibbits added a reviewer: echristo.
jhibbits added a subscriber: Unknown Object (MLST).
emaste added a subscriber: emaste.Sep 12 2014, 11:37 AM

Was suggested to add rafael to reviewers.

Can someone review this?

Just a few nits.

(If this isn't already being done in another patch) Can you add some amount of functionality, so this can be tested in some trivial way?

Unfortunately, I'm not the right person to sign off on this..

include/llvm/ExecutionEngine/ExecutionEngine.h
586

Please add a period.

include/llvm/MC/MCCodeGenInfo.h
41

Please remove extra whitespace before " );"

lib/Target/PowerPC/MCTargetDesc/PPCMCTargetDesc.cpp
112

No need for extra braces.

In D5332#9, @mcrosier wrote:

Just a few nits.

(If this isn't already being done in another patch) Can you add some amount of functionality, so this can be tested in some trivial way?

Unfortunately, I'm not the right person to sign off on this..

Thanks for reviewing. This is just the plumbing patch. The real meat (the reason I created this in the first place) is in D5399, adding -fpic (small-model PIC) to PowerPC. This is accompanied by tests.

This patch itself should have no impact on functionality, only on API usage, which D5400 demonstrates for Clang. So, really, D5400 and this belong together, since Clang breaks without the patch in D5400.

echristo edited edge metadata.Oct 6 2014, 12:21 AM

I'll look at this today.

So the general comment here is that the pic level should be module
dependent and merge according to whichever is "least pic-like". Can
you make this a module level attribute that's looked up and merge that
way?

At which point I'm not sure whether or not the plumbing you've done
here makes sense versus grabbing it otherwise.

Thoughts?

-eric

jhibbits abandoned this revision.Oct 21 2014, 7:45 AM

Abandoned in favor of D5882.

jhibbits reclaimed this revision.Oct 21 2014, 1:05 PM

After consensus on IRC, best to ressurrect this.

jhibbits abandoned this revision.Nov 8 2014, 10:06 PM