[RISCV] Put data smaller than eight bytes to small data section

[shiva0217]

Summary

Because of gp = sdata_start_address + 0x800, gp with signed twelve-bit offset could covert most of the small data section. Linker relaxation could transfer the multiple data accessing instructions to a gp base with signed twelve-bit offset instruction.

[apazos]

lib/Target/RISCV/RISCVTargetObjectFile.cpp

39

It seems we force a default value (4 or 8) even when the threshold flag is off (value is 0)?
So how to disable the optimization?

[shiva0217]

Fix the issue for -G0 as Ana pointed out.

[shiva0217]

lib/Target/RISCV/RISCVTargetObjectFile.cpp

39

Hi Ana,
You're right. I've updated the revision to address the issue.
Thanks for the catch.

[shiva0217]

Correct the default small data limitation to 8 bytes for RV32 and RV64.
Setting the limitation to 0 for PIC.
The test case didn' add the testing line for PIC because we don't support pic yet, it will trigger "Unable to lowerGlobalAddress" error message.

[shiva0217]

Add testing for large code model.

[shiva0217]

Remove guard condition for large code model which will implement in clang.

[shiva0217]

Add getModuleMetadata() to read SmallDataLimit Module flag.

[apazos]

lib/Target/RISCV/RISCVTargetObjectFile.cpp

103

Here the flag value is overwritten by the module setting, so how can we use the flag?

[shiva0217]

lib/Target/RISCV/RISCVTargetObjectFile.cpp

103

We probably could remove riscv-ssection-threshold flag because front end won't pass the flag and emit the threshold by module flag.
Thanks for the catch.

[shiva0217]

Remove riscv-ssection-threshold flag because front end will pass the threshold by module flag.

[efriedma]

lib/Target/RISCV/RISCVTargetObjectFile.cpp

17

This isn't thread-safe.

67

We never put variables with internal linkage into a small data section? That seems a little weird.

[apazos]

lib/Target/RISCV/RISCVTargetObjectFile.cpp

17

We can put it in RISCVELFTargetObjectFile just like SmallDataSection and SmallBSSSection

[shiva0217]

lib/Target/RISCV/RISCVTargetObjectFile.cpp

17

Hi Eli and Ana, thanks for the guidance, I'll put SSThreshold in RISCVELFTargetObjectFile.

67

We should handle internal linkage as well, thanks for the catch.

[shiva0217]

Update patch to address the comments from Eli and Ana.

[asb]

Many thanks for this. I've added some minor formatting and comment phrasing nits, but otherwise this is looking good to me.

lib/CodeGen/AsmPrinter/AsmPrinter.cpp

260

clang-format prefers const_cast<TargetLoweringObjectFile &>

lib/Target/RISCV/RISCVTargetObjectFile.cpp

97

Style: should be auto &MFE : ModuleFlags

test/CodeGen/RISCV/sdata-limit-0.ll

7 ↗

