<div dir="ltr"><div>On 13 December 2013 07:23, Sasa Stankovic <span dir="ltr"><<a href="mailto:Sasa.Stankovic@rt-rk.com" target="_blank">Sasa.Stankovic@rt-rk.com</a>></span> wrote:<br></div><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Hi Mark,<br>
<br>
The modified patch is attached. I used the method<br>
MCExpr::EvaluateAsAbsolute for computing the symbol value. This also<br>
handles the case of two symbols in different sections (EvaluateAsAbsolute<br>
won't compute symbol value in that case).<br></blockquote><div><br></div><div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
+$L1:<br>+        .set    $temp1, $L1-$L2<br>+        lui     $4, %hi($temp1)<br>+        addiu   $4, $4, %lo($temp1)<br>+$L2:<br>+        .set    $temp2, $L2-$L1<br>+        lui     $5, %hi($temp2)<br>+        lw      $5, %lo($temp2)($5)<br>
+<br>+# CHECK-INSTR:    lui     $4, 0<br>+# CHECK-INSTR:    addiu   $4, $4, -8<br>+# CHECK-INSTR:    lui     $5, 0<br>+# CHECK-INSTR:    lw      $5, 8($5)</blockquote></div><div><br></div><div>This test isn't testing that %hi works because your uses of %hi(X) always evaluate to 0 in this test, which might easily happen by accident.  How about changing it to do something like the following?</div>
<div><br></div><div>sym1:</div><div>.fill 0x43210</div><div>sym2:</div><div><br></div><div>Then you can test that %hi(sym2 - sym1) evaluates to 4.</div><div><br></div><div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
@@ -297,6 +299,29 @@<br>     // access to MCContext from here which allows us to report a fatal error<br>     // with *possibly* a source code location.<br>     (void)adjustFixupValue(Fixup, Value, &Asm.getContext());<br>
</blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">+<br>+    // Check for expressions %hi($tmp) and %lo($tmp), where $tmp is symbol<br>
+    // defined as a difference of two symbols, $tmp=sym1-sym2.<br>+    if ((unsigned)Fixup.getKind() == Mips::fixup_Mips_HI16<br>+        || (unsigned)Fixup.getKind() == Mips::fixup_Mips_LO16) {<br></blockquote><div><br>
</div><div>Since the code below isn't specific to HI16 or LO16, could you remove this check?  Maybe then the call to adjustFixupValue() should come after the new code so that the value is checked for the other fixup types that have constraints.</div>
<div><br></div><div>With that check removed, this is no longer MIPS-specific, so should the new code go into evaluateFixup() in lib/MC/MCAssembler.cpp instead?  I tried that and all the tests in test/MC/ still passed.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">+<br>+      if (Target.getSymA() != NULL && Target.getSymA()->getSymbol().isVariable()<br>
+          && Target.getSymB() == NULL && Target.getConstant() == 0) {<br>+        const MCExpr *SymValue = Target.getSymA()->getSymbol()<br>+                                       .getVariableValue();<br>
+        const MCBinaryExpr *BE = dyn_cast<MCBinaryExpr>(SymValue);<br>+<br>+        if (BE && BE->getOpcode() == MCBinaryExpr::Sub) {<br></blockquote><div><br></div><div>Since the code below isn't specific to MCBinaryExpr::Sub, I think you can remove this check and just do SymValue->EvaluateAsAbsolute(AbsValue, Layout).</div>
<div><br></div><div>Cheers,</div><div>Mark</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
+          int64_t AbsValue;<br>+          if (BE->EvaluateAsAbsolute(AbsValue, Layout)) {<br>+            Value = AbsValue;<br>+            // Don't emit relocation for expressions %hi(tmp) and %lo(tmp),<br>+            // where tmp = sym1-sym2.<br>
+            IsResolved = true;<br>+          }<br>+        }<br>+      }<br>+    }</blockquote></div><div><br></div></div></div></div>