This is an archive of the discontinued LLVM Phabricator instance.

Differential D5882

Add Position-independent Code model Module API.
ClosedPublic

Authored by jhibbits on Oct 21 2014, 7:44 AM.

Details

Reviewers
echristo
• rafael
Commits
rG771c132e0f05: Add Position-independent Code model Module API.
rL221510: Add Position-independent Code model Module API.
Summary

This makes PIC levels a Module flag attribute, which can be queried by the
backend. The flag is named PIC Level, and can have a value of:

0 - Backend-default
1 - Small-model (-fpic)
2 - Large-model (-fPIC)

These match the `-pic-level' command line argument for clang, and the value of the
preprocessor macro `PIC'.

Diff Detail

Repository
rL LLVM

Event Timeline

jhibbits updated this revision to Diff 15189.Oct 21 2014, 7:44 AM
jhibbits retitled this revision from to Add Position-independent Code model Module API..
jhibbits updated this object.
jhibbits edited the test plan for this revision. (Show Details)
jhibbits added a reviewer: echristo.
jhibbits set the repository for this revision to rL LLVM.
jhibbits added a subscriber: Unknown Object (MLST).
• rafael added a subscriber: • rafael.Oct 21 2014, 9:03 AM

This should probably include a test showing what llvm-link does when merging these.

tools/llc/llc.cpp
290 ↗(On Diff #15189)

Is it intentional that any pre-existing metadata is always discarded?

jhibbits added a comment.Oct 21 2014, 11:01 AM
In D5882#5, @rafael wrote:

This should probably include a test showing what llvm-link does when merging these.

Thanks for the review. I'm updating the patch now, to also fix bugs I found (such as an error being thrown if I specify the command line, as well as the module flag, even if I change it to ModFlagBehavior::Override)

tools/llc/llc.cpp
290 ↗(On Diff #15189)

The intent is to allow you to override the module flag with the command line (-pic-level=small/large). So, really, this should have a check for if PLLevel != PICLevel::Default.

jhibbits updated this revision to Diff 15448.Oct 24 2014, 8:35 PM
jhibbits edited the test plan for this revision. (Show Details)

Update the patch to address Rafael's comments. Add some tests.

• rafael added inline comments.Oct 26 2014, 7:10 PM
test/Linker/module-flags-pic-1-a.ll
1 ↗(On Diff #15448)

This test has no CHECK line. You also need to pass %s to FileCheck

test/Linker/module-flags-pic-1-b.ll
1 ↗(On Diff #15448)

Please put this in test/Linker/Inputs. That way you don't need the dummy RUN line.

test/Linker/module-flags-pic-2-a.ll
2 ↗(On Diff #15448)

Why XFAIL?

jhibbits added a comment.Oct 27 2014, 12:38 PM

I'll upload a new diff this evening. Thanks!

test/Linker/module-flags-pic-1-a.ll
1 ↗(On Diff #15448)

Oops, wonder how this even passed the test run.

test/Linker/module-flags-pic-1-b.ll
1 ↗(On Diff #15448)

That definitely will make it cleaner.

test/Linker/module-flags-pic-2-a.ll
2 ↗(On Diff #15448)

This test is intended to fail (test merging two different values for !flag_pic). Is there a better way to do this?

• rafael added a comment.Oct 27 2014, 3:49 PM

rafael wrote:

Why XFAIL?

This test is intended to fail (test merging two different values for !flag_pic). Is there a better way to do this?

Yes, use the not utility (RUN: not llvm-link...). It is probably a
good idea to FileCheck for whatever error message we print.

Cheers,
Rafael

jhibbits updated this revision to Diff 15516.Oct 27 2014, 8:43 PM

Address Rafael's comments, updating the tests.

• rafael added a comment.Oct 29 2014, 10:26 PM

I think this is fine. We can always change the error to an merge logic in the future if needed. Eric, what do you think?

test/Linker/module-flags-pic-2-a.ll
10 ↗(On Diff #15516)

I think I fixed the "EROR: WARNING" in another patch. This should be just ERROR now.

jhibbits added a comment.Nov 4 2014, 11:02 AM

Ping?

jhibbits updated this object.Nov 4 2014, 11:10 AM
echristo edited edge metadata.Nov 4 2014, 12:00 PM

One comment inline.

lib/IR/Module.cpp
472 ↗(On Diff #15516)

One bike shed: "PIC Level" perhaps? since it's not just -fpic, but also -fPIC?

jhibbits added inline comments.Nov 4 2014, 12:30 PM
lib/IR/Module.cpp
472 ↗(On Diff #15516)

Works for me.

jhibbits updated this revision to Diff 15807.Nov 5 2014, 7:20 AM
jhibbits updated this object.
jhibbits edited the test plan for this revision. (Show Details)
jhibbits edited edge metadata.

Rename 'flag_pic' to 'PIC Level'

• rafael accepted this revision.Nov 5 2014, 7:31 AM
• rafael added a reviewer: • rafael.

LGTM

This revision is now accepted and ready to land.Nov 5 2014, 7:31 AM
echristo accepted this revision.Nov 5 2014, 10:00 AM
echristo edited edge metadata.

LGTM.

jhibbits closed this revision.Nov 6 2014, 8:57 PM
jhibbits updated this revision to Diff 15909.

Closed by commit rL221510 (authored by @jhibbits).

Revision Contents

PathSize
llvm/
trunk/
include/
llvm/
IR/
10 lines
Support/
4 lines
lib/
IR/
13 lines
test/
Linker/
Inputs/
1 line
3 lines
9 lines
10 lines

Diff 15909

llvm/trunk/include/llvm/IR/Module.h

Show All 17 Lines
#include "llvm/ADT/iterator_range.h" #include "llvm/ADT/iterator_range.h"
#include "llvm/IR/Comdat.h" #include "llvm/IR/Comdat.h"
#include "llvm/IR/DataLayout.h" #include "llvm/IR/DataLayout.h"
#include "llvm/IR/Function.h" #include "llvm/IR/Function.h"
#include "llvm/IR/GlobalAlias.h" #include "llvm/IR/GlobalAlias.h"
#include "llvm/IR/GlobalVariable.h" #include "llvm/IR/GlobalVariable.h"
#include "llvm/IR/Metadata.h" #include "llvm/IR/Metadata.h"
#include "llvm/Support/CBindingWrapping.h" #include "llvm/Support/CBindingWrapping.h"
#include "llvm/Support/CodeGen.h"
#include "llvm/Support/DataTypes.h" #include "llvm/Support/DataTypes.h"
#include <system_error> #include <system_error>
namespace llvm { namespace llvm {
class FunctionType; class FunctionType;
class GVMaterializer; class GVMaterializer;
class LLVMContext; class LLVMContext;
class RandomNumberGenerator; class RandomNumberGenerator;
▲ Show 20 Lines • Show All 595 Lines • ▼ Show 20 Lines
/// @} /// @}
/// @name Utility functions for querying Debug information. /// @name Utility functions for querying Debug information.
/// @{ /// @{
/// \brief Returns the Dwarf Version by checking module flags. /// \brief Returns the Dwarf Version by checking module flags.
unsigned getDwarfVersion() const; unsigned getDwarfVersion() const;
/// @} /// @}
/// @name Utility functions for querying and setting PIC level
/// @{
/// \brief Returns the PIC level (small or large model)
PICLevel::Level getPICLevel() const;
/// \brief Set the PIC level (small or large model)
void setPICLevel(PICLevel::Level PL);
/// @}
}; };
/// An raw_ostream inserter for modules. /// An raw_ostream inserter for modules.
inline raw_ostream &operator<<(raw_ostream &O, const Module &M) { inline raw_ostream &operator<<(raw_ostream &O, const Module &M) {
M.print(O, nullptr); M.print(O, nullptr);
return O; return O;
} }
Show All 13 Lines

llvm/trunk/include/llvm/Support/CodeGen.h

Show All 24 Lines namespace Reloc {
enum Model { Default, Static, PIC_, DynamicNoPIC }; enum Model { Default, Static, PIC_, DynamicNoPIC };
} }
// Code model types. // Code model types.
namespace CodeModel { namespace CodeModel {
enum Model { Default, JITDefault, Small, Kernel, Medium, Large }; enum Model { Default, JITDefault, Small, Kernel, Medium, Large };
} }
namespace PICLevel {
enum Level { Default=0, Small=1, Large=2 };
}
// TLS models. // TLS models.
namespace TLSModel { namespace TLSModel {
enum Model { enum Model {
GeneralDynamic, GeneralDynamic,
LocalDynamic, LocalDynamic,
InitialExec, InitialExec,
LocalExec LocalExec
}; };
▲ Show 20 Lines • Show All 51 Lines • Show Last 20 Lines

llvm/trunk/lib/IR/Module.cpp

Show First 20 Lines • Show All 453 Lines • ▼ Show 20 Lines
Comdat *Module::getOrInsertComdat(StringRef Name) { Comdat *Module::getOrInsertComdat(StringRef Name) {
Comdat C; Comdat C;
StringMapEntry<Comdat> &Entry = StringMapEntry<Comdat> &Entry =
ComdatSymTab.GetOrCreateValue(Name, std::move(C)); ComdatSymTab.GetOrCreateValue(Name, std::move(C));
Entry.second.Name = &Entry; Entry.second.Name = &Entry;
return &Entry.second; return &Entry.second;
} }
PICLevel::Level Module::getPICLevel() const {
Value *Val = getModuleFlag("PIC Level");
if (Val == NULL)
return PICLevel::Default;
return static_cast<PICLevel::Level>(cast<ConstantInt>(Val)->getZExtValue());
}
void Module::setPICLevel(PICLevel::Level PL) {
addModuleFlag(ModFlagBehavior::Error, "PIC Level", PL);
}

llvm/trunk/test/Linker/Inputs/module-flags-pic-1-b.ll

llvm/trunk/test/Linker/Inputs/module-flags-pic-2-b.ll

!0 = metadata !{ i32 1, metadata !"PIC Level", i32 2 }
!llvm.module.flags = !{!0}

llvm/trunk/test/Linker/module-flags-pic-1-a.ll

; RUN: llvm-link %s %p/Inputs/module-flags-pic-1-b.ll -S -o - | FileCheck %s
; test linking modules with specified and default PIC levels
!0 = metadata !{ i32 1, metadata !"PIC Level", i32 1 }
!llvm.module.flags = !{!0}
; CHECK: !llvm.module.flags = !{!0}
; CHECK: !0 = metadata !{i32 1, metadata !"PIC Level", i32 1}

llvm/trunk/test/Linker/module-flags-pic-2-a.ll

; RUN: not llvm-link %s %p/Inputs/module-flags-pic-2-b.ll -S -o - 2> %t
; RUN: FileCheck --check-prefix=CHECK-ERRORS < %t %s
; test linking modules with two different PIC levels
!0 = metadata !{ i32 1, metadata !"PIC Level", i32 1 }
!llvm.module.flags = !{!0}
; CHECK-ERRORS: ERROR: linking module flags 'PIC Level': IDs have conflicting values