This is an archive of the discontinued LLVM Phabricator instance.

[CSKY] Optimize conditional branch with BHZ32/BHSZ32/BLSZ32/BLZ32
ClosedPublic

Authored by benshi001 on Jun 22 2023, 9:26 PM.

Details

Summary

Add more Pats which generates BHZ32/BHSZ32/BLSZ32/BLZ32.

Diff Detail

Event Timeline

benshi001 created this revision.Jun 22 2023, 9:26 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 22 2023, 9:26 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
benshi001 requested review of this revision.Jun 22 2023, 9:26 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 22 2023, 9:26 PM
benshi001 updated this revision to Diff 533888.Jun 23 2023, 1:02 AM
benshi001 updated this revision to Diff 533889.

need more case about bhz/bhsz?

benshi001 added a comment.EditedJun 30 2023, 12:40 AM

need more case about bhz/bhsz?

It is hard to add tests for

def : Pat<(brcond (i32 (setge GPR:$rs1, (i32 1))), bb:$imm16),
          (BHZ32 GPR:$rs1, bb:$imm16)>;

Since the original IR pattern

%icmp = icmp sge i32 %x, 1
br i1 %icmp, label %label1, label %label2

is always transformed to an equivalent form by DAGCombiner

%icmp = icmp slt i32 %x, 1
br i1 %icmp, label %label2, label %label1

before reaching to the CSKY backend.

And BHSZ32 has the same issue.

I add these two Pats for BHSZ32 and BHZ32, just in case there is future changes in the DAGCombiner.

zixuan-wu accepted this revision.Jul 2 2023, 10:59 PM
This revision is now accepted and ready to land.Jul 2 2023, 10:59 PM
zixuan-wu added a comment.EditedJul 2 2023, 11:00 PM

I think the title can be more words. And the description should not be empty. Normally those two parts would be commit msg. @benshi001

barannikov88 added a subscriber: barannikov88.EditedJul 2 2023, 11:07 PM

need more case about bhz/bhsz?

It is hard to add tests for

def : Pat<(brcond (i32 (setge GPR:$rs1, (i32 1))), bb:$imm16),
          (BHZ32 GPR:$rs1, bb:$imm16)>;

Since the original IR pattern

%icmp = icmp sge i32 %x, 1
br i1 %icmp, label %label1, label %label2

is always transformed to an equivalent form by DAGCombiner

%icmp = icmp slt i32 %x, 1
br i1 %icmp, label %label2, label %label1

before reaching to the CSKY backend.

And BHSZ32 has the same issue.

I add these two Pats for BHSZ32 and BHZ32, just in case there is future changes in the DAGCombiner.

Try something like:

extern void on_false(void);
void test_ge_1(int x) {
  if (!(x >= 1))
    on_false();
}

need more case about bhz/bhsz?

It is hard to add tests for

def : Pat<(brcond (i32 (setge GPR:$rs1, (i32 1))), bb:$imm16),
          (BHZ32 GPR:$rs1, bb:$imm16)>;

Since the original IR pattern

%icmp = icmp sge i32 %x, 1
br i1 %icmp, label %label1, label %label2

is always transformed to an equivalent form by DAGCombiner

%icmp = icmp slt i32 %x, 1
br i1 %icmp, label %label2, label %label1

before reaching to the CSKY backend.

And BHSZ32 has the same issue.

I add these two Pats for BHSZ32 and BHZ32, just in case there is future changes in the DAGCombiner.

Try something like:

extern void on_false(void);
void test_ge_1(int x) {
  if (!(x >= 1))
    on_false();
}

Unfortunately your program can not lead to BHSZ or BHZ, still BLSZ as follow,

        .type   test_ge_1,@function
test_ge_1:                              # @test_ge_1
# %bb.0:
        subi16  sp, sp, 4
        st32.w  lr, (sp, 0)                     # 4-byte Folded Spill
        blsz32  a0, .LBB0_2
# %bb.1:
        ld32.w  lr, (sp, 0)                     # 4-byte Folded Reload
        addi16  sp, sp, 4
        rts16
.LBB0_2:
        jsri32  [.LCPI0_0]
        ld32.w  lr, (sp, 0)                     # 4-byte Folded Reload
        addi16  sp, sp, 4
        rts16
        .p2align        1
# %bb.3:
        .p2align        2, 0x0
.LCPI0_0:
        .long   on_false
.Lfunc_end0:
        .size   test_ge_1, .Lfunc_end0-test_ge_1
                                        # -- End function
