[Mips] Add MipsPat (MipsGPRel tglobaladdr:$in) to select MipsISD::GPRel TargetGlobalAddress#165531
Conversation
|
@llvm/pr-subscribers-backend-mips Author: None (yingopq) ChangesWhen SelectionDAG process visitSELECT, would fold select(cond, binop(x, y), binop(x, z)) to binop(x, select(cond, y, z)). Full diff: https://github.com/llvm/llvm-project/pull/165531.diff 2 Files Affected:
diff --git a/llvm/lib/Target/Mips/MipsCondMov.td b/llvm/lib/Target/Mips/MipsCondMov.td
index e9e09a188bf5b..947a52bddebb4 100644
--- a/llvm/lib/Target/Mips/MipsCondMov.td
+++ b/llvm/lib/Target/Mips/MipsCondMov.td
@@ -196,6 +196,9 @@ let AdditionalPredicates = [NotInMicroMips] in {
}
// Instantiation of conditional move patterns.
+ def : MipsPat<(add GPR64:$gp, (select GPR32:$cond, (MipsGPRel tglobaladdr:$T), (MipsGPRel tglobaladdr:$F))),
+ (MOVN_I_I64 (i64 (DADDiu GPR64:$gp, tglobaladdr:$T)), GPR32:$cond, (i64 (DADDiu GPR64:$gp, tglobaladdr:$F)))>, ISA_MIPS3, ABI_N64;
+
defm : MovzPats0<GPR32, GPR32, MOVZ_I_I, SLT, SLTu, SLTi, SLTiu>,
INSN_MIPS4_32_NOT_32R6_64R6;
defm : MovzPats1<GPR32, GPR32, MOVZ_I_I, XOR>, INSN_MIPS4_32_NOT_32R6_64R6;
diff --git a/llvm/test/CodeGen/Mips/llvm-ir/select-globaladdr.ll b/llvm/test/CodeGen/Mips/llvm-ir/select-globaladdr.ll
new file mode 100644
index 0000000000000..54c272a832721
--- /dev/null
+++ b/llvm/test/CodeGen/Mips/llvm-ir/select-globaladdr.ll
@@ -0,0 +1,22 @@
+; RUN: llc < %s -mattr +noabicalls -mgpopt | FileCheck %s -check-prefixes=MIPS64
+
+target datalayout = "E-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
+target triple = "mips64-unknown-linux-muslabi64"
+
+@.str = external constant [6 x i8]
+@.str.1 = external constant [6 x i8]
+
+define ptr @tst_select_ptr_ptr(i1 %tobool.not) {
+entry:
+; MIPS64-LABEL: tst_select_ptr_ptr:
+; MIPS64: # %bb.0: # %entry
+; MIPS64: sll $1, $4, 0
+; MIPS64: andi $1, $1, 1
+; MIPS64: daddiu $2, $gp, %gp_rel(.str)
+; MIPS64: daddiu $3, $gp, %gp_rel(.str.1)
+; MIPS64: jr $ra
+; MIPS64: movn $2, $3, $1
+
+ %cond = select i1 %tobool.not, ptr @.str.1, ptr @.str
+ ret ptr %cond
+}
|
|
cc @arsenm |
arsenm
left a comment
There was a problem hiding this comment.
It's not obvious to me what the problem being solved here is from the description. It sounds like an optimization, but when I try the test it's a selection failure. This isn't really a solution for that, you need to MipsISD::GPRel as a leaf
There was a problem hiding this comment.
Should remove redundant datalayout string
Look at the bug report mentioned. |
The commit message should directly contain all of this information |
Sorry, I would update the commit message. |
arsenm
left a comment
There was a problem hiding this comment.
The bigger issue is there isn't a freestanding pattern for the global address
5a5f0d8 to
c12da8d
Compare
Please help see the new commit message. And I did not get the meanings you said. |
The failure is on not handling a freestanding MipsGPRel tglobaladdr:$T. This patch is not a suitable fix for that problem. This patch can only be an optimization, which is hiding a missed pattern on the raw address materialize. If you remove the select from this, I'd expect it could still fail the same way |
I got it, thanks. |
5cd8861 to
804f872
Compare
|
@arsenm Can you review again, thanks! |
|
Ping. |
|
@arsenm Can you review again, thanks! |
|
Can you update the patch title and description |
OK. |
…Rel TargetGlobalAddress
The original logic:
```
SelectionDAG has 17 nodes:
t0: ch,glue = EntryToken
t2: i64,ch = CopyFromReg t0, Register:i64 %0
t11: i32 = truncate t2
t14: i32 = and t11, Constant:i32<1>
t20: i64 = MipsISD::GPRel TargetGlobalAddress:i64<ptr @.str.1> 0 [TF=3]
t21: i64 = add Register:i64 $gp_64, t20
t16: i64 = MipsISD::GPRel TargetGlobalAddress:i64<ptr @.str> 0 [TF=3]
t18: i64 = add Register:i64 $gp_64, t16
t7: i64 = select t14, t21, t18
```
When SelectionDAG process visitSELECT, would fold select(cond,
binop(x, y), binop(x, z)) to binop(x, select(cond, y, z)).
As follows:
```
SelectionDAG has 16 nodes:
t0: ch,glue = EntryToken
t2: i64,ch = CopyFromReg t0, Register:i64 %0
t11: i32 = truncate t2
t14: i32 = and t11, Constant:i32<1>
t20: i64 = MipsISD::GPRel TargetGlobalAddress:i64<ptr @.str.1> 0 [TF=3]
t16: i64 = MipsISD::GPRel TargetGlobalAddress:i64<ptr @.str> 0 [TF=3]
t22: i64 = select t14, t20, t16
t23: i64 = add Register:i64 $gp_64, t22
```
Therefore, the original MipsPat `add GPR64:$gp, (MipsGPRel
tglobaladdr:$in)` is no longer available. And there would be an assert:
```
ISEL: Starting selection on root node: t20: i64 = MipsISD::GPRel TargetGlobalAddress:i64<ptr @.str.1> 0 [TF=3]
ISEL: Starting pattern match
Initial Opcode index to 0
Match failed at index 0
LLVM ERROR: Cannot select: t20: i64 = MipsISD::GPRel TargetGlobalAddress:i64<ptr @.str.1> 0 [TF=3]
```
So we add a new MipsPat:
`def : MipsPat<(MipsGPRel tglobaladdr:$in),
(DADDiu ZERO_64, tglobaladdr:$in)>, ISA_MIPS3, ABI_N64;`
to parse MipsISD::GPRel TargetGlobalAddress.
Fix llvm#142060.
804f872 to
631fc2e
Compare
add $gp, (select $cond, tglobaladdr, tglobaladdr) to parse MipsISD::GPRel TargetGlobalAddress(MipsGPRel tglobaladdr:$in) to parse MipsISD::GPRel TargetGlobalAddress
🪟 Windows x64 Test Results
Failed Tests(click on a test name to see its output) LLVMLLVM.CodeGen/RISCV/riscv-macho.llIf these failures are unrelated to your changes (for example tests are broken or flaky at HEAD), please open an issue at https://github.com/llvm/llvm-project/issues and add the |
Can you help review again? |
(MipsGPRel tglobaladdr:$in) to parse MipsISD::GPRel TargetGlobalAddress(MipsGPRel tglobaladdr:$in) to select MipsISD::GPRel TargetGlobalAddress
…PRel TargetGlobalAddress (llvm#165531) The original logic: ``` SelectionDAG has 17 nodes: t0: ch,glue = EntryToken t2: i64,ch = CopyFromReg t0, Register:i64 %0 t11: i32 = truncate t2 t14: i32 = and t11, Constant:i32<1> t20: i64 = MipsISD::GPRel TargetGlobalAddress:i64<ptr @.str.1> 0 [TF=3] t21: i64 = add Register:i64 $gp_64, t20 t16: i64 = MipsISD::GPRel TargetGlobalAddress:i64<ptr @.str> 0 [TF=3] t18: i64 = add Register:i64 $gp_64, t16 t7: i64 = select t14, t21, t18 ``` When SelectionDAG process visitSELECT, would fold select(cond, binop(x, y), binop(x, z)) to binop(x, select(cond, y, z)). As follows: ``` SelectionDAG has 16 nodes: t0: ch,glue = EntryToken t2: i64,ch = CopyFromReg t0, Register:i64 %0 t11: i32 = truncate t2 t14: i32 = and t11, Constant:i32<1> t20: i64 = MipsISD::GPRel TargetGlobalAddress:i64<ptr @.str.1> 0 [TF=3] t16: i64 = MipsISD::GPRel TargetGlobalAddress:i64<ptr @.str> 0 [TF=3] t22: i64 = select t14, t20, t16 t23: i64 = add Register:i64 $gp_64, t22 ``` Therefore, the original MipsPat `add GPR64:$gp, (MipsGPRel tglobaladdr:$in)` is no longer available. And there would be an assert: ``` ISEL: Starting selection on root node: t20: i64 = MipsISD::GPRel TargetGlobalAddress:i64<ptr @.str.1> 0 [TF=3] ISEL: Starting pattern match Initial Opcode index to 0 Match failed at index 0 LLVM ERROR: Cannot select: t20: i64 = MipsISD::GPRel TargetGlobalAddress:i64<ptr @.str.1> 0 [TF=3] ``` So we add a new MipsPat `def : MipsPat<(MipsGPRel tglobaladdr:$in), (DADDiu ZERO_64, tglobaladdr:$in)>, ISA_MIPS3, ABI_N64;` to parse MipsISD::GPRel TargetGlobalAddress. Fix llvm#142060.
The original logic:
When SelectionDAG process visitSELECT, would fold select(cond,
binop(x, y), binop(x, z)) to binop(x, select(cond, y, z)).
As follows:
Therefore, the original MipsPat
add GPR64:$gp, (MipsGPRel tglobaladdr:$in)is no longer available. And there would be an assert:So we add a new MipsPat
def : MipsPat<(MipsGPRel tglobaladdr:$in), (DADDiu ZERO_64, tglobaladdr:$in)>, ISA_MIPS3, ABI_N64;to parse MipsISD::GPRel TargetGlobalAddress.
Fix #142060.