[PATCH] [mips] Add the following MIPS options that control gp-relative addressing of small data items: -mgpopt, -mlocal-sdata, -mextern-sdata.

Sasa Stankovic Sasa.Stankovic at imgtec.com
Thu Oct 9 10:15:04 PDT 2014


================
Comment at: lib/Target/Mips/MipsSubtarget.cpp:183-186
@@ -173,2 +182,6 @@
 
-  // Set UseSmallSection.
+  // -mabicalls defaults to false for targets other than Linux and NaCl.
+  if (!(IsLinux || isTargetNaCl())
+      && ((getFeatureBits() & Mips::FeatureNoABICalls) == 0))
+    NoABICalls = true;
+
----------------
dsanders wrote:
> This is unnecessary. ParseSubtargetFeatures() has already checked the feature bit and set NoABICalls to true, and Linux/NaCl get their behaviour from -mabicalls being the default (which can be overridden with -mno-abicalls).
> 
> I think you may be trying to make llc behave the same way as the clang driver. Is this correct? If so, I'm not sure this is a sensible direction. We'd end up with two implementations of the driver, one in the backend and one in the frontend. It would be preferable to pass the appropriate options in the testcases.
I added this to support non-Linux and non-NaCl targets (like mipsel-sde-elf), because clang driver currently doesn't support it. But I agree this is driver's job, so I removed it (and the similair check below).

================
Comment at: lib/Target/Mips/MipsSubtarget.cpp:194-209
@@ -176,3 +193,18 @@
   //       bare-metal.
-  UseSmallSection = !IsLinux && (TM->getRelocationModel() == Reloc::Static);
+  if (NoABICalls) {
+    if (!GPOpt)
+      SmallSectionThreshold = 0;
+    else if (SSThreshold.getNumOccurrences() > 0)
+      SmallSectionThreshold = SSThreshold;
+    else {
+      // If -mips-ssection-threshold is not specified, it defaults to 8 for
+      // targets other than Linux and NaCl.
+      SmallSectionThreshold = !(IsLinux || isTargetNaCl()) ? 8 : 0;
+    }
+  } else {
+    if (GPOpt)
+      errs() << "warning: cannot use small-data accesses for '-mabicalls'"
+             << "\n";
+    SmallSectionThreshold = 0;
+  }
 }
----------------
dsanders wrote:
> This seems more complicated than necessary. I don't think we need to test for Linux and NaCl since their behaviour is determined by the fact that they enable -mabicalls by default (which in turn disables $gp relative accesses). Also, why not have the default set to 8 and force the value to zero when !GPOpt or NoAbiCalls?
Done. (I actually restored the logic from initial implementation of the patch, and simplified it by removing the check for Linux and NaCl).

================
Comment at: lib/Target/Mips/MipsTargetObjectFile.cpp:53
@@ +52,3 @@
+bool MipsTargetObjectFile::IsInSmallSection(uint64_t Size) const {
+  return (Size > 0
+          && Size <= TM->getSubtarget<MipsSubtarget>()
----------------
dsanders wrote:
> Could you explain why zero-sized objects are excluded?
I don't know the reason for it. I followed gcc - there is a comment in file gcc/config/mips/mips.c, in function mips_in_small_data_p:

  /* We have traditionally not treated zero-sized objects as small data,
     so this is now effectively part of the ABI.  */
  return size > 0 && size <= mips_small_data_threshold;

================
Comment at: lib/Target/Mips/MipsTargetObjectFile.cpp:65
@@ -51,1 +64,3 @@
+  // definitions.
   if (GV->isDeclaration() || GV->hasAvailableExternallyLinkage())
+    return IsGlobalInSmallSectionImpl(GV, TM);
----------------
dsanders wrote:
> Are static globals allowed in the small section?
Do you mean globals defined in the same file? If so, they are.

================
Comment at: lib/Target/Mips/MipsTargetObjectFile.cpp:126-142
@@ +125,18 @@
+
+/// Return true if this constant should be placed into small data section.
+bool MipsTargetObjectFile::
+IsConstantInSmallSection(const Constant *CN, const TargetMachine &TM) const {
+  return (TM.getSubtarget<MipsSubtarget>().useSmallSection()
+          && LocalSData
+          && IsInSmallSection(TM.getSubtargetImpl()->getDataLayout()
+                              ->getTypeAllocSize(CN->getType())));
+}
+
+const MCSection *MipsTargetObjectFile::
+getSectionForConstant(SectionKind Kind, const Constant *C) const {
+  if (IsConstantInSmallSection(C, *TM))
+    return SmallDataSection;
+
+  // Otherwise, we work the same as ELF.
+  return TargetLoweringObjectFileELF::getSectionForConstant(Kind, C);
+}
----------------
dsanders wrote:
> Just a question: Are constants better off in .rodata where they are protected from accidental writes or .sdata?
I suppose they are. There is actually another MIPS-specific gcc option that can control this, -membedded-data (off by default), that if enabled places constants in .rodata. It also applies to read-only variables.

http://reviews.llvm.org/D4903






More information about the llvm-commits mailing list