This comment was removed by benshi001.
benshi001 edited the summary of this revision. (Show Details)Jul 2 2023, 11:13 PM
benshi001 retitled this revision from [CSKY] Optimize conditional branch to [CSKY] Optimize conditional branch with BHZ/BHSZ/BLSZ/BLZ.
benshi001 retitled this revision from [CSKY] Optimize conditional branch with BHZ/BHSZ/BLSZ/BLZ to [CSKY] Optimize conditional branch with BHZ32/BHSZ32/BLSZ32/BLZ32.Jul 2 2023, 11:15 PM
benshi001 edited the summary of this revision. (Show Details)

need more case about bhz/bhsz?

It is hard to add tests for

def : Pat<(brcond (i32 (setge GPR:$rs1, (i32 1))), bb:$imm16),
          (BHZ32 GPR:$rs1, bb:$imm16)>;

Since the original IR pattern

%icmp = icmp sge i32 %x, 1
br i1 %icmp, label %label1, label %label2

is always transformed to an equivalent form by DAGCombiner

%icmp = icmp slt i32 %x, 1
br i1 %icmp, label %label2, label %label1

before reaching to the CSKY backend.

And BHSZ32 has the same issue.

I add these two Pats for BHSZ32 and BHZ32, just in case there is future changes in the DAGCombiner.

Try something like:

extern void on_false(void);
void test_ge_1(int x) {
  if (!(x >= 1))
    on_false();
}

Unfortunately your program can not lead to BHSZ or BHZ, still BLSZ as follow,

        .type   test_ge_1,@function
test_ge_1:                              # @test_ge_1
# %bb.0:
        subi16  sp, sp, 4
        st32.w  lr, (sp, 0)                     # 4-byte Folded Spill
        blsz32  a0, .LBB0_2
# %bb.1:
        ld32.w  lr, (sp, 0)                     # 4-byte Folded Reload
        addi16  sp, sp, 4
        rts16
.LBB0_2:
        jsri32  [.LCPI0_0]
        ld32.w  lr, (sp, 0)                     # 4-byte Folded Reload
        addi16  sp, sp, 4
        rts16
        .p2align        1
# %bb.3:
        .p2align        2, 0x0
.LCPI0_0:
        .long   on_false
.Lfunc_end0:
        .size   test_ge_1, .Lfunc_end0-test_ge_1
                                        # -- End function

It does test the pattern though. You can see BHZ32 if you pass -stop-after=finalize-isel.

need more case about bhz/bhsz?

It is hard to add tests for

def : Pat<(brcond (i32 (setge GPR:$rs1, (i32 1))), bb:$imm16),
          (BHZ32 GPR:$rs1, bb:$imm16)>;

Since the original IR pattern

%icmp = icmp sge i32 %x, 1
br i1 %icmp, label %label1, label %label2

is always transformed to an equivalent form by DAGCombiner

%icmp = icmp slt i32 %x, 1
br i1 %icmp, label %label2, label %label1

before reaching to the CSKY backend.

And BHSZ32 has the same issue.

I add these two Pats for BHSZ32 and BHZ32, just in case there is future changes in the DAGCombiner.

Try something like:

extern void on_false(void);
void test_ge_1(int x) {
  if (!(x >= 1))
    on_false();
}

Unfortunately your program can not lead to BHSZ or BHZ, still BLSZ as follow,

        .type   test_ge_1,@function
test_ge_1:                              # @test_ge_1
# %bb.0:
        subi16  sp, sp, 4
        st32.w  lr, (sp, 0)                     # 4-byte Folded Spill
        blsz32  a0, .LBB0_2
# %bb.1:
        ld32.w  lr, (sp, 0)                     # 4-byte Folded Reload
        addi16  sp, sp, 4
        rts16
.LBB0_2:
        jsri32  [.LCPI0_0]
        ld32.w  lr, (sp, 0)                     # 4-byte Folded Reload
        addi16  sp, sp, 4
        rts16
        .p2align        1
# %bb.3:
        .p2align        2, 0x0
.LCPI0_0:
        .long   on_false
.Lfunc_end0:
        .size   test_ge_1, .Lfunc_end0-test_ge_1
                                        # -- End function

It does test the pattern though. You can see BHZ32 if you pass -stop-after=finalize-isel.

I still can not get BHZ32/BHSZ32, could you please make another patch for that?

I still can not get BHZ32/BHSZ32, could you please make another patch for that?

Sorry, I'm not familiar with architecture. I was just passing by.