Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SOL] Remove neg and change the semantics of sub #73

Merged
merged 4 commits into from
Dec 16, 2023
Merged

[SOL] Remove neg and change the semantics of sub #73

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ee22760
Remove neg and change the semantics of sub
LucasSte Dec 12, 2023
5b91bed
Reorganize tests
LucasSte Dec 13, 2023
30f98d9
Reorganize instruction declarations
LucasSte Dec 14, 2023
8497ade
Start feature flag
LucasSte Dec 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion llvm/lib/Target/SBF/SBFInstrFormats.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ def SBF_OR : SBFArithOp<0x4>;
def SBF_AND : SBFArithOp<0x5>;
def SBF_LSH : SBFArithOp<0x6>;
def SBF_RSH : SBFArithOp<0x7>;
def SBF_NEG : SBFArithOp<0x8>;
def SBF_XOR : SBFArithOp<0xa>;
def SBF_MOV : SBFArithOp<0xb>;
def SBF_ARSH : SBFArithOp<0xc>;
Expand Down
67 changes: 37 additions & 30 deletions llvm/lib/Target/SBF/SBFInstrInfo.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,33 +263,58 @@ class ALU_RR<SBFOpClass Class, SBFArithOp Opc,
let SBFClass = Class;
}

multiclass ALU<SBFArithOp Opc, string Mnemonic, SDNode OpNode> {
// ALU with register operand
multiclass ALU_REG<SBFArithOp Opc, string Mnemonic, SDNode OpNode> {
def _rr : ALU_RR<SBF_ALU64, Opc,
(outs GPR:$dst),
(ins GPR:$src2, GPR:$src),
Mnemonic # "64 $dst, $src",
[(set GPR:$dst, (OpNode i64:$src2, i64:$src))]>;
def _rr_32 : ALU_RR<SBF_ALU, Opc,
(outs GPR32:$dst),
(ins GPR32:$src2, GPR32:$src),
Mnemonic # "32 $dst, $src",
[(set GPR32:$dst, (OpNode i32:$src2, i32:$src))]>;
}

// ALU with immediate operand
multiclass ALU<SBFArithOp Opc, string Mnemonic, SDNode OpNode> {
defm "" : ALU_REG<Opc, Mnemonic, OpNode>;
def _ri : ALU_RI<SBF_ALU64, Opc,
(outs GPR:$dst),
(ins GPR:$src2, i64imm:$imm),
Mnemonic # "64 $dst, $imm",
[(set GPR:$dst, (OpNode GPR:$src2, i64immSExt32:$imm))]>;

def _ri_32 : ALU_RI<SBF_ALU, Opc,
(outs GPR32:$dst),
(ins GPR32:$src2, i32imm:$imm),
Mnemonic # "32 $dst, $imm",
[(set GPR32:$dst, (OpNode GPR32:$src2, i32immSExt32:$imm))]>;
}

// ALU with immediate operand, but reversed semantics.
// Currently, this is only used for sub:
// sub r1, 2 means r1 = 2 - r2
multiclass ALU_REV<SBFArithOp Opc, string Mnemonic, SDNode OpNode> {
defm "" : ALU_REG<Opc, Mnemonic, OpNode>;
def _ri : ALU_RI<SBF_ALU64, Opc,
(outs GPR:$dst),
(ins GPR:$src2, i64imm:$imm),
Mnemonic # "64 $dst, $imm",
[(set GPR:$dst, (OpNode GPR:$src2, i64immSExt32:$imm))]>;
def _rr_32 : ALU_RR<SBF_ALU, Opc,
(outs GPR32:$dst),
(ins GPR32:$src2, GPR32:$src),
Mnemonic # "32 $dst, $src",
[(set GPR32:$dst, (OpNode i32:$src2, i32:$src))]>;
[(set GPR:$dst, (OpNode i64immSExt32:$imm, GPR:$src2))]>;

def _ri_32 : ALU_RI<SBF_ALU, Opc,
(outs GPR32:$dst),
(ins GPR32:$src2, i32imm:$imm),
Mnemonic # "32 $dst, $imm",
[(set GPR32:$dst, (OpNode GPR32:$src2, i32immSExt32:$imm))]>;
}
[(set GPR32:$dst, (OpNode i32immSExt32:$imm, GPR32:$src2))]>;
}

