[llvm] [X86] Refine X86::isOffsetSuitableForCodeModel() (PR #75641)

Arthur Eubanks via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 18 13:15:50 PST 2023


https://github.com/aeubanks updated https://github.com/llvm/llvm-project/pull/75641

>From fd517b22c801c8091ed75c0b4b56192146538e2d Mon Sep 17 00:00:00 2001
From: Arthur Eubanks <aeubanks at google.com>
Date: Fri, 8 Dec 2023 11:20:22 -0800
Subject: [PATCH 1/6] [X86] Refine X86::isOffsetSuitableForCodeModel()

By treating the medium code model the same as the small code model for small data.

The small code model with large data is already broken (should be 64 bit
constants) so those changes don't matter.
---
 llvm/lib/Target/X86/X86ISelDAGToDAG.cpp |  6 ++--
 llvm/lib/Target/X86/X86ISelLowering.cpp | 38 +++++++++++++------------
 llvm/lib/Target/X86/X86ISelLowering.h   |  7 +++--
 llvm/test/CodeGen/X86/code-model-elf.ll | 19 ++++++-------
 4 files changed, 36 insertions(+), 34 deletions(-)

diff --git a/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp b/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
index 7ec59c74f5f58c..61c606a3433cfc 100644
--- a/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
+++ b/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
@@ -1736,11 +1736,9 @@ bool X86DAGToDAGISel::foldOffsetIntoAddress(uint64_t Offset,
   if (Val != 0 && (AM.ES || AM.MCSym))
     return true;
 
-  CodeModel::Model M = TM.getCodeModel();
   if (Subtarget->is64Bit()) {
-    if (Val != 0 &&
-        !X86::isOffsetSuitableForCodeModel(Val, M,
-                                           AM.hasSymbolicDisplacement()))
+    if (Val != 0 && !X86::isOffsetSuitableForCodeModel(
+                        Val, TM, AM.GV, AM.hasSymbolicDisplacement()))
       return true;
     // In addition to the checks required for a register base, check that
     // we do not try to use an unsafe Disp with a frame index.
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index b80c766c7ffa75..684f9a8c21715c 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -2674,34 +2674,35 @@ SDValue X86TargetLowering::getReturnAddressFrameIndex(SelectionDAG &DAG) const {
   return DAG.getFrameIndex(ReturnAddrIndex, getPointerTy(DAG.getDataLayout()));
 }
 
-bool X86::isOffsetSuitableForCodeModel(int64_t Offset, CodeModel::Model M,
-                                       bool hasSymbolicDisplacement) {
+bool X86::isOffsetSuitableForCodeModel(int64_t Offset, const TargetMachine &TM,
+                                       const GlobalValue *GV,
+                                       bool HasSymbolicDisplacement) {
   // Offset should fit into 32 bit immediate field.
   if (!isInt<32>(Offset))
     return false;
 
   // If we don't have a symbolic displacement - we don't have any extra
   // restrictions.
-  if (!hasSymbolicDisplacement)
+  if (!HasSymbolicDisplacement)
     return true;
 
-  // FIXME: Some tweaks might be needed for medium code model.
-  if (M != CodeModel::Small && M != CodeModel::Kernel)
+  // We can fold offsets in cases where we have a 64-bit relocation, but it
+  // doesn't really help.
+  if (TM.getCodeModel() == CodeModel::Large ||
+      (GV && TM.isLargeGlobalValue(GV)))
     return false;
 
-  // For small code model we assume that latest object is 16MB before end of 31
-  // bits boundary. We may also accept pretty large negative constants knowing
-  // that all objects are in the positive half of address space.
-  if (M == CodeModel::Small && Offset < 16*1024*1024)
-    return true;
-
   // For kernel code model we know that all object resist in the negative half
   // of 32bits address space. We may not accept negative offsets, since they may
   // be just off and we may accept pretty large positive ones.
-  if (M == CodeModel::Kernel && Offset >= 0)
-    return true;
+  if (TM.getCodeModel() == CodeModel::Kernel)
+    return Offset >= 0;
 
-  return false;
+  // For other non-large code models we assume that latest small object is 16MB
+  // before end of 31 bits boundary. We may also accept pretty large negative
+  // constants knowing that all objects are in the positive half of address
+  // space.
+  return Offset < 16 * 1024 * 1024;
 }
 
 /// Return true if the condition is an signed comparison operation.
@@ -18442,7 +18443,6 @@ SDValue X86TargetLowering::LowerGlobalOrExternal(SDValue Op, SelectionDAG &DAG,
   bool HasPICReg = isGlobalRelativeToPICBase(OpFlags);
   bool NeedsLoad = isGlobalStubReference(OpFlags);
 
-  CodeModel::Model M = DAG.getTarget().getCodeModel();
   auto PtrVT = getPointerTy(DAG.getDataLayout());
   SDValue Result;
 
@@ -18454,7 +18454,8 @@ SDValue X86TargetLowering::LowerGlobalOrExternal(SDValue Op, SelectionDAG &DAG,
     // relocation will compute to a negative value, which is invalid.
     int64_t GlobalOffset = 0;
     if (OpFlags == X86II::MO_NO_FLAG && Offset >= 0 &&
-        X86::isOffsetSuitableForCodeModel(Offset, M, true)) {
+        X86::isOffsetSuitableForCodeModel(Offset, getTargetMachine(), GV,
+                                          true)) {
       std::swap(GlobalOffset, Offset);
     }
     Result = DAG.getTargetGlobalAddress(GV, dl, PtrVT, GlobalOffset, OpFlags);
@@ -33518,8 +33519,9 @@ bool X86TargetLowering::isLegalAddressingMode(const DataLayout &DL,
   CodeModel::Model M = getTargetMachine().getCodeModel();
 
   // X86 allows a sign-extended 32-bit immediate field as a displacement.
-  if (!X86::isOffsetSuitableForCodeModel(AM.BaseOffs, M, AM.BaseGV != nullptr))
-    return false;
+  if (!X86::isOffsetSuitableForCodeModel(AM.BaseOffs, getTargetMachine(),
+                                         AM.BaseGV, AM.BaseGV != nullptr))
+  return false;
 
   if (AM.BaseGV) {
     unsigned GVFlags = Subtarget.classifyGlobalReference(AM.BaseGV);
diff --git a/llvm/lib/Target/X86/X86ISelLowering.h b/llvm/lib/Target/X86/X86ISelLowering.h
index 9bd1622cb0d3a6..5d3a16792046ea 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.h
+++ b/llvm/lib/Target/X86/X86ISelLowering.h
@@ -933,8 +933,11 @@ namespace llvm {
 
     /// Returns true of the given offset can be
     /// fit into displacement field of the instruction.
-    bool isOffsetSuitableForCodeModel(int64_t Offset, CodeModel::Model M,
-                                      bool hasSymbolicDisplacement);
+    /// GV can be null if not referencing a GlobalValue (e.g. constant pool,
+    /// jump table, etc.).
+    bool isOffsetSuitableForCodeModel(int64_t Offset, const TargetMachine &TM,
+                                      const GlobalValue *GV,
+                                      bool HasSymbolicDisplacement);
 
     /// Determines whether the callee is required to pop its
     /// own arguments. Callee pop is necessary to support tail calls.
diff --git a/llvm/test/CodeGen/X86/code-model-elf.ll b/llvm/test/CodeGen/X86/code-model-elf.ll
index 6112f2a57b82c3..53958757290132 100644
--- a/llvm/test/CodeGen/X86/code-model-elf.ll
+++ b/llvm/test/CodeGen/X86/code-model-elf.ll
@@ -385,8 +385,8 @@ define dso_local i32 @load_forced_small_data() #0 {
 ;
 ; MEDIUM-STATIC-LABEL: load_forced_small_data:
 ; MEDIUM-STATIC:       # %bb.0:
-; MEDIUM-STATIC-NEXT:    movl $forced_small_data, %eax
-; MEDIUM-STATIC-NEXT:    movl 8(%rax), %eax
+; MEDIUM-STATIC-NEXT:    movl $forced_small_data+8, %eax
+; MEDIUM-STATIC-NEXT:    movl (%rax), %eax
 ; MEDIUM-STATIC-NEXT:    retq
 ;
 ; LARGE-STATIC-LABEL: load_forced_small_data:
@@ -402,14 +402,12 @@ define dso_local i32 @load_forced_small_data() #0 {
 ;
 ; MEDIUM-SMALL-DATA-PIC-LABEL: load_forced_small_data:
 ; MEDIUM-SMALL-DATA-PIC:       # %bb.0:
-; MEDIUM-SMALL-DATA-PIC-NEXT:    leaq forced_small_data(%rip), %rax
-; MEDIUM-SMALL-DATA-PIC-NEXT:    movl 8(%rax), %eax
+; MEDIUM-SMALL-DATA-PIC-NEXT:    movl forced_small_data+8(%rip), %eax
 ; MEDIUM-SMALL-DATA-PIC-NEXT:    retq
 ;
 ; MEDIUM-PIC-LABEL: load_forced_small_data:
 ; MEDIUM-PIC:       # %bb.0:
-; MEDIUM-PIC-NEXT:    leaq forced_small_data(%rip), %rax
-; MEDIUM-PIC-NEXT:    movl 8(%rax), %eax
+; MEDIUM-PIC-NEXT:    movl forced_small_data+8(%rip), %eax
 ; MEDIUM-PIC-NEXT:    retq
 ;
 ; LARGE-PIC-LABEL: load_forced_small_data:
@@ -497,7 +495,8 @@ define dso_local ptr @lea_forced_large_data() #0 {
 define dso_local i32 @load_forced_large_data() #0 {
 ; SMALL-STATIC-LABEL: load_forced_large_data:
 ; SMALL-STATIC:       # %bb.0:
-; SMALL-STATIC-NEXT:    movl forced_large_data+8(%rip), %eax
+; SMALL-STATIC-NEXT:    movl $8, %eax
+; SMALL-STATIC-NEXT:    movl forced_large_data(%rax), %eax
 ; SMALL-STATIC-NEXT:    retq
 ;
 ; MEDIUM-STATIC-LABEL: load_forced_large_data:
@@ -515,7 +514,8 @@ define dso_local i32 @load_forced_large_data() #0 {
 ; SMALL-PIC-LABEL: load_forced_large_data:
 ; SMALL-PIC:       # %bb.0:
 ; SMALL-PIC-NEXT:    leaq _GLOBAL_OFFSET_TABLE_(%rip), %rax
-; SMALL-PIC-NEXT:    movl forced_large_data at GOTOFF+8(%rax), %eax
+; SMALL-PIC-NEXT:    movl $8, %ecx
+; SMALL-PIC-NEXT:    movl forced_large_data at GOTOFF(%rax,%rcx), %eax
 ; SMALL-PIC-NEXT:    retq
 ;
 ; MEDIUM-SMALL-DATA-PIC-LABEL: load_forced_large_data:
@@ -580,8 +580,7 @@ define dso_local i32 @load_global_data() #0 {
 ;
 ; MEDIUM-SMALL-DATA-PIC-LABEL: load_global_data:
 ; MEDIUM-SMALL-DATA-PIC:       # %bb.0:
-; MEDIUM-SMALL-DATA-PIC-NEXT:    leaq global_data(%rip), %rax
-; MEDIUM-SMALL-DATA-PIC-NEXT:    movl 8(%rax), %eax
+; MEDIUM-SMALL-DATA-PIC-NEXT:    movl global_data+8(%rip), %eax
 ; MEDIUM-SMALL-DATA-PIC-NEXT:    retq
 ;
 ; MEDIUM-PIC-LABEL: load_global_data:

>From 643cd3f97a8b0d4c253c7878d85d25dcea848c0d Mon Sep 17 00:00:00 2001
From: Arthur Eubanks <aeubanks at google.com>
Date: Mon, 18 Dec 2023 10:10:06 -0800
Subject: [PATCH 2/6] fix formatting

---
 llvm/lib/Target/X86/X86ISelLowering.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index 684f9a8c21715c..dc0915c376daba 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -33521,7 +33521,7 @@ bool X86TargetLowering::isLegalAddressingMode(const DataLayout &DL,
   // X86 allows a sign-extended 32-bit immediate field as a displacement.
   if (!X86::isOffsetSuitableForCodeModel(AM.BaseOffs, getTargetMachine(),
                                          AM.BaseGV, AM.BaseGV != nullptr))
-  return false;
+    return false;
 
   if (AM.BaseGV) {
     unsigned GVFlags = Subtarget.classifyGlobalReference(AM.BaseGV);

>From 2e5e462ca56094973a5a9d53f1791b6c28ec610e Mon Sep 17 00:00:00 2001
From: Arthur Eubanks <aeubanks at google.com>
Date: Fri, 15 Dec 2023 12:18:08 -0800
Subject: [PATCH 3/6] [X86] matchWrapper

---
 llvm/lib/Target/X86/X86ISelDAGToDAG.cpp       | 31 +++++++++----------
 llvm/test/CodeGen/X86/code-model-elf.ll       | 31 ++++++++++++-------
 .../CodeGen/X86/fast-isel-large-object.ll     |  5 +--
 3 files changed, 36 insertions(+), 31 deletions(-)

diff --git a/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp b/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
index 61c606a3433cfc..881be6bb586295 100644
--- a/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
+++ b/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
@@ -1826,9 +1826,7 @@ bool X86DAGToDAGISel::matchWrapper(SDValue N, X86ISelAddressMode &AM) {
   // That signifies access to globals that are known to be "near",
   // such as the GOT itself.
   CodeModel::Model M = TM.getCodeModel();
-  if (Subtarget->is64Bit() &&
-      ((M == CodeModel::Large && !IsRIPRelTLS) ||
-       (M == CodeModel::Medium && !IsRIPRel)))
+  if (Subtarget->is64Bit() && M == CodeModel::Large && !IsRIPRelTLS)
     return true;
 
   // Base and index reg must be 0 in order to use %rip as base.
@@ -1864,6 +1862,12 @@ bool X86DAGToDAGISel::matchWrapper(SDValue N, X86ISelAddressMode &AM) {
   } else
     llvm_unreachable("Unhandled symbol reference node.");
 
+  if (Subtarget->is64Bit() && !IsRIPRel && AM.GV &&
+      TM.isLargeGlobalValue(AM.GV)) {
+    AM = Backup;
+    return true;
+  }
+
   if (foldOffsetIntoAddress(Offset, AM)) {
     AM = Backup;
     return true;
@@ -1908,20 +1912,13 @@ bool X86DAGToDAGISel::matchAddress(SDValue N, X86ISelAddressMode &AM) {
 
   // Post-processing: Convert foo to foo(%rip), even in non-PIC mode,
   // because it has a smaller encoding.
-  // TODO: Which other code models can use this?
-  switch (TM.getCodeModel()) {
-    default: break;
-    case CodeModel::Small:
-    case CodeModel::Kernel:
-      if (Subtarget->is64Bit() &&
-          AM.Scale == 1 &&
-          AM.BaseType == X86ISelAddressMode::RegBase &&
-          AM.Base_Reg.getNode() == nullptr &&
-          AM.IndexReg.getNode() == nullptr &&
-          AM.SymbolFlags == X86II::MO_NO_FLAG &&
-          AM.hasSymbolicDisplacement())
-        AM.Base_Reg = CurDAG->getRegister(X86::RIP, MVT::i64);
-      break;
+  if (TM.getCodeModel() != CodeModel::Large &&
+      (!AM.GV || !TM.isLargeGlobalValue(AM.GV)) &&
+      Subtarget->is64Bit() && AM.Scale == 1 &&
+      AM.BaseType == X86ISelAddressMode::RegBase &&
+      AM.Base_Reg.getNode() == nullptr && AM.IndexReg.getNode() == nullptr &&
+      AM.SymbolFlags == X86II::MO_NO_FLAG && AM.hasSymbolicDisplacement()) {
+    AM.Base_Reg = CurDAG->getRegister(X86::RIP, MVT::i64);
   }
 
   return false;
diff --git a/llvm/test/CodeGen/X86/code-model-elf.ll b/llvm/test/CodeGen/X86/code-model-elf.ll
index 53958757290132..fdb5a38433f931 100644
--- a/llvm/test/CodeGen/X86/code-model-elf.ll
+++ b/llvm/test/CodeGen/X86/code-model-elf.ll
@@ -11,6 +11,16 @@
 ; RUN: llc -verify-machineinstrs < %s -relocation-model=pic    -code-model=large  | FileCheck %s --check-prefix=CHECK --check-prefix=LARGE-PIC
 ; RUN: llc -verify-machineinstrs < %s -relocation-model=pic    -code-model=large  -large-data-threshold=1000 | FileCheck %s --check-prefix=CHECK --check-prefix=LARGE-SMALL-DATA-PIC
 
+; Check that the relocations we emit are valid.
+; RUN: llc -verify-machineinstrs < %s -relocation-model=static -code-model=small  -filetype=obj -o /dev/null
+; RUN: llc -verify-machineinstrs < %s -relocation-model=static -code-model=medium -filetype=obj -o /dev/null
+; RUN: llc -verify-machineinstrs < %s -relocation-model=static -code-model=large  -filetype=obj -o /dev/null
+; RUN: llc -verify-machineinstrs < %s -relocation-model=pic    -code-model=small  -filetype=obj -o /dev/null
+; RUN: llc -verify-machineinstrs < %s -relocation-model=pic    -code-model=medium -large-data-threshold=1000 -filetype=obj -o /dev/null
+; RUN: llc -verify-machineinstrs < %s -relocation-model=pic    -code-model=medium -filetype=obj -o /dev/null
+; RUN: llc -verify-machineinstrs < %s -relocation-model=pic    -code-model=large  -filetype=obj -o /dev/null
+; RUN: llc -verify-machineinstrs < %s -relocation-model=pic    -code-model=large  -large-data-threshold=1000 -filetype=obj -o /dev/null
+
 ; Generated from this C source:
 ;
 ; static int static_data[10];
@@ -376,7 +386,6 @@ define dso_local ptr @lea_forced_small_data() #0 {
   ret ptr @forced_small_data
 }
 
-; TODO: make small and medium instruction sequence the same
 define dso_local i32 @load_forced_small_data() #0 {
 ; SMALL-STATIC-LABEL: load_forced_small_data:
 ; SMALL-STATIC:       # %bb.0:
@@ -385,8 +394,7 @@ define dso_local i32 @load_forced_small_data() #0 {
 ;
 ; MEDIUM-STATIC-LABEL: load_forced_small_data:
 ; MEDIUM-STATIC:       # %bb.0:
-; MEDIUM-STATIC-NEXT:    movl $forced_small_data+8, %eax
-; MEDIUM-STATIC-NEXT:    movl (%rax), %eax
+; MEDIUM-STATIC-NEXT:    movl forced_small_data+8(%rip), %eax
 ; MEDIUM-STATIC-NEXT:    retq
 ;
 ; LARGE-STATIC-LABEL: load_forced_small_data:
@@ -433,7 +441,6 @@ define dso_local i32 @load_forced_small_data() #0 {
   ret i32 %rv
 }
 
-; TODO: fix small code model instruction sequences to use 64-bit constants
 define dso_local ptr @lea_forced_large_data() #0 {
 ; SMALL-STATIC-LABEL: lea_forced_large_data:
 ; SMALL-STATIC:       # %bb.0:
@@ -452,8 +459,9 @@ define dso_local ptr @lea_forced_large_data() #0 {
 ;
 ; SMALL-PIC-LABEL: lea_forced_large_data:
 ; SMALL-PIC:       # %bb.0:
-; SMALL-PIC-NEXT:    leaq _GLOBAL_OFFSET_TABLE_(%rip), %rax
-; SMALL-PIC-NEXT:    leaq forced_large_data at GOTOFF(%rax), %rax
+; SMALL-PIC-NEXT:    leaq _GLOBAL_OFFSET_TABLE_(%rip), %rcx
+; SMALL-PIC-NEXT:    movabsq $forced_large_data at GOTOFF, %rax
+; SMALL-PIC-NEXT:    addq %rcx, %rax
 ; SMALL-PIC-NEXT:    retq
 ;
 ; MEDIUM-SMALL-DATA-PIC-LABEL: lea_forced_large_data:
@@ -495,8 +503,8 @@ define dso_local ptr @lea_forced_large_data() #0 {
 define dso_local i32 @load_forced_large_data() #0 {
 ; SMALL-STATIC-LABEL: load_forced_large_data:
 ; SMALL-STATIC:       # %bb.0:
-; SMALL-STATIC-NEXT:    movl $8, %eax
-; SMALL-STATIC-NEXT:    movl forced_large_data(%rax), %eax
+; SMALL-STATIC-NEXT:    movabsq $forced_large_data, %rax
+; SMALL-STATIC-NEXT:    movl 8(%rax), %eax
 ; SMALL-STATIC-NEXT:    retq
 ;
 ; MEDIUM-STATIC-LABEL: load_forced_large_data:
@@ -514,8 +522,8 @@ define dso_local i32 @load_forced_large_data() #0 {
 ; SMALL-PIC-LABEL: load_forced_large_data:
 ; SMALL-PIC:       # %bb.0:
 ; SMALL-PIC-NEXT:    leaq _GLOBAL_OFFSET_TABLE_(%rip), %rax
-; SMALL-PIC-NEXT:    movl $8, %ecx
-; SMALL-PIC-NEXT:    movl forced_large_data at GOTOFF(%rax,%rcx), %eax
+; SMALL-PIC-NEXT:    movabsq $forced_large_data at GOTOFF, %rcx
+; SMALL-PIC-NEXT:    movl 8(%rax,%rcx), %eax
 ; SMALL-PIC-NEXT:    retq
 ;
 ; MEDIUM-SMALL-DATA-PIC-LABEL: load_forced_large_data:
@@ -1126,8 +1134,7 @@ define dso_local float @load_constant_pool(float %x) #0 {
 ;
 ; MEDIUM-STATIC-LABEL: load_constant_pool:
 ; MEDIUM-STATIC:       # %bb.0:
-; MEDIUM-STATIC-NEXT:    movl ${{\.?LCPI[0-9]+_[0-9]+}}, %eax
-; MEDIUM-STATIC-NEXT:    addss (%rax), %xmm0
+; MEDIUM-STATIC-NEXT:    addss {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0
 ; MEDIUM-STATIC-NEXT:    retq
 ;
 ; LARGE-STATIC-LABEL: load_constant_pool:
diff --git a/llvm/test/CodeGen/X86/fast-isel-large-object.ll b/llvm/test/CodeGen/X86/fast-isel-large-object.ll
index 6ca2c424072379..9acdfdeaf7cc96 100644
--- a/llvm/test/CodeGen/X86/fast-isel-large-object.ll
+++ b/llvm/test/CodeGen/X86/fast-isel-large-object.ll
@@ -6,8 +6,9 @@
 define ptr @f() {
 ; CHECK-LABEL: f:
 ; CHECK:       # %bb.0:
-; CHECK-NEXT:    leaq _GLOBAL_OFFSET_TABLE_(%rip), %rax
-; CHECK-NEXT:    leaq g at GOTOFF(%rax), %rax
+; CHECK-NEXT:    leaq _GLOBAL_OFFSET_TABLE_(%rip), %rcx
+; CHECK-NEXT:    movabsq $g at GOTOFF, %rax
+; CHECK-NEXT:    addq %rcx, %rax
 ; CHECK-NEXT:    retq
   ret ptr @g
 }

>From 71edfbce844a72896ccff30c2cfab1ba5ac6a35e Mon Sep 17 00:00:00 2001
From: Arthur Eubanks <aeubanks at google.com>
Date: Mon, 18 Dec 2023 13:03:32 -0800
Subject: [PATCH 4/6] revert X86::isOffsetSuitableForCodeModel() signature
 change

and always fold offsets in large code model
---
 llvm/lib/Target/X86/X86ISelDAGToDAG.cpp |  2 +-
 llvm/lib/Target/X86/X86ISelLowering.cpp | 21 +++++++---------
 llvm/lib/Target/X86/X86ISelLowering.h   |  7 ++----
 llvm/test/CodeGen/X86/code-model-elf.ll | 32 ++++++++++++-------------
 4 files changed, 28 insertions(+), 34 deletions(-)

diff --git a/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp b/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
index 881be6bb586295..49bb279809096e 100644
--- a/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
+++ b/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
@@ -1738,7 +1738,7 @@ bool X86DAGToDAGISel::foldOffsetIntoAddress(uint64_t Offset,
 
   if (Subtarget->is64Bit()) {
     if (Val != 0 && !X86::isOffsetSuitableForCodeModel(
-                        Val, TM, AM.GV, AM.hasSymbolicDisplacement()))
+                        Val, TM.getCodeModel(), AM.hasSymbolicDisplacement()))
       return true;
     // In addition to the checks required for a register base, check that
     // we do not try to use an unsafe Disp with a frame index.
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index dc0915c376daba..f6385b75b1a3e1 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -2674,8 +2674,7 @@ SDValue X86TargetLowering::getReturnAddressFrameIndex(SelectionDAG &DAG) const {
   return DAG.getFrameIndex(ReturnAddrIndex, getPointerTy(DAG.getDataLayout()));
 }
 
-bool X86::isOffsetSuitableForCodeModel(int64_t Offset, const TargetMachine &TM,
-                                       const GlobalValue *GV,
+bool X86::isOffsetSuitableForCodeModel(int64_t Offset, CodeModel::Model CM,
                                        bool HasSymbolicDisplacement) {
   // Offset should fit into 32 bit immediate field.
   if (!isInt<32>(Offset))
@@ -2686,16 +2685,15 @@ bool X86::isOffsetSuitableForCodeModel(int64_t Offset, const TargetMachine &TM,
   if (!HasSymbolicDisplacement)
     return true;
 
-  // We can fold offsets in cases where we have a 64-bit relocation, but it
-  // doesn't really help.
-  if (TM.getCodeModel() == CodeModel::Large ||
-      (GV && TM.isLargeGlobalValue(GV)))
-    return false;
+  // We can fold large offsets in the large code model because we always use
+  // 64-bit offsets.
+  if (CM == CodeModel::Large)
+    return true;
 
   // For kernel code model we know that all object resist in the negative half
   // of 32bits address space. We may not accept negative offsets, since they may
   // be just off and we may accept pretty large positive ones.
-  if (TM.getCodeModel() == CodeModel::Kernel)
+  if (CM == CodeModel::Kernel)
     return Offset >= 0;
 
   // For other non-large code models we assume that latest small object is 16MB
@@ -18454,8 +18452,8 @@ SDValue X86TargetLowering::LowerGlobalOrExternal(SDValue Op, SelectionDAG &DAG,
     // relocation will compute to a negative value, which is invalid.
     int64_t GlobalOffset = 0;
     if (OpFlags == X86II::MO_NO_FLAG && Offset >= 0 &&
-        X86::isOffsetSuitableForCodeModel(Offset, getTargetMachine(), GV,
-                                          true)) {
+        X86::isOffsetSuitableForCodeModel(
+            Offset, getTargetMachine().getCodeModel(), true)) {
       std::swap(GlobalOffset, Offset);
     }
     Result = DAG.getTargetGlobalAddress(GV, dl, PtrVT, GlobalOffset, OpFlags);
@@ -33519,8 +33517,7 @@ bool X86TargetLowering::isLegalAddressingMode(const DataLayout &DL,
   CodeModel::Model M = getTargetMachine().getCodeModel();
 
   // X86 allows a sign-extended 32-bit immediate field as a displacement.
-  if (!X86::isOffsetSuitableForCodeModel(AM.BaseOffs, getTargetMachine(),
-                                         AM.BaseGV, AM.BaseGV != nullptr))
+  if (!X86::isOffsetSuitableForCodeModel(AM.BaseOffs, M, AM.BaseGV != nullptr))
     return false;
 
   if (AM.BaseGV) {
diff --git a/llvm/lib/Target/X86/X86ISelLowering.h b/llvm/lib/Target/X86/X86ISelLowering.h
index 5d3a16792046ea..9bd1622cb0d3a6 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.h
+++ b/llvm/lib/Target/X86/X86ISelLowering.h
@@ -933,11 +933,8 @@ namespace llvm {
 
     /// Returns true of the given offset can be
     /// fit into displacement field of the instruction.
-    /// GV can be null if not referencing a GlobalValue (e.g. constant pool,
-    /// jump table, etc.).
-    bool isOffsetSuitableForCodeModel(int64_t Offset, const TargetMachine &TM,
-                                      const GlobalValue *GV,
-                                      bool HasSymbolicDisplacement);
+    bool isOffsetSuitableForCodeModel(int64_t Offset, CodeModel::Model M,
+                                      bool hasSymbolicDisplacement);
 
     /// Determines whether the callee is required to pop its
     /// own arguments. Callee pop is necessary to support tail calls.
diff --git a/llvm/test/CodeGen/X86/code-model-elf.ll b/llvm/test/CodeGen/X86/code-model-elf.ll
index fdb5a38433f931..afcffb3a7adeda 100644
--- a/llvm/test/CodeGen/X86/code-model-elf.ll
+++ b/llvm/test/CodeGen/X86/code-model-elf.ll
@@ -399,8 +399,8 @@ define dso_local i32 @load_forced_small_data() #0 {
 ;
 ; LARGE-STATIC-LABEL: load_forced_small_data:
 ; LARGE-STATIC:       # %bb.0:
-; LARGE-STATIC-NEXT:    movl $forced_small_data, %eax
-; LARGE-STATIC-NEXT:    movl 8(%rax), %eax
+; LARGE-STATIC-NEXT:    movl $forced_small_data+8, %eax
+; LARGE-STATIC-NEXT:    movl (%rax), %eax
 ; LARGE-STATIC-NEXT:    retq
 ;
 ; SMALL-PIC-LABEL: load_forced_small_data:
@@ -503,20 +503,20 @@ define dso_local ptr @lea_forced_large_data() #0 {
 define dso_local i32 @load_forced_large_data() #0 {
 ; SMALL-STATIC-LABEL: load_forced_large_data:
 ; SMALL-STATIC:       # %bb.0:
-; SMALL-STATIC-NEXT:    movabsq $forced_large_data, %rax
-; SMALL-STATIC-NEXT:    movl 8(%rax), %eax
+; SMALL-STATIC-NEXT:    movabsq $forced_large_data+8, %rax
+; SMALL-STATIC-NEXT:    movl (%rax), %eax
 ; SMALL-STATIC-NEXT:    retq
 ;
 ; MEDIUM-STATIC-LABEL: load_forced_large_data:
 ; MEDIUM-STATIC:       # %bb.0:
-; MEDIUM-STATIC-NEXT:    movabsq $forced_large_data, %rax
-; MEDIUM-STATIC-NEXT:    movl 8(%rax), %eax
+; MEDIUM-STATIC-NEXT:    movabsq $forced_large_data+8, %rax
+; MEDIUM-STATIC-NEXT:    movl (%rax), %eax
 ; MEDIUM-STATIC-NEXT:    retq
 ;
 ; LARGE-STATIC-LABEL: load_forced_large_data:
 ; LARGE-STATIC:       # %bb.0:
-; LARGE-STATIC-NEXT:    movabsq $forced_large_data, %rax
-; LARGE-STATIC-NEXT:    movl 8(%rax), %eax
+; LARGE-STATIC-NEXT:    movabsq $forced_large_data+8, %rax
+; LARGE-STATIC-NEXT:    movl (%rax), %eax
 ; LARGE-STATIC-NEXT:    retq
 ;
 ; SMALL-PIC-LABEL: load_forced_large_data:
@@ -571,14 +571,14 @@ define dso_local i32 @load_global_data() #0 {
 ;
 ; MEDIUM-STATIC-LABEL: load_global_data:
 ; MEDIUM-STATIC:       # %bb.0:
-; MEDIUM-STATIC-NEXT:    movabsq $global_data, %rax
-; MEDIUM-STATIC-NEXT:    movl 8(%rax), %eax
+; MEDIUM-STATIC-NEXT:    movabsq $global_data+8, %rax
+; MEDIUM-STATIC-NEXT:    movl (%rax), %eax
 ; MEDIUM-STATIC-NEXT:    retq
 ;
 ; LARGE-STATIC-LABEL: load_global_data:
 ; LARGE-STATIC:       # %bb.0:
-; LARGE-STATIC-NEXT:    movabsq $global_data, %rax
-; LARGE-STATIC-NEXT:    movl 8(%rax), %eax
+; LARGE-STATIC-NEXT:    movabsq $global_data+8, %rax
+; LARGE-STATIC-NEXT:    movl (%rax), %eax
 ; LARGE-STATIC-NEXT:    retq
 ;
 ; SMALL-PIC-LABEL: load_global_data:
@@ -691,14 +691,14 @@ define dso_local i32 @load_unknown_size_data() #0 {
 ;
 ; MEDIUM-STATIC-LABEL: load_unknown_size_data:
 ; MEDIUM-STATIC:       # %bb.0:
-; MEDIUM-STATIC-NEXT:    movabsq $unknown_size_data, %rax
-; MEDIUM-STATIC-NEXT:    movl 8(%rax), %eax
+; MEDIUM-STATIC-NEXT:    movabsq $unknown_size_data+8, %rax
+; MEDIUM-STATIC-NEXT:    movl (%rax), %eax
 ; MEDIUM-STATIC-NEXT:    retq
 ;
 ; LARGE-STATIC-LABEL: load_unknown_size_data:
 ; LARGE-STATIC:       # %bb.0:
-; LARGE-STATIC-NEXT:    movabsq $unknown_size_data, %rax
-; LARGE-STATIC-NEXT:    movl 8(%rax), %eax
+; LARGE-STATIC-NEXT:    movabsq $unknown_size_data+8, %rax
+; LARGE-STATIC-NEXT:    movl (%rax), %eax
 ; LARGE-STATIC-NEXT:    retq
 ;
 ; SMALL-PIC-LABEL: load_unknown_size_data:

>From aaf5d934a9f64e5609dcc10dd312a55b49b3639a Mon Sep 17 00:00:00 2001
From: Arthur Eubanks <aeubanks at google.com>
Date: Mon, 18 Dec 2023 13:08:02 -0800
Subject: [PATCH 5/6] revert some unnecessary code changes

---
 llvm/lib/Target/X86/X86ISelDAGToDAG.cpp | 7 +++++--
 llvm/lib/Target/X86/X86ISelLowering.cpp | 4 ++--
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp b/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
index 49bb279809096e..affcfc8b41b78c 100644
--- a/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
+++ b/llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
@@ -1736,9 +1736,11 @@ bool X86DAGToDAGISel::foldOffsetIntoAddress(uint64_t Offset,
   if (Val != 0 && (AM.ES || AM.MCSym))
     return true;
 
+  CodeModel::Model M = TM.getCodeModel();
   if (Subtarget->is64Bit()) {
-    if (Val != 0 && !X86::isOffsetSuitableForCodeModel(
-                        Val, TM.getCodeModel(), AM.hasSymbolicDisplacement()))
+    if (Val != 0 &&
+        !X86::isOffsetSuitableForCodeModel(Val, M,
+                                           AM.hasSymbolicDisplacement()))
       return true;
     // In addition to the checks required for a register base, check that
     // we do not try to use an unsafe Disp with a frame index.
@@ -1862,6 +1864,7 @@ bool X86DAGToDAGISel::matchWrapper(SDValue N, X86ISelAddressMode &AM) {
   } else
     llvm_unreachable("Unhandled symbol reference node.");
 
+  // Can't use an addressing mode with large globals.
   if (Subtarget->is64Bit() && !IsRIPRel && AM.GV &&
       TM.isLargeGlobalValue(AM.GV)) {
     AM = Backup;
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index f6385b75b1a3e1..58d1d741fbcb8d 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -18441,6 +18441,7 @@ SDValue X86TargetLowering::LowerGlobalOrExternal(SDValue Op, SelectionDAG &DAG,
   bool HasPICReg = isGlobalRelativeToPICBase(OpFlags);
   bool NeedsLoad = isGlobalStubReference(OpFlags);
 
+  CodeModel::Model M = DAG.getTarget().getCodeModel();
   auto PtrVT = getPointerTy(DAG.getDataLayout());
   SDValue Result;
 
@@ -18452,8 +18453,7 @@ SDValue X86TargetLowering::LowerGlobalOrExternal(SDValue Op, SelectionDAG &DAG,
     // relocation will compute to a negative value, which is invalid.
     int64_t GlobalOffset = 0;
     if (OpFlags == X86II::MO_NO_FLAG && Offset >= 0 &&
-        X86::isOffsetSuitableForCodeModel(
-            Offset, getTargetMachine().getCodeModel(), true)) {
+        X86::isOffsetSuitableForCodeModel(Offset, M, true)) {
       std::swap(GlobalOffset, Offset);
     }
     Result = DAG.getTargetGlobalAddress(GV, dl, PtrVT, GlobalOffset, OpFlags);

>From 4d143e6b3011eb8749138daeb5fc6de0745f084b Mon Sep 17 00:00:00 2001
From: Arthur Eubanks <aeubanks at google.com>
Date: Mon, 18 Dec 2023 13:11:08 -0800
Subject: [PATCH 6/6] fix fold-add.ll

---
 llvm/test/CodeGen/X86/fold-add.ll | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/llvm/test/CodeGen/X86/fold-add.ll b/llvm/test/CodeGen/X86/fold-add.ll
index 597e51d877eb44..8c28d66597fb3b 100644
--- a/llvm/test/CodeGen/X86/fold-add.ll
+++ b/llvm/test/CodeGen/X86/fold-add.ll
@@ -45,8 +45,7 @@ define dso_local i64 @one() #0 {
 ;
 ; MSTATIC-LABEL: one:
 ; MSTATIC:       # %bb.0: # %entry
-; MSTATIC-NEXT:    movabsq $foo, %rax
-; MSTATIC-NEXT:    incq %rax
+; MSTATIC-NEXT:    movabsq $foo+1, %rax
 ; MSTATIC-NEXT:    retq
 ;
 ; MPIC-LABEL: one:



More information about the llvm-commits mailing list