Skip to content

Commit

Permalink
[mips] Fix 64-bit address loading in case of applying 32-bit mask to …
Browse files Browse the repository at this point in the history
…the result

If result of 64-bit address loading combines with 32-bit mask, LLVM
tries to optimize the code and remove "redundant" loading of upper
32-bits of the address. It leads to incorrect code on MIPS64 targets.

MIPS backend creates the following chain of commands to load 64-bit
address in the `MipsTargetLowering::getAddrNonPICSym64` method:
```
(add (shl (add (shl (add %highest(sym), %higher(sym)),
                    16),
               %hi(sym)),
          16),
     %lo(%sym))
```

If the mask presents, LLVM decides to optimize the chain of commands. It
really does not make sense to load upper 32-bits because the 0x0fffffff
mask anyway clears them. After removing redundant commands we get this
chain:
```
(add (shl (%hi(sym), 16), %lo(%sym))
```

There is no patterns matched `(MipsHi (i64 symbol))`. Due a bug in `SYM_32`
predicate definition, backend incorrectly selects a pattern for a 32-bit
symbols and uses the `lui` instruction for loading `%hi(sym)`.

As a result we get incorrect set of instructions with unnecessary 16-bit
left shifting:
```
lui     at,0x0
    R_MIPS_HI16     foo
dsll    at,at,0x10
daddiu  at,at,0
    R_MIPS_LO16     foo
```

This patch resolves two problems:
- Fix `SYM_32/SYM_64` predicates to prevent selection of patterns dedicated
  to 32-bit symbols in case of using N64 ABI.
- Add missed patterns for 64-bit symbols for `%hi/%lo`.

Fix PR42736.

Differential Revision: https://reviews.llvm.org/D66228

llvm-svn: 370268
  • Loading branch information
atanasyan committed Aug 28, 2019
1 parent 01cd91a commit 59bb360
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 12 deletions.
3 changes: 3 additions & 0 deletions llvm/lib/Target/Mips/Mips.td
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ class PredicateControl {
list<Predicate> GPRPredicates = [];
// Predicates for the PTR size such as IsPTR64bit
list<Predicate> PTRPredicates = [];
// Predicates for a symbol's size such as hasSym32.
list<Predicate> SYMPredicates = [];
// Predicates for the FGR size and layout such as IsFP64bit
list<Predicate> FGRPredicates = [];
// Predicates for the instruction group membership such as ISA's.
Expand All @@ -38,6 +40,7 @@ class PredicateControl {
list<Predicate> Predicates = !listconcat(EncodingPredicates,
GPRPredicates,
PTRPredicates,
SYMPredicates,
FGRPredicates,
InsnPredicates,
HardFloatPredicate,
Expand Down
34 changes: 34 additions & 0 deletions llvm/lib/Target/Mips/Mips64InstrInfo.td
Original file line number Diff line number Diff line change
Expand Up @@ -682,6 +682,20 @@ let AdditionalPredicates = [NotInMicroMips] in {
(DADDiu GPR64:$hi, tjumptable:$lo)>, ISA_MIPS3, GPR_64, SYM_64;
def : MipsPat<(add GPR64:$hi, (MipsHigher (i64 tconstpool:$lo))),
(DADDiu GPR64:$hi, tconstpool:$lo)>, ISA_MIPS3, GPR_64, SYM_64;
def : MipsPat<(add GPR64:$hi, (MipsHigher (i64 texternalsym:$lo))),
(DADDiu GPR64:$hi, texternalsym:$lo)>,
ISA_MIPS3, GPR_64, SYM_64;

def : MipsPat<(MipsHi (i64 tglobaladdr:$in)),
(DADDiu ZERO_64, tglobaladdr:$in)>, ISA_MIPS3, GPR_64, SYM_64;
def : MipsPat<(MipsHi (i64 tblockaddress:$in)),
(DADDiu ZERO_64, tblockaddress:$in)>, ISA_MIPS3, GPR_64, SYM_64;
def : MipsPat<(MipsHi (i64 tjumptable:$in)),
(DADDiu ZERO_64, tjumptable:$in)>, ISA_MIPS3, GPR_64, SYM_64;
def : MipsPat<(MipsHi (i64 tconstpool:$in)),
(DADDiu ZERO_64, tconstpool:$in)>, ISA_MIPS3, GPR_64, SYM_64;
def : MipsPat<(MipsHi (i64 texternalsym:$in)),
(DADDiu ZERO_64, texternalsym:$in)>, ISA_MIPS3, GPR_64, SYM_64;

def : MipsPat<(add GPR64:$hi, (MipsHi (i64 tglobaladdr:$lo))),
(DADDiu GPR64:$hi, tglobaladdr:$lo)>, ISA_MIPS3, GPR_64, SYM_64;
Expand All @@ -692,6 +706,23 @@ let AdditionalPredicates = [NotInMicroMips] in {
(DADDiu GPR64:$hi, tjumptable:$lo)>, ISA_MIPS3, GPR_64, SYM_64;
def : MipsPat<(add GPR64:$hi, (MipsHi (i64 tconstpool:$lo))),
(DADDiu GPR64:$hi, tconstpool:$lo)>, ISA_MIPS3, GPR_64, SYM_64;
def : MipsPat<(add GPR64:$hi, (MipsHi (i64 texternalsym:$lo))),
(DADDiu GPR64:$hi, texternalsym:$lo)>,
ISA_MIPS3, GPR_64, SYM_64;

def : MipsPat<(MipsLo (i64 tglobaladdr:$in)),
(DADDiu ZERO_64, tglobaladdr:$in)>, ISA_MIPS3, GPR_64, SYM_64;
def : MipsPat<(MipsLo (i64 tblockaddress:$in)),
(DADDiu ZERO_64, tblockaddress:$in)>, ISA_MIPS3, GPR_64, SYM_64;
def : MipsPat<(MipsLo (i64 tjumptable:$in)),
(DADDiu ZERO_64, tjumptable:$in)>, ISA_MIPS3, GPR_64, SYM_64;
def : MipsPat<(MipsLo (i64 tconstpool:$in)),
(DADDiu ZERO_64, tconstpool:$in)>, ISA_MIPS3, GPR_64, SYM_64;
def : MipsPat<(MipsLo (i64 tglobaltlsaddr:$in)),
(DADDiu ZERO_64, tglobaltlsaddr:$in)>,
ISA_MIPS3, GPR_64, SYM_64;
def : MipsPat<(MipsLo (i64 texternalsym:$in)),
(DADDiu ZERO_64, texternalsym:$in)>, ISA_MIPS3, GPR_64, SYM_64;

def : MipsPat<(add GPR64:$hi, (MipsLo (i64 tglobaladdr:$lo))),
(DADDiu GPR64:$hi, tglobaladdr:$lo)>, ISA_MIPS3, GPR_64, SYM_64;
Expand All @@ -705,6 +736,9 @@ let AdditionalPredicates = [NotInMicroMips] in {
def : MipsPat<(add GPR64:$hi, (MipsLo (i64 tglobaltlsaddr:$lo))),
(DADDiu GPR64:$hi, tglobaltlsaddr:$lo)>, ISA_MIPS3, GPR_64,
SYM_64;
def : MipsPat<(add GPR64:$hi, (MipsLo (i64 texternalsym:$lo))),
(DADDiu GPR64:$hi, texternalsym:$lo)>,
ISA_MIPS3, GPR_64, SYM_64;
}

// gp_rel relocs
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/Target/Mips/MipsInstrInfo.td
Original file line number Diff line number Diff line change
Expand Up @@ -211,9 +211,9 @@ def HasCnMips : Predicate<"Subtarget->hasCnMips()">,
AssemblerPredicate<"FeatureCnMips">;
def NotCnMips : Predicate<"!Subtarget->hasCnMips()">,
AssemblerPredicate<"!FeatureCnMips">;
def IsSym32 : Predicate<"Subtarget->HasSym32()">,
def IsSym32 : Predicate<"Subtarget->hasSym32()">,
AssemblerPredicate<"FeatureSym32">;
def IsSym64 : Predicate<"!Subtarget->HasSym32()">,
def IsSym64 : Predicate<"!Subtarget->hasSym32()">,
AssemblerPredicate<"!FeatureSym32">;
def IsN64 : Predicate<"Subtarget->isABI_N64()">;
def IsNotN64 : Predicate<"!Subtarget->isABI_N64()">;
Expand Down
14 changes: 6 additions & 8 deletions llvm/test/CodeGen/Mips/indirect-jump-hazard/long-calls.ll
Original file line number Diff line number Diff line change
Expand Up @@ -74,20 +74,18 @@ define void @caller() {
; N64-NEXT: daddiu $25, $1, %lo(callee)
; N64-NEXT: jalr.hb $25
; N64-NEXT: nop
; N64-NEXT: daddiu $1, $zero, %higher(memset)
; N64-NEXT: lui $2, %highest(memset)
; N64-NEXT: daddu $1, $2, $1
; N64-NEXT: dsll $1, $1, 16
; N64-NEXT: lui $2, %hi(memset)
; N64-NEXT: daddu $1, $1, $2
; N64-NEXT: dsll $1, $1, 16
; N64-NEXT: daddiu $25, $1, %lo(memset)
; N64-NEXT: lui $1, %highest(val)
; N64-NEXT: daddiu $1, $1, %higher(val)
; N64-NEXT: dsll $1, $1, 16
; N64-NEXT: daddiu $1, $1, %hi(val)
; N64-NEXT: dsll $1, $1, 16
; N64-NEXT: lui $2, %highest(memset)
; N64-NEXT: daddiu $4, $1, %lo(val)
; N64-NEXT: daddiu $1, $2, %higher(memset)
; N64-NEXT: dsll $1, $1, 16
; N64-NEXT: daddiu $1, $1, %hi(memset)
; N64-NEXT: dsll $1, $1, 16
; N64-NEXT: daddiu $25, $1, %lo(memset)
; N64-NEXT: daddiu $5, $zero, 0
; N64-NEXT: jalr.hb $25
; N64-NEXT: daddiu $6, $zero, 80
Expand Down
6 changes: 4 additions & 2 deletions llvm/test/CodeGen/Mips/long-calls.ll
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,11 @@ define void @caller() {
; ON64: daddiu $25, $1, %lo(callee)
; ON64: jalr $25

; ON64: daddiu $1, $zero, %higher(memset)
; ON64: lui $2, %highest(memset)
; ON64: lui $2, %hi(memset)
; ON64: daddiu $1, $2, %higher(memset)
; ON64: dsll $1, $1, 16
; ON64: daddiu $1, $1, %hi(memset)
; ON64: dsll $1, $1, 16
; ON64: daddiu $25, $1, %lo(memset)
; ON64: jalr $25

Expand Down
28 changes: 28 additions & 0 deletions llvm/test/CodeGen/Mips/pr42736.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
; RUN: llc -mtriple=mips64-linux-gnuabi64 \
; RUN: -relocation-model=pic < %s | FileCheck %s -check-prefix=PIC
; RUN: llc -mtriple=mips64-linux-gnuabi64 \
; RUN: -relocation-model=static < %s | FileCheck %s -check-prefix=STATIC

define void @bar1() nounwind {
entry:
; PIC: lui $[[R0:[0-9]+]], 4095
; PIC-NEXT: ori $[[R0]], $[[R0]], 65535
; PIC-NEXT: ld $[[R1:[0-9]+]], %got_disp(foo)(${{[0-9]+}})
; PIC-NEXT: and $[[R1]], $[[R1]], $[[R0]]
; PIC-NEXT: sd $[[R1]]

; STATIC: lui $[[R0:[0-9]+]], 4095
; STATIC-NEXT: ori $[[R0]], $[[R0]], 65535
; STATIC-NEXT: daddiu $[[R1:[0-9]+]], $zero, %hi(foo)
; STATIC-NEXT: dsll $[[R1]], $[[R1]], 16
; STATIC-NEXT: daddiu $[[R1]], $[[R1]], %lo(foo)
; STATIC-NEXT: and $[[R0]], $[[R1]], $[[R0]]
; STATIC-NEXT: sd $[[R0]]

%val = alloca i64, align 8
store i64 and (i64 ptrtoint (void ()* @foo to i64), i64 268435455), i64* %val, align 8
%0 = load i64, i64* %val, align 8
ret void
}

declare void @foo()

0 comments on commit 59bb360

Please sign in to comment.