This is an archive of the discontinued LLVM Phabricator instance.

[mips] Fix sc, scs, ll, lld instructions expanding
ClosedPublic

Authored by atanasyan on Nov 24 2019, 1:26 PM.

Details

Summary

There are a couple of bugs with the sc, scs, ll, lld instructions expanding:

  1. On R6 these instruction pack immediate offset into a 9-bit field. Now if an immediate exceeds 9-bits assembler does not perform expansion and just rejects such instruction.
  1. On 64-bit non-PIC code if an operand is a symbol assembler generates incorrect sequence of instructions. It uses` R_MIPS_HI16` and R_MIPS_LO16 relocations and skips R_MIPS_HIGHEST and R_MIPS_HIGHER ones.

To solve these problems this patch:

  • Introduces mem_simm9_exp to mark 9-bit memory immediate operands which require expansion. Probably later all mem_simm9 operands will be able to migrate on mem_simm9_exp and we rename it to mem_simm9.
  • Adds new OPERAND_MEM_SIMM9 operand type and assigns it to the mem_simm9_exp. That allows to know operand size in the processInstruction method and decide whether we need to expand instruction.
  • Adds expandMem9Inst method to expand instructions with 9-bit memory immediate operand. This method just load immediate into a "base" register used by original instruction:
sc $2, 256($sp) => addiu  $1, $sp, 256
                   sc     $2, 0($1)
  • Fix expandMem16Inst to support a correct set of relocations for symbol loading in case of 64-bit non-PIC code.
ll $12, symbol => lui    $12, 0
                      R_MIPS_HIGHEST symbol
                  daddiu $12, $12, 0
                      R_MIPS_HIGHER symbol
                  dsll   $12, $12, 16
                  daddiu $12, $12, 0
                      R_MIPS_HI16 symbol
                  dsll   $12, $12, 16
                  ll     $12, 0($12)
                      R_MIPS_LO16 symbol
  • Fix expandMem16Inst to unify handling of 3 and 4 operands instructions.
  • Delete unused now MipsTargetStreamer::emitSCWithSymOffset method.

Task for next patches - implement expanding for other instructions use mem_simm9 operand and other mem_simm## operands.

Diff Detail

Event Timeline

atanasyan created this revision.Nov 24 2019, 1:26 PM
atanasyan edited the summary of this revision. (Show Details)Nov 24 2019, 1:27 PM
Petar.Avramovic accepted this revision.Nov 26 2019, 4:18 AM

LGTM.

llvm/lib/Target/Mips/AsmParser/MipsAsmParser.cpp
3745

isGP64bit() feel more like an assert here or in ArePtrs64bit().
ABI.ArePtrs64bit() ? Mips::DADDu : Mips::ADDu implies that gpr are 64 bit when pointers are 64.
Flags: llvm-mc -triple mips64el -mattr=-gp64 ... make subtarget with N64 and no gp64, and this works like O32?

This revision is now accepted and ready to land.Nov 26 2019, 4:18 AM
atanasyan marked an inline comment as done.Nov 26 2019, 6:28 AM
atanasyan added inline comments.
llvm/lib/Target/Mips/AsmParser/MipsAsmParser.cpp
3745

It's a result of copy-paste. I think if (ABI.IsN64()) condition is enough here.

Thanks for review.

This revision was automatically updated to reflect the committed changes.