(On Diff #192789)

"so we expect no data will be put in sbss or sdata."

test/CodeGen/RISCV/sdata-limit-4.ll

7 ↗

(On Diff #192789)

"@v will be put in sbss, but @r won't be put in sdata"

test/CodeGen/RISCV/sdata-limit-8.ll

7 ↗

(On Diff #192789)

"@v will be put in sbss and @r will be put in sdata"

test/CodeGen/RISCV/sdata-local-sym.ll

8 ↗

(On Diff #192789)

"@v will b put in sbss and @r will be put in sdata"

[shiva0217]

Update patch to address Alex's comments.
Hi Alex, thanks for the review.

include/llvm/Target/TargetLoweringObjectFile.h

public:
   virtual void Initialize(MCContext &ctx, const TargetMachine &TM);
 
   virtual void emitPersonalityValue(MCStreamer &Streamer, const DataLayout &TM,
                                     const MCSymbol *Sym) const;
 
   /// Emit the module-level metadata that the platform cares about.
   virtual void emitModuleMetadata(MCStreamer &Streamer, Module &M) const {}
 
+  /// Get the module-level metadata that the platform cares about.
+  virtual void getModuleMetadata(Module &M) const {}
+
   /// Given a constant with the SectionKind, return a section that it should be
   /// placed in.
   virtual MCSection *getSectionForConstant(const DataLayout &DL,
                                            SectionKind Kind,
                                            const Constant *C,
                                            unsigned &Align) const;
 
   /// Classify the specified global variable into a set of target independent

lib/CodeGen/AsmPrinter/AsmPrinter.cpp

 
 bool AsmPrinter::doInitialization(Module &M) {
   MMI = getAnalysisIfAvailable<MachineModuleInfo>();
 
   // Initialize TargetLoweringObjectFile.
   const_cast<TargetLoweringObjectFile&>(getObjFileLowering())
     .Initialize(OutContext, TM);
 
+  getObjFileLowering().getModuleMetadata(M);
+
   OutStreamer->InitSections(false);
 
   // Emit the version-min deployment target directive if needed.
   //
   // FIXME: If we end up with a collection of these sorts of Darwin-specific
   // or ELF-specific things, it may make sense to have a platform helper class
   // that will work with the target helper class. For now keep it here, as the
   // alternative is duplicated code in each of the target asm printers that

lib/Target/RISCV/RISCVTargetObjectFile.h

 
 #include "llvm/CodeGen/TargetLoweringObjectFileImpl.h"
 
 namespace llvm {
 class RISCVTargetMachine;
 
 /// This implementation is used for RISCV ELF targets.
 class RISCVELFTargetObjectFile : public TargetLoweringObjectFileELF {
+  MCSection *SmallDataSection;
+  MCSection *SmallBSSSection;
+
+public:
   void Initialize(MCContext &Ctx, const TargetMachine &TM) override;
+
+  /// Return true if this global address should be placed into small data/bss
+  /// section.
+  bool isGlobalInSmallSection(const GlobalObject *GO,
+                              const TargetMachine &TM) const;
+
+  MCSection *SelectSectionForGlobal(const GlobalObject *GO, SectionKind Kind,
+                                    const TargetMachine &TM) const override;
+
+  /// Return true if this constant should be placed into small data section.
+  bool isConstantInSmallSection(const DataLayout &DL, const Constant *CN) const;
+
+  MCSection *getSectionForConstant(const DataLayout &DL, SectionKind Kind,
+                                   const Constant *C,
+                                   unsigned &Align) const override;
+
+  void getModuleMetadata(Module &M) const override;
 };
 
 } // end namespace llvm
 
 #endif

lib/Target/RISCV/RISCVTargetObjectFile.cpp

 //===-- RISCVTargetObjectFile.cpp - RISCV Object Info -----------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "RISCVTargetObjectFile.h"
 #include "RISCVTargetMachine.h"
+#include "llvm/BinaryFormat/ELF.h"
+#include "llvm/MC/MCContext.h"
+#include "llvm/MC/MCSectionELF.h"
 
 using namespace llvm;
 
+// riscv-ssection-threshold will be passed by clang if has -G flag.
+static cl::opt<unsigned> SSThreshold(
+    "riscv-ssection-threshold", cl::Hidden,
+    cl::desc("Small data and bss section threshold size (default=8)"),
+    cl::init(8));
+
 void RISCVELFTargetObjectFile::Initialize(MCContext &Ctx,
                                           const TargetMachine &TM) {
   TargetLoweringObjectFileELF::Initialize(Ctx, TM);
   InitializeELF(TM.Options.UseInitArray);
+
+  SmallDataSection = getContext().getELFSection(
+      ".sdata", ELF::SHT_PROGBITS, ELF::SHF_WRITE | ELF::SHF_ALLOC);
+  SmallBSSSection = getContext().getELFSection(".sbss", ELF::SHT_NOBITS,
+                                               ELF::SHF_WRITE | ELF::SHF_ALLOC);
+}
+
+// A address must be loaded from a small section if its size is less than the
+// small section size threshold. Data in this section could be addressed by
+// using gp_rel operator.
+static bool isInSmallSection(uint64_t Size) {
+  // gcc has traditionally not treated zero-sized objects as small data, so this
+  // is effectively part of the ABI.
+  return Size > 0 && Size <= SSThreshold;
+}
+
+// Return true if this global address should be placed into small data/bss
+// section.
+bool RISCVELFTargetObjectFile::isGlobalInSmallSection(
+    const GlobalObject *GO, const TargetMachine &TM) const {
+  // Only global variables, not functions.
+  const GlobalVariable *GVA = dyn_cast<GlobalVariable>(GO);
+  if (!GVA)
+    return false;
+
+  // If the variable has an explicit section, it is placed in that section.
+  if (GVA->hasSection()) {
+    StringRef Section = GVA->getSection();
+
+    // Explicitly placing any variable in the small data section overrides
+    // the global -G value.
+    if (Section == ".sdata" || Section == ".sbss")
+      return true;
+
+    // Otherwise reject putting the variable to small section if it has an
+    // explicit section name.
+    return false;
+  }
+
+  if (GVA->hasLocalLinkage())
+    return false;
+
+  if (((GVA->hasExternalLinkage() && GVA->isDeclaration()) ||
+       GVA->hasCommonLinkage()))
+    return false;
+
+  Type *Ty = GVA->getValueType();
+  // It is possible that the type of the global is unsized, i.e. a declaration
+  // of a extern struct. In this case don't presume it is in the small data
+  // section. This happens e.g. when building the FreeBSD kernel.
+  if (!Ty->isSized())
+    return false;
+
+  return isInSmallSection(
+      GVA->getParent()->getDataLayout().getTypeAllocSize(Ty));
+}
+
+MCSection *RISCVELFTargetObjectFile::SelectSectionForGlobal(
+    const GlobalObject *GO, SectionKind Kind, const TargetMachine &TM) const {
+  // Handle Small Section classification here.
+  if (Kind.isBSS() && isGlobalInSmallSection(GO, TM))
+    return SmallBSSSection;
+  if (Kind.isData() && isGlobalInSmallSection(GO, TM))
+    return SmallDataSection;
+
+  // Otherwise, we work the same as ELF.
+  return TargetLoweringObjectFileELF::SelectSectionForGlobal(GO, Kind, TM);
+}
+
+void RISCVELFTargetObjectFile::getModuleMetadata(Module &M) const {
+  SmallVector<Module::ModuleFlagEntry, 8> ModuleFlags;
+  M.getModuleFlagsMetadata(ModuleFlags);
+
+  for (const auto &MFE: ModuleFlags) {
+    StringRef Key = MFE.Key->getString();
+    if (Key == "SmallDataLimit") {
+      SSThreshold = mdconst::extract<ConstantInt>(MFE.Val)->getZExtValue();
+      break;
+    }
+  }
+}
+
+/// Return true if this constant should be placed into small data section.
+bool RISCVELFTargetObjectFile::isConstantInSmallSection(
+    const DataLayout &DL, const Constant *CN) const {
+  return isInSmallSection(DL.getTypeAllocSize(CN->getType()));
+}
+
+MCSection *RISCVELFTargetObjectFile::getSectionForConstant(
+    const DataLayout &DL, SectionKind Kind, const Constant *C,
+    unsigned &Align) const {
+  if (isConstantInSmallSection(DL, C))
+    return SmallDataSection;
+
+  // Otherwise, we work the same as ELF.
+  return TargetLoweringObjectFileELF::getSectionForConstant(DL, Kind, C, Align);
 }

test/CodeGen/RISCV/sdata-module-flag.ll

+; RUN: llc -mtriple=riscv32 < %s | FileCheck -check-prefix=RV32 %s
+; RUN: llc -mtriple=riscv64 < %s | FileCheck -check-prefix=RV64 %s
+
+@v = dso_local global i32 0, align 4
+@r = dso_local global i64 7, align 8
+
+define i32 @test(i32 %k) {
+entry:
+  %k.addr = alloca i32, align 4
+  store i32 %k, i32* %k.addr, align 4
+  %0 = load i32, i32* %k.addr, align 4
+  %1 = load i32, i32* @v, align 4
+  %add = add nsw i32 %0, %1
+  %conv = sext i32 %add to i64
+  %2 = load i64, i64* @r, align 8
+  %add1 = add nsw i64 %conv, %2
+  %conv2 = trunc i64 %add1 to i32
+  ret i32 %conv2
+}
+
+; SmallDataLimit set to 4, so we expect @v will put in sbss,
+; but @r won't put in sdata
+!llvm.module.flags = !{!0}
+!0 = !{i32 1, !"SmallDataLimit", i32 4}
+
+; RV32:    .section        .sbss
+; RV32-NOT:    .section        .sdata
+; RV64:    .section        .sbss
+; RV64-NOT:    .section        .sdata

test/CodeGen/RISCV/sdata.ll

+; RUN: llc -mtriple=riscv32 < %s | FileCheck -check-prefix=RV32 %s
+; RUN: llc -mtriple=riscv32 -riscv-ssection-threshold=0 < %s | FileCheck -check-prefix=RV32-G0 %s
+; RUN: llc -mtriple=riscv64 < %s | FileCheck -check-prefix=RV64 %s
+; RUN: llc -mtriple=riscv64 -riscv-ssection-threshold=0 < %s | FileCheck -check-prefix=RV64-G0 %s
+
+@v = dso_local global i32 0, align 4
+@r = dso_local global i64 7, align 8
+
+define i32 @test(i32 %k) {
+entry:
+  %k.addr = alloca i32, align 4
+  store i32 %k, i32* %k.addr, align 4
+  %0 = load i32, i32* %k.addr, align 4
+  %1 = load i32, i32* @v, align 4
+  %add = add nsw i32 %0, %1
+  %conv = sext i32 %add to i64
+  %2 = load i64, i64* @r, align 8
+  %add1 = add nsw i64 %conv, %2
+  %conv2 = trunc i64 %add1 to i32
+  ret i32 %conv2
+}
+
+; RV32:    .section        .sbss
+; RV32:    .section        .sdata
+; RV32-G0-NOT:    .section        .sbss
+; RV32-G0-NOT:    .section        .sdata
+; RV64:    .section        .sbss
+; RV64:    .section        .sdata
+; RV64-G0-NOT:    .section        .sbss
+; RV64-G0-NOT:    .section        .sdata