%hi($tmp) and %lo($tmp) relocations for Mips backend, where $tmp = sym1 - sym2

[Mark Seaborn]

On 11 December 2013 14:05, Sasa Stankovic <Sasa.Stankovic at rt-rk.com> wrote:

> +      // Check for expressions %hi(tmp) and %lo(tmp), where tmp =
> sym1-sym2.
>
+      if (Target.getSymA() != NULL &&
> Target.getSymA()->getSymbol().isVariable()
> +          && Target.getSymB() == NULL && Target.getConstant() == 0) {
> +        const MCExpr *SymValue = Target.getSymA()->getSymbol()
> +                                       .getVariableValue();
> +        const MCBinaryExpr *BE = dyn_cast<MCBinaryExpr>(SymValue);
> +
> +        if (BE && BE->getOpcode() == MCBinaryExpr::Sub) {
> +          const MCSymbolRefExpr *LHS =
> dyn_cast<MCSymbolRefExpr>(BE->getLHS());
> +          const MCSymbolRefExpr *RHS =
> dyn_cast<MCSymbolRefExpr>(BE->getRHS());
> +          if (LHS != NULL && LHS->getSymbol().isDefined()
> +              && RHS != NULL && RHS->getSymbol().isDefined()) {
> +            // Don't emit relocation for expressions %hi(tmp) and
> %lo(tmp),
> +            // where tmp = sym1-sym2.
> +            IsResolved = true;
> +
> +            // Calculate symbol value.
> +            uint64_t Value1 =
> +
>  Layout.getSymbolOffset(&Asm.getSymbolData(LHS->getSymbol()));
> +            uint64_t Value2 =
> +
>  Layout.getSymbolOffset(&Asm.getSymbolData(RHS->getSymbol()));
> +            Value = Value1 - Value2;
> +          }
> +        }
> +      }


I think you shouldn't need to add code for evaluating a Sub expression to a
MIPS-specific file (as I commented on an earlier version of this change at
https://codereview.chromium.org/27690005/diff/188001/lib/Target/Mips/MCTargetDesc/MipsAsmBackend.cpp#newcode116
).

There is already code in the architecture-independent part of MC for
evaluating expressions that you should be able to reuse.  For example,
subtractions like the following already work on MIPS:

.long label2 - label1
label2:
.fill 0x54321
label1:

The code for evaluating that lives in AttemptToFoldSymbolOffsetDifference()
in MCExpr.cpp, which calls getSymbolOffset() like your patch does.

There's also a bug in your patch:  it doesn't check whether the symbols
being subtracted are in the same section, so I think it would give
incorrect output if the symbols were in different sections.

Cheers,
Mark
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131212/73722103/attachment.html>