[llvm] r233187 - Fix fixup evaluation when deciding what to relocate with.

Rafael Espindola rafael.espindola at gmail.com
Wed Mar 25 06:16:54 PDT 2015


Author: rafael
Date: Wed Mar 25 08:16:53 2015
New Revision: 233187

URL: http://llvm.org/viewvc/llvm-project?rev=233187&view=rev
Log:
Fix fixup evaluation when deciding what to relocate with.

The previous logic was to first try without relocations at all
and failing that stop on the first defined symbol.

That was inefficient and incorrect in the case part of the
expression could be simplified and another part could not
(see included test).

We now stop the evaluation when we get to a variable whose value
can change (i.e. is weak).

Added:
    llvm/trunk/test/MC/X86/expand-var.s
Modified:
    llvm/trunk/include/llvm/MC/MCExpr.h
    llvm/trunk/include/llvm/MC/MCObjectWriter.h
    llvm/trunk/lib/MC/ELFObjectWriter.cpp
    llvm/trunk/lib/MC/MCAssembler.cpp
    llvm/trunk/lib/MC/MCExpr.cpp
    llvm/trunk/lib/MC/MCObjectWriter.cpp

Modified: llvm/trunk/include/llvm/MC/MCExpr.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCExpr.h?rev=233187&r1=233186&r2=233187&view=diff
==============================================================================
--- llvm/trunk/include/llvm/MC/MCExpr.h (original)
+++ llvm/trunk/include/llvm/MC/MCExpr.h Wed Mar 25 08:16:53 2015
@@ -61,8 +61,7 @@ protected:
   bool EvaluateAsRelocatableImpl(MCValue &Res, const MCAssembler *Asm,
                                  const MCAsmLayout *Layout,
                                  const MCFixup *Fixup,
-                                 const SectionAddrMap *Addrs, bool InSet,
-                                 bool ForceVarExpansion) const;
+                                 const SectionAddrMap *Addrs, bool InSet) const;
 
 public:
   /// @name Accessors
@@ -106,15 +105,6 @@ public:
   bool EvaluateAsRelocatable(MCValue &Res, const MCAsmLayout *Layout,
                              const MCFixup *Fixup) const;
 
-  /// \brief Try to evaluate the expression to the form (a - b + constant) where
-  /// neither a nor b are variables.
-  ///
-  /// This is a more aggressive variant of EvaluateAsRelocatable. The intended
-  /// use is for when relocations are not available, like the symbol value in
-  /// the symbol table.
-  bool EvaluateAsValue(MCValue &Res, const MCAsmLayout *Layout,
-                       const MCFixup *Fixup) const;
-
   /// FindAssociatedSection - Find the "associated section" for this expression,
   /// which is currently defined as the absolute section for constants, or
   /// otherwise the section associated with the first defined symbol in the

Modified: llvm/trunk/include/llvm/MC/MCObjectWriter.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCObjectWriter.h?rev=233187&r1=233186&r2=233187&view=diff
==============================================================================
--- llvm/trunk/include/llvm/MC/MCObjectWriter.h (original)
+++ llvm/trunk/include/llvm/MC/MCObjectWriter.h Wed Mar 25 08:16:53 2015
@@ -99,6 +99,11 @@ public:
                                          bool InSet,
                                          bool IsPCRel) const;
 
+  /// \brief True if this symbol (which is a variable) is weak. This is not
+  /// just STB_WEAK, but more generally whether or not we can evaluate
+  /// past it.
+  virtual bool isWeak(const MCSymbolData &SD) const;
+
   /// \brief Write the object file.
   ///
   /// This routine is called by the assembler after layout and relaxation is

Modified: llvm/trunk/lib/MC/ELFObjectWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/ELFObjectWriter.cpp?rev=233187&r1=233186&r2=233187&view=diff
==============================================================================
--- llvm/trunk/lib/MC/ELFObjectWriter.cpp (original)
+++ llvm/trunk/lib/MC/ELFObjectWriter.cpp Wed Mar 25 08:16:53 2015
@@ -312,6 +312,8 @@ class ELFObjectWriter : public MCObjectW
                                            bool InSet,
                                            bool IsPCRel) const override;
 
+    bool isWeak(const MCSymbolData &SD) const override;
+
     void WriteObject(MCAssembler &Asm, const MCAsmLayout &Layout) override;
     void writeSection(MCAssembler &Asm,
                       const SectionIndexMapTy &SectionIndexMap,
@@ -847,7 +849,7 @@ void ELFObjectWriter::RecordRelocation(M
           Fixup.getLoc(), "Cannot represent a difference across sections");
 
     const MCSymbolData &SymBD = Asm.getSymbolData(SymB);
-    if (isWeak(SymBD))
+    if (::isWeak(SymBD))
       Asm.getContext().FatalError(
           Fixup.getLoc(), "Cannot represent a subtraction with a weak symbol");
 
@@ -1817,12 +1819,16 @@ ELFObjectWriter::IsSymbolRefDifferenceFu
                                                       const MCFragment &FB,
                                                       bool InSet,
                                                       bool IsPCRel) const {
-  if (isWeak(DataA))
+  if (::isWeak(DataA))
     return false;
   return MCObjectWriter::IsSymbolRefDifferenceFullyResolvedImpl(
                                                  Asm, DataA, FB,InSet, IsPCRel);
 }
 
+bool ELFObjectWriter::isWeak(const MCSymbolData &SD) const {
+  return ::isWeak(SD);
+}
+
 MCObjectWriter *llvm::createELFObjectWriter(MCELFObjectTargetWriter *MOTW,
                                             raw_ostream &OS,
                                             bool IsLittleEndian) {

Modified: llvm/trunk/lib/MC/MCAssembler.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCAssembler.cpp?rev=233187&r1=233186&r2=233187&view=diff
==============================================================================
--- llvm/trunk/lib/MC/MCAssembler.cpp (original)
+++ llvm/trunk/lib/MC/MCAssembler.cpp Wed Mar 25 08:16:53 2015
@@ -142,7 +142,7 @@ static bool getSymbolOffsetImpl(const MC
 
   // If SD is a variable, evaluate it.
   MCValue Target;
-  if (!S.getVariableValue()->EvaluateAsValue(Target, &Layout, nullptr))
+  if (!S.getVariableValue()->EvaluateAsRelocatable(Target, &Layout, nullptr))
     report_fatal_error("unable to evaluate offset for variable '" +
                        S.getName() + "'");
 
@@ -188,7 +188,7 @@ const MCSymbol *MCAsmLayout::getBaseSymb
 
   const MCExpr *Expr = Symbol.getVariableValue();
   MCValue Value;
-  if (!Expr->EvaluateAsValue(Value, this, nullptr))
+  if (!Expr->EvaluateAsRelocatable(Value, this, nullptr))
     llvm_unreachable("Invalid Expression");
 
   const MCSymbolRefExpr *RefB = Value.getSymB();
@@ -473,18 +473,6 @@ const MCSymbolData *MCAssembler::getAtom
   return SD->getFragment()->getAtom();
 }
 
-// Try to fully compute Expr to an absolute value and if that fails produce
-// a relocatable expr.
-// FIXME: Should this be the behavior of EvaluateAsRelocatable itself?
-static bool evaluate(const MCExpr &Expr, const MCAsmLayout &Layout,
-                     const MCFixup &Fixup, MCValue &Target) {
-  if (Expr.EvaluateAsValue(Target, &Layout, &Fixup)) {
-    if (Target.isAbsolute())
-      return true;
-  }
-  return Expr.EvaluateAsRelocatable(Target, &Layout, &Fixup);
-}
-
 bool MCAssembler::evaluateFixup(const MCAsmLayout &Layout,
                                 const MCFixup &Fixup, const MCFragment *DF,
                                 MCValue &Target, uint64_t &Value) const {
@@ -494,7 +482,7 @@ bool MCAssembler::evaluateFixup(const MC
   // probably merge the two into a single callback that tries to evaluate a
   // fixup and records a relocation if one is needed.
   const MCExpr *Expr = Fixup.getValue();
-  if (!evaluate(*Expr, Layout, Fixup, Target))
+  if (!Expr->EvaluateAsRelocatable(Target, &Layout, &Fixup))
     getContext().FatalError(Fixup.getLoc(), "expected relocatable expression");
 
   bool IsPCRel = Backend.getFixupKindInfo(

Modified: llvm/trunk/lib/MC/MCExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCExpr.cpp?rev=233187&r1=233186&r2=233187&view=diff
==============================================================================
--- llvm/trunk/lib/MC/MCExpr.cpp (original)
+++ llvm/trunk/lib/MC/MCExpr.cpp Wed Mar 25 08:16:53 2015
@@ -432,8 +432,8 @@ bool MCExpr::evaluateAsAbsolute(int64_t
     return true;
   }
 
-  bool IsRelocatable = EvaluateAsRelocatableImpl(
-      Value, Asm, Layout, nullptr, Addrs, InSet, /*ForceVarExpansion*/ true);
+  bool IsRelocatable =
+      EvaluateAsRelocatableImpl(Value, Asm, Layout, nullptr, Addrs, InSet);
 
   // Record the current value.
   Res = Value.getConstant();
@@ -586,21 +586,21 @@ bool MCExpr::EvaluateAsRelocatable(MCVal
                                    const MCFixup *Fixup) const {
   MCAssembler *Assembler = Layout ? &Layout->getAssembler() : nullptr;
   return EvaluateAsRelocatableImpl(Res, Assembler, Layout, Fixup, nullptr,
-                                   false, /*ForceVarExpansion*/ false);
+                                   false);
 }
 
-bool MCExpr::EvaluateAsValue(MCValue &Res, const MCAsmLayout *Layout,
-                             const MCFixup *Fixup) const {
-  MCAssembler *Assembler = Layout ? &Layout->getAssembler() : nullptr;
-  return EvaluateAsRelocatableImpl(Res, Assembler, Layout, Fixup, nullptr,
-                                   false, /*ForceVarExpansion*/ true);
+static bool canExpand(const MCSymbol &Sym, const MCAssembler *Asm) {
+  if (!Asm)
+    return false;
+  const MCSymbolData &SD = Asm->getSymbolData(Sym);
+  return !Asm->getWriter().isWeak(SD);
 }
 
 bool MCExpr::EvaluateAsRelocatableImpl(MCValue &Res, const MCAssembler *Asm,
                                        const MCAsmLayout *Layout,
                                        const MCFixup *Fixup,
-                                       const SectionAddrMap *Addrs, bool InSet,
-                                       bool ForceVarExpansion) const {
+                                       const SectionAddrMap *Addrs,
+                                       bool InSet) const {
   ++stats::MCExprEvaluate;
 
   switch (getKind()) {
@@ -617,28 +617,23 @@ bool MCExpr::EvaluateAsRelocatableImpl(M
     const MCSymbol &Sym = SRE->getSymbol();
 
     // Evaluate recursively if this is a variable.
-    if (Sym.isVariable() && SRE->getKind() == MCSymbolRefExpr::VK_None) {
+    if (Sym.isVariable() && SRE->getKind() == MCSymbolRefExpr::VK_None &&
+        canExpand(Sym, Asm)) {
       if (Sym.getVariableValue()->EvaluateAsRelocatableImpl(
-              Res, Asm, Layout, Fixup, Addrs, true, ForceVarExpansion)) {
+              Res, Asm, Layout, Fixup, Addrs, true)) {
+        if (!SRE->hasSubsectionsViaSymbols())
+          return true;
+
         const MCSymbolRefExpr *A = Res.getSymA();
         const MCSymbolRefExpr *B = Res.getSymB();
-
-        if (SRE->hasSubsectionsViaSymbols()) {
-          // FIXME: This is small hack. Given
-          // a = b + 4
-          // .long a
-          // the OS X assembler will completely drop the 4. We should probably
-          // include it in the relocation or produce an error if that is not
-          // possible.
-          if (!A && !B)
-            return true;
-        } else {
-          if (ForceVarExpansion)
-            return true;
-          bool IsSymbol = A && A->getSymbol().isDefined();
-          if (!IsSymbol)
-            return true;
-        }
+        // FIXME: This is small hack. Given
+        // a = b + 4
+        // .long a
+        // the OS X assembler will completely drop the 4. We should probably
+        // include it in the relocation or produce an error if that is not
+        // possible.
+        if (!A && !B)
+          return true;
       }
     }
 
@@ -650,9 +645,8 @@ bool MCExpr::EvaluateAsRelocatableImpl(M
     const MCUnaryExpr *AUE = cast<MCUnaryExpr>(this);
     MCValue Value;
 
-    if (!AUE->getSubExpr()->EvaluateAsRelocatableImpl(Value, Asm, Layout,
-                                                      Fixup, Addrs, InSet,
-                                                      ForceVarExpansion))
+    if (!AUE->getSubExpr()->EvaluateAsRelocatableImpl(Value, Asm, Layout, Fixup,
+                                                      Addrs, InSet))
       return false;
 
     switch (AUE->getOpcode()) {
@@ -685,12 +679,10 @@ bool MCExpr::EvaluateAsRelocatableImpl(M
     const MCBinaryExpr *ABE = cast<MCBinaryExpr>(this);
     MCValue LHSValue, RHSValue;
 
-    if (!ABE->getLHS()->EvaluateAsRelocatableImpl(LHSValue, Asm, Layout,
-                                                  Fixup, Addrs, InSet,
-                                                  ForceVarExpansion) ||
-        !ABE->getRHS()->EvaluateAsRelocatableImpl(RHSValue, Asm, Layout,
-                                                  Fixup, Addrs, InSet,
-                                                  ForceVarExpansion))
+    if (!ABE->getLHS()->EvaluateAsRelocatableImpl(LHSValue, Asm, Layout, Fixup,
+                                                  Addrs, InSet) ||
+        !ABE->getRHS()->EvaluateAsRelocatableImpl(RHSValue, Asm, Layout, Fixup,
+                                                  Addrs, InSet))
       return false;
 
     // We only support a few operations on non-constant expressions, handle

Modified: llvm/trunk/lib/MC/MCObjectWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/MC/MCObjectWriter.cpp?rev=233187&r1=233186&r2=233187&view=diff
==============================================================================
--- llvm/trunk/lib/MC/MCObjectWriter.cpp (original)
+++ llvm/trunk/lib/MC/MCObjectWriter.cpp Wed Mar 25 08:16:53 2015
@@ -54,3 +54,5 @@ MCObjectWriter::IsSymbolRefDifferenceFul
   // On ELF and COFF  A - B is absolute if A and B are in the same section.
   return &SecA == &SecB;
 }
+
+bool MCObjectWriter::isWeak(const MCSymbolData &SD) const { return false; }

Added: llvm/trunk/test/MC/X86/expand-var.s
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/X86/expand-var.s?rev=233187&view=auto
==============================================================================
--- llvm/trunk/test/MC/X86/expand-var.s (added)
+++ llvm/trunk/test/MC/X86/expand-var.s Wed Mar 25 08:16:53 2015
@@ -0,0 +1,12 @@
+// RUN: llvm-mc -filetype=obj -triple x86_64-pc-linux < %s | llvm-readobj -r | FileCheck %s
+
+// CHECK:       Section (2) .rela.text {
+// CHECK-NEXT:    0x0 R_X86_64_32 d 0x0
+// CHECK-NEXT:  }
+
+a:
+        b = a
+        c = a
+        d = a
+        .weak d
+        .long d + (b - c)





More information about the llvm-commits mailing list