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

[Mark Seaborn]

Thanks for the detailed explanation, and sorry for the delayed reply.

The fact that %lo and %hi (VK_Mips_ABS_{HI,LO}) can only be attached to a
MCSymbolRefExpr does seem to be a bit wrong.  They are really functions,
which should be able to wrap any MCExpr.  The fact that "%hi(x - y)" seems
to be converted to "%hi(x) - %hi(y)" by the parser adds to the confusion.
 More comments below...

On 24 December 2013 12:39, Sasa Stankovic <Sasa.Stankovic at rt-rk.com> wrote:

> > 2) Why is it that you handle cases like:
> >
> > .set diff, label2 - label1
> > lui $2, %lo(diff)
> >
> > but not cases where that is written more directly as:
> >
> > lui $2, %lo(label2 - label1)
> >
> > (which still gives the UNREACHABLE error I described earlier in the
> > thread)?  Is it just because the former is easier to implement?  If so,
> is
> > that just a quirk of how these cases are handled via different code paths
> > in MC?
>
> Implementing %lo(label2 - label1) was first thing that I tried. The
> problem was that no MCBinaryExpr constructor takes relocation kind as
> argument; relocations can only be passed to MCSymbolRefExpr. Currently,
> when MipsAsmParser parses %lo(label2 - label1), it creates MCBinaryExpr
> where both operands are MCSymbolRefExpr, and both operands have relocation
> kind for %lo relocation (VK_Mips_ABS_LO). This is the reason for
> UNREACHABLE error, because there are two identical relocations for the
> same offset in .o file.
>
> I then tried to pass VK_Mips_None instead of VK_Mips_ABS_LO to second
> operand, but this also produced couple of UNREACHABLE errors, this time in
> Mips specific code.
>
> Finally, I created target-specific MCExpr subclass, as a subclass of
> MCTargetExpr. The implementation was similar to
> lib/Target/ARM/MCTargetDesc/ARMMCExpr.h. This class has members for
> expression and relocation kind, so associating relocation to expression as
> a whole was easy.
>

Thanks for pointing out ARMMCExpr.h.  Taking a similar approach to
ARMMCExpr.h does seem like it would be a lot cleaner.

Grepping for EvaluateAsRelocatableImpl() shows that this does seem to be
the normal way to implement hi/lo functions in MC.  The following targets
define their own MCExpr subclasses:

 * lib/Target/AArch64/MCTargetDesc/AArch64MCExpr.h
 * lib/Target/ARM/MCTargetDesc/ARMMCExpr.h
 * lib/Target/NVPTX/NVPTXMCExpr.h
 * lib/Target/PowerPC/MCTargetDesc/PPCMCExpr.h (confusingly, this defines
VK_PPC_LO, but there's another VK_PPC_LO defined in
include/llvm/MC/MCExpr.h)
 * lib/Target/Sparc/MCTargetDesc/SparcMCExpr.h

The ARM and NVPTX implementations of EvaluateAsRelocatableImpl() don't
delegate to their subexpressions, but the rest do, calling
getSubExpr()->EvaluateAsRelocatable(), which is what you'd want to do for a
MIPS version so that %lo/%hi work in any context that can accept a constant.



> The patch worked, but then people at Mips noted that gas supports
> assigning symbol difference to temp symbol and applying %hi and %lo on
> that symbol. That seemed much simpler.


The temp symbol approach might be a smaller patch, but it does seem like a
hack that's working around limitations elsewhere.  Can you post the earlier
patch you had for adding a Mips MCExpr subclass?

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