Skip to content

[X86] Respect code models more when determining if a global reference can fit in 32 bits #75386

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Dec 14, 2023
Merged

[X86] Respect code models more when determining if a global reference can fit in 32 bits #75386

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
55c27af
[X86] Respect code models more when determining if a global reference…
aeubanks Dec 13, 2023
e7fd77d
move aliasee check into TargetMachine::isLargeGlobalValue() since fol…
aeubanks Dec 13, 2023
57d3180
update comments
aeubanks Dec 14, 2023
303d6d1
format
aeubanks Dec 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion llvm/include/llvm/Target/TargetMachine.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ class TargetMachine {
void setCodeModel(CodeModel::Model CM) { CMModel = CM; }

void setLargeDataThreshold(uint64_t LDT) { LargeDataThreshold = LDT; }
bool isLargeGlobalObject(const GlobalObject *GO) const;
bool isLargeGlobalValue(const GlobalValue *GV) const;

bool isPositionIndependent() const;

Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -645,7 +645,7 @@ getELFSectionNameForGlobal(const GlobalObject *GO, SectionKind Kind,
Name = ".rodata.cst";
Name += utostr(EntrySize);
} else {
Name = getSectionPrefixForGlobal(Kind, TM.isLargeGlobalObject(GO));
Name = getSectionPrefixForGlobal(Kind, TM.isLargeGlobalValue(GO));
}

bool HasPrefix = false;
Expand Down Expand Up @@ -765,7 +765,7 @@ getGlobalObjectInfo(const GlobalObject *GO, const TargetMachine &TM) {
Group = C->getName();
IsComdat = C->getSelectionKind() == Comdat::Any;
}
if (TM.isLargeGlobalObject(GO))
if (TM.isLargeGlobalValue(GO))
Flags |= ELF::SHF_X86_64_LARGE;
return {Group, IsComdat, Flags};
}
Expand Down
8 changes: 7 additions & 1 deletion llvm/lib/Target/TargetMachine.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,16 @@ TargetMachine::TargetMachine(const Target &T, StringRef DataLayoutString,

TargetMachine::~TargetMachine() = default;

bool TargetMachine::isLargeGlobalObject(const GlobalObject *GO) const {
bool TargetMachine::isLargeGlobalValue(const GlobalValue *GVal) const {
if (getTargetTriple().getArch() != Triple::x86_64)
return false;

auto *GO = GVal->getAliaseeObject();

// Be conservative if we can't find an underlying GlobalObject.
if (!GO)
return true;

auto *GV = dyn_cast<GlobalVariable>(GO);

// Functions/GlobalIFuncs are only large under the large code model.
Expand Down
6 changes: 2 additions & 4 deletions llvm/lib/Target/X86/X86FastISel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -715,10 +715,8 @@ bool X86FastISel::handleConstantAddresses(const Value *V, X86AddressMode &AM) {
return false;

// Can't handle large objects yet.
if (auto *GO = dyn_cast<GlobalObject>(GV)) {
if (TM.isLargeGlobalObject(GO))
return false;
}
if (TM.isLargeGlobalValue(GV))
return false;

// Can't handle TLS yet.
if (GV->isThreadLocal())
Expand Down
21 changes: 14 additions & 7 deletions llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2927,6 +2927,10 @@ bool X86DAGToDAGISel::selectAddr(SDNode *Parent, SDValue N, SDValue &Base,
}

bool X86DAGToDAGISel::selectMOV64Imm32(SDValue N, SDValue &Imm) {
// Cannot use 32 bit constants to reference objects in kernel code model.
if (TM.getCodeModel() == CodeModel::Kernel)
return false;

// In static codegen with small code model, we can get the address of a label
// into a register with 'movl'
if (N->getOpcode() != X86ISD::Wrapper)
Expand All @@ -2940,15 +2944,18 @@ bool X86DAGToDAGISel::selectMOV64Imm32(SDValue N, SDValue &Imm) {
return false;

Imm = N;
if (N->getOpcode() != ISD::TargetGlobalAddress)
return TM.getCodeModel() == CodeModel::Small;
// Small/medium code model can reference non-TargetGlobalAddress objects with
// 32 bit constants.
if (N->getOpcode() != ISD::TargetGlobalAddress) {
return TM.getCodeModel() == CodeModel::Small ||
TM.getCodeModel() == CodeModel::Medium;
}

std::optional<ConstantRange> CR =
cast<GlobalAddressSDNode>(N)->getGlobal()->getAbsoluteSymbolRange();
if (!CR)
return TM.getCodeModel() == CodeModel::Small;
const GlobalValue *GV = cast<GlobalAddressSDNode>(N)->getGlobal();
if (std::optional<ConstantRange> CR = GV->getAbsoluteSymbolRange())
return CR->getUnsignedMax().ult(1ull << 32);

return CR->getUnsignedMax().ult(1ull << 32);
return !TM.isLargeGlobalValue(GV);
}

bool X86DAGToDAGISel::selectLEA64_32Addr(SDValue N, SDValue &Base,
Expand Down
16 changes: 8 additions & 8 deletions llvm/lib/Target/X86/X86Subtarget.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,16 +86,16 @@ X86Subtarget::classifyLocalReference(const GlobalValue *GV) const {
CodeModel::Model CM = TM.getCodeModel();
assert(CM != CodeModel::Tiny &&
"Tiny codesize model not supported on X86");
// In the large code model, even referencing a global under the large data
// threshold which is considered "small", we need to use GOTOFF.
// In the large code model, all text is far from any global data, so we
// use GOTOFF.
if (CM == CodeModel::Large)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this condition correct? It seems like it won't handle globals explicitly marked with code_model "small" correctly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even a small global in the large code model cannot be referenced rip-relatively since ltext is far from all data. So it needs to use GOTOFF, whether that's a 32-bit (for small data) or 64-bit (for large data) GOTOFF.

return X86II::MO_GOTOFF;
// Large objects use GOTOFF, otherwise use RIP-rel access.
if (auto *GO = dyn_cast_or_null<GlobalObject>(GV))
return TM.isLargeGlobalObject(GO) ? X86II::MO_GOTOFF
: X86II::MO_NO_FLAG;
// For non-GlobalObjects, the small and medium code models treat them as
// accessible with a RIP-rel access.
// Large GlobalValues use GOTOFF, otherwise use RIP-rel access.
if (GV)
return TM.isLargeGlobalValue(GV) ? X86II::MO_GOTOFF : X86II::MO_NO_FLAG;
// GV == nullptr is for all other non-GlobalValue global data like the
// constant pool, jump tables, labels, etc. The small and medium code
// models treat these as accessible with a RIP-rel access.
return X86II::MO_NO_FLAG;
}

Expand Down
Loading