let Constraints = "$dst = $src2" in {
let isAsCheapAsAMove = 1 in {
defm ADD : ALU<SBF_ADD, "add", add>;
defm SUB : ALU<SBF_SUB, "sub", sub>;
defm SUB : ALU_REV<SBF_SUB, "sub", sub>;
defm OR : ALU<SBF_OR, "or", or>;
defm AND : ALU<SBF_AND, "and", and>;
defm SLL : ALU<SBF_LSH, "lsh", shl>;
Expand All @@ -306,24 +331,6 @@ let Constraints = "$dst = $src2" in {
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The entire part of the patch above involving the ALU multiclass and the SUB definition need to be reworked and simplified. That is, there is no need to refactor and duplicate the multiclass. We can succinctly adjust the original multiclass by conditionally swapping the pattern operands as follows:

@@ -264,6 +264,12 @@ class ALU_RR<SBFOpClass Class, SBFArithOp Opc,
 }
 
 multiclass ALU<SBFArithOp Opc, string Mnemonic, SDNode OpNode> {
+  // Match swapped operand order only for special case of 'sub reg,imm`.
+  defvar DoSwap = !eq(OpNode, sub);
+  defvar RegImmPat = !if(DoSwap, (OpNode i64immSExt32:$imm, GPR:$src2),
+                                 (OpNode GPR:$src2, i64immSExt32:$imm));
+  defvar RegImmPat32 = !if(DoSwap,(OpNode i32immSExt32:$imm, GPR32:$src2),
+                                  (OpNode GPR32:$src2, i32immSExt32:$imm));
   def _rr : ALU_RR<SBF_ALU64, Opc,
                    (outs GPR:$dst),
                    (ins GPR:$src2, GPR:$src),
@@ -273,7 +279,7 @@ multiclass ALU<SBFArithOp Opc, string Mnemonic, SDNode OpNod
e> {
                    (outs GPR:$dst),
                    (ins GPR:$src2, i64imm:$imm),
                    Mnemonic # "64 $dst, $imm",
-                   [(set GPR:$dst, (OpNode GPR:$src2, i64immSExt32:$imm))]>;
+                   [(set GPR:$dst, RegImmPat)]>;
   def _rr_32 : ALU_RR<SBF_ALU, Opc,
                    (outs GPR32:$dst),
                    (ins GPR32:$src2, GPR32:$src),
@@ -283,7 +289,7 @@ multiclass ALU<SBFArithOp Opc, string Mnemonic, SDNode OpNod
e> {
                    (outs GPR32:$dst),
                    (ins GPR32:$src2, i32imm:$imm),
                    Mnemonic # "32 $dst, $imm",
-                   [(set GPR32:$dst, (OpNode GPR32:$src2, i32immSExt32:$imm))]>
;
+                   [(set GPR32:$dst, RegImmPat32)]>;
 }
 
 let Constraints = "$dst = $src2" in {
@@ -306,24 +312,6 @@ let Constraints = "$dst = $src2" in {
     }
 }

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One related general question I have is-- should we conditionally be generating the swapped special case sub? We need to exactly match the runtime semantics on chain today and down the road.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new syntax can be behind the sbfv2 cpu perhaps?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't find a way to encapsulate the new sub inside a predicate for sbfv2. All the other example cases do not redefine the same instruction. Instead, the predicate encapsulate definitions that only exist for a certain feature and do not override other definitions.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything should be behind a feature flag now.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for incorporating my offline suggestions, this is ready to land (other than some trivial coding standard nits, which I'll note shortly).


class NEG_RR<SBFOpClass Class, SBFArithOp Opc,
dag outs, dag ins, string asmstr, list<dag> pattern>
: TYPE_ALU_JMP<Opc.Value, 0, outs, ins, asmstr, pattern> {
bits<4> dst;

let Inst{51-48} = dst;
let SBFClass = Class;
}

let Constraints = "$dst = $src", isAsCheapAsAMove = 1 in {
def NEG_64: NEG_RR<SBF_ALU64, SBF_NEG, (outs GPR:$dst), (ins GPR:$src),
"neg64 $dst",
[(set GPR:$dst, (ineg i64:$src))]>;
def NEG_32: NEG_RR<SBF_ALU, SBF_NEG, (outs GPR32:$dst), (ins GPR32:$src),
"neg32 $dst",
[(set GPR32:$dst, (ineg i32:$src))]>;
}

class LD_IMM64<bits<4> Pseudo, string Mnemonic>
: TYPE_LD_ST<SBF_IMM.Value, SBF_DW.Value,
(outs GPR:$dst),
Expand Down Expand Up @@ -748,9 +755,9 @@ let Constraints = "$dst = $val" in {
// atomic_load_sub can be represented as a neg followed
// by an atomic_load_add.
def : Pat<(atomic_load_sub_32 ADDRri:$addr, GPR32:$val),
(XFADDW32 ADDRri:$addr, (NEG_32 GPR32:$val))>;
(XFADDW32 ADDRri:$addr, (SUB_ri_32 GPR32:$val, 0))>;
def : Pat<(atomic_load_sub_64 ADDRri:$addr, GPR:$val),
(XFADDD ADDRri:$addr, (NEG_64 GPR:$val))>;
(XFADDD ADDRri:$addr, (SUB_ri GPR:$val, 0))>;

let Predicates = [SBFSubtargetSolana], usesCustomInserter = 1, isCodeGenOnly = 1 in {
def ATOMIC_FENCE : Pseudo<
Expand Down
59 changes: 59 additions & 0 deletions llvm/test/CodeGen/SBF/neg_instr_with_sub.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
; RUN: llc < %s -march=sbf -mattr=+alu32 | FileCheck %s
;
; Source:
; int test_func_64(long * vec, long idx) {
; vec[idx] = -idx;
; return idx;
; }
;
; int test_func_32(int * vec, int idx) {
; vec[idx] = -idx;
; return idx;
; }
;
; Compilation flag:
; clang -S -emit-llvm test.c

; Function Attrs: noinline nounwind optnone ssp uwtable(sync)
define i32 @test_func_32(ptr noundef %vec, i32 noundef %idx) #0 {
; CHECK-LABEL: test_func_32:
; CHECK: stxw [r10 - 12], w2
; CHECK: sub32 w2, 0
; CHECK: stxw [r1 + 0], w2
entry:
%vec.addr = alloca ptr, align 8
%idx.addr = alloca i32, align 4
store ptr %vec, ptr %vec.addr, align 8
store i32 %idx, ptr %idx.addr, align 4
%0 = load i32, ptr %idx.addr, align 4
%sub = sub nsw i32 0, %0
%1 = load ptr, ptr %vec.addr, align 8
%2 = load i32, ptr %idx.addr, align 4
%idxprom = sext i32 %2 to i64
%arrayidx = getelementptr inbounds i32, ptr %1, i64 %idxprom
store i32 %sub, ptr %arrayidx, align 4
%3 = load i32, ptr %idx.addr, align 4
ret i32 %3
}

; Function Attrs: noinline nounwind optnone ssp uwtable(sync)
define i32 @test_func_64(ptr noundef %vec, i64 noundef %idx) #0 {
entry:
; CHECK-LABEL: test_func_64:
; CHECK: stxdw [r10 - 16], r2
; CHECK: sub64 r2, 0
; CHECK: stxdw [r1 + 0], r2
%vec.addr = alloca ptr, align 8
%idx.addr = alloca i64, align 8
store ptr %vec, ptr %vec.addr, align 8
store i64 %idx, ptr %idx.addr, align 8
%0 = load i64, ptr %idx.addr, align 8
%sub = sub nsw i64 0, %0
%1 = load ptr, ptr %vec.addr, align 8
%2 = load i64, ptr %idx.addr, align 8
%arrayidx = getelementptr inbounds i64, ptr %1, i64 %2
store i64 %sub, ptr %arrayidx, align 8
%3 = load i64, ptr %idx.addr, align 8
%conv = trunc i64 %3 to i32
ret i32 %conv
}
82 changes: 82 additions & 0 deletions llvm/test/CodeGen/SBF/sub_reversed_immidiate.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
; RUN: llc < %s -march=sbf -mattr=+alu32 | FileCheck %s
;
; Source:
; void test_func_64(long * vec) {
; vec[0] = 50 - vec[0];
; vec[1] = vec[3] - vec[2];
; }
;
; void test_func_32(int * vec) {
; vec[0] = 50 - vec[0];
; vec[1] = vec[3] - vec[2];
; }
;
; Compilation flag:
; clang -S -emit-llvm test.c


; Function Attrs: noinline nounwind optnone ssp uwtable(sync)
define void @test_func_64(ptr noundef %vec) #0 {
entry:
; CHECK-LABEL: test_func_64:
%vec.addr = alloca ptr, align 8
store ptr %vec, ptr %vec.addr, align 8
%0 = load ptr, ptr %vec.addr, align 8
%arrayidx = getelementptr inbounds i64, ptr %0, i64 0
%1 = load i64, ptr %arrayidx, align 8
%sub = sub nsw i64 50, %1
; CHECK: ldxdw r2, [r1 + 0]
; CHECK: sub64 r2, 50
; CHECK: stxdw [r1 + 0], r2
%2 = load ptr, ptr %vec.addr, align 8
%arrayidx1 = getelementptr inbounds i64, ptr %2, i64 0
store i64 %sub, ptr %arrayidx1, align 8
%3 = load ptr, ptr %vec.addr, align 8
%arrayidx2 = getelementptr inbounds i64, ptr %3, i64 3
%4 = load i64, ptr %arrayidx2, align 8
%5 = load ptr, ptr %vec.addr, align 8
%arrayidx3 = getelementptr inbounds i64, ptr %5, i64 2
%6 = load i64, ptr %arrayidx3, align 8
%sub4 = sub nsw i64 %4, %6
; CHECK: ldxdw r2, [r1 + 16]
; CHECK: ldxdw r3, [r1 + 24]
; CHECK: sub64 r3, r2
; CHECK: stxdw [r1 + 8], r3
%7 = load ptr, ptr %vec.addr, align 8
%arrayidx5 = getelementptr inbounds i64, ptr %7, i64 1
store i64 %sub4, ptr %arrayidx5, align 8
ret void
}

; Function Attrs: noinline nounwind optnone ssp uwtable(sync)
define void @test_func_32(ptr noundef %vec) #0 {
entry:
; CHECK-LABEL: test_func_32:
%vec.addr = alloca ptr, align 8
store ptr %vec, ptr %vec.addr, align 8
%0 = load ptr, ptr %vec.addr, align 8
%arrayidx = getelementptr inbounds i32, ptr %0, i64 0
%1 = load i32, ptr %arrayidx, align 4
%sub = sub nsw i32 50, %1
; CHECK: ldxw w2, [r1 + 0]
; CHECK: sub32 w2, 50
; CHECK: stxw [r1 + 0], w2
%2 = load ptr, ptr %vec.addr, align 8
%arrayidx1 = getelementptr inbounds i32, ptr %2, i64 0
store i32 %sub, ptr %arrayidx1, align 4
%3 = load ptr, ptr %vec.addr, align 8
%arrayidx2 = getelementptr inbounds i32, ptr %3, i64 3
%4 = load i32, ptr %arrayidx2, align 4
%5 = load ptr, ptr %vec.addr, align 8
%arrayidx3 = getelementptr inbounds i32, ptr %5, i64 2
%6 = load i32, ptr %arrayidx3, align 4
%sub4 = sub nsw i32 %4, %6
; CHECK: ldxw w2, [r1 + 8]
; CHECK: ldxw w3, [r1 + 12]
; CHECK: sub32 w3, w2
; CHECK: stxw [r1 + 4], w3
%7 = load ptr, ptr %vec.addr, align 8
%arrayidx5 = getelementptr inbounds i32, ptr %7, i64 1
store i32 %sub4, ptr %arrayidx5, align 4
ret void
}
8 changes: 0 additions & 8 deletions llvm/test/MC/Disassembler/SBF/sbf-alu.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,14 +224,6 @@



# CHECK-NEW: neg64 r9
0x87,0x09,0x00,0x00,0x00,0x00,0x00,0x00

# CHECK-NEW: neg32 w6
0x84,0x06,0x00,0x00,0x00,0x00,0x00,0x00



# CHECK-NEW: mov64 r0, r9
0xbf,0x90,0x00,0x00,0x00,0x00,0x00,0x00

Expand Down
2 changes: 0 additions & 2 deletions llvm/test/MC/SBF/insn-unit-32.s
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,10 @@
# RUN: llvm-objdump --mattr=+alu32 -d -r %t | FileCheck %s

// ======== BPF_ALU Class ========
neg32 w1 // BPF_NEG
add32 w0, w1 // BPF_ADD | BPF_X
sub32 w1, w2 // BPF_SUB | BPF_X
mul32 w2, w3 // BPF_MUL | BPF_X
div32 w3, w4 // BPF_DIV | BPF_X
// CHECK: 84 01 00 00 00 00 00 00 neg32 w1
// CHECK: 0c 10 00 00 00 00 00 00 add32 w0, w1
// CHECK: 1c 21 00 00 00 00 00 00 sub32 w1, w2
// CHECK: 2c 32 00 00 00 00 00 00 mul32 w2, w3
Expand Down
2 changes: 0 additions & 2 deletions llvm/test/MC/SBF/insn-unit.s
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,6 @@
// CHECK: 3f 43 00 00 00 00 00 00 div64 r3, r4

Llabel0 :
neg64 r2 // BPF_NEG
or64 r4, r5 // BPF_OR | BPF_X
and64 r5, r6 // BPF_AND | BPF_X
lsh64 r6, r7 // BPF_LSH | BPF_X
Expand All @@ -116,7 +115,6 @@ Llabel0 :
mov64 r9, r10 // BPF_MOV | BPF_X
arsh64 r10, r0 // BPF_ARSH | BPF_X
// CHECK: <Llabel0>:
// CHECK: 87 02 00 00 00 00 00 00 neg64 r2
// CHECK: 4f 54 00 00 00 00 00 00 or64 r4, r5
// CHECK: 5f 65 00 00 00 00 00 00 and64 r5, r6
// CHECK: 6f 76 00 00 00 00 00 00 lsh64 r6, r7
Expand Down
10 changes: 0 additions & 10 deletions llvm/test/MC/SBF/sbf-alu.s
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,16 +294,6 @@ arsh32 w5, -123



# CHECK-OBJ-NEW: neg64 r9
# CHECK-ASM-NEW: encoding: [0x87,0x09,0x00,0x00,0x00,0x00,0x00,0x00]
neg64 r9

# CHECK-OBJ-NEW: neg32 w6
# CHECK-ASM-NEW: encoding: [0x84,0x06,0x00,0x00,0x00,0x00,0x00,0x00]
neg32 w6



# CHECK-OBJ-NEW: mov64 r0, r9
# CHECK-ASM-NEW: encoding: [0xbf,0x90,0x00,0x00,0x00,0x00,0x00,0x00]
mov64 r0, r9
Expand Down