This is an archive of the discontinued LLVM Phabricator instance.

Differential D142281

[AVR] Optimize 16-bit comparison with a constant
ClosedPublic

Authored by benshi001 on Jan 21 2023, 5:45 AM.

Details

Reviewers
aykevl
dylanmckay
jacquesguan
Commits
rGf37d7c9381ff: [AVR] Optimize 16-bit comparison with a constant
Summary

Fixes https://github.com/llvm/llvm-project/issues/30923

Diff Detail

Event Timeline

benshi001 created this revision.Jan 21 2023, 5:45 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 21 2023, 5:45 AM
Herald added subscribers: Jim, hiraditya, dylanmckay. · View Herald Transcript
benshi001 requested review of this revision.Jan 21 2023, 5:45 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 21 2023, 5:45 AM
Herald added subscribers: llvm-commits, jacquesguan. · View Herald Transcript
benshi001 added inline comments.Jan 21 2023, 6:14 AM
llvm/lib/Target/AVR/AVRISelLowering.cpp
660

Only a zero constant with be put at left by previous DAG transform, all other non-zero constant are dropped as RHS. So we have to handle it specially.

llvm/test/CodeGen/AVR/cmp.ll
213

x > 0 is compiled to 0 < x, and the low 0 value byte and the high 0 value byte are represented by the zero register.

benshi001 updated this revision to Diff 491070.Jan 21 2023, 6:16 AM
benshi001 added inline comments.Jan 21 2023, 6:20 AM
llvm/test/CodeGen/AVR/cmp.ll
232

x > 126 is compiled to x >= 127, the low byte value is 127, and the high byte valued zero is represented via the zero register.

252

x > 1023 is compiled to x >= 1024, the high byte is 4, and the low byte is represented via the zero register.

benshi001 added a reviewer: dylanmckay.Jan 21 2023, 6:20 AM
benshi001 updated this revision to Diff 491071.Jan 21 2023, 6:23 AM
Harbormaster completed remote builds in B209145: Diff 491071.Jan 21 2023, 7:21 AM
benshi001 updated this revision to Diff 491116.Jan 21 2023, 6:46 PM
Harbormaster completed remote builds in B209185: Diff 491116.Jan 21 2023, 7:58 PM
benshi001 updated this revision to Diff 494966.Feb 5 2023, 5:38 PM
benshi001 updated this revision to Diff 494967.Feb 5 2023, 5:53 PM
benshi001 added inline comments.Feb 5 2023, 6:01 PM
llvm/test/CodeGen/AVR/cmp.ll
243

x > 126 is compiled to x >= 127, the low byte value is 127, and the high byte valued zero is represented via the zero register.

289

x > 1023 is compiled to x >= 1024, the high byte is 4, and the low byte is represented via the zero register.

Harbormaster completed remote builds in B211975: Diff 494967.Feb 5 2023, 6:51 PM
jacquesguan accepted this revision.Feb 8 2023, 11:09 PM

LGTM.

This revision is now accepted and ready to land.Feb 8 2023, 11:09 PM
Closed by commit rGf37d7c9381ff: [AVR] Optimize 16-bit comparison with a constant (authored by benshi001). · Explain WhyFeb 8 2023, 11:13 PM
This revision was automatically updated to reflect the committed changes.
benshi001 added a commit: rGf37d7c9381ff: [AVR] Optimize 16-bit comparison with a constant.

Revision Contents

PathSize
llvm/
lib/
Target/
AVR/
26 lines
test/
CodeGen/
AVR/
96 lines

Diff 496028

llvm/lib/Target/AVR/AVRISelLowering.cpp

Show First 20 Lines • Show All 628 Lines • ▼ Show 20 Lines assert((LHS.getSimpleValueType() == RHS.getSimpleValueType()) &&
"LHS and RHS have different types"); "LHS and RHS have different types");
assert(((LHS.getSimpleValueType() == MVT::i16) || assert(((LHS.getSimpleValueType() == MVT::i16) ||
(LHS.getSimpleValueType() == MVT::i8)) && (LHS.getSimpleValueType() == MVT::i8)) &&
"invalid comparison type"); "invalid comparison type");
SDValue Cmp; SDValue Cmp;
if (LHS.getSimpleValueType() == MVT::i16 && isa<ConstantSDNode>(RHS)) { if (LHS.getSimpleValueType() == MVT::i16 && isa<ConstantSDNode>(RHS)) {
// Generate a CPI/CPC pair if RHS is a 16-bit constant. uint64_t Imm = cast<ConstantSDNode>(RHS)->getZExtValue();
// Generate a CPI/CPC pair if RHS is a 16-bit constant. Use the zero
// register for the constant RHS if its lower or higher byte is zero.
SDValue LHSlo = DAG.getNode(ISD::EXTRACT_ELEMENT, DL, MVT::i8, LHS, SDValue LHSlo = DAG.getNode(ISD::EXTRACT_ELEMENT, DL, MVT::i8, LHS,
DAG.getIntPtrConstant(0, DL)); DAG.getIntPtrConstant(0, DL));
SDValue LHShi = DAG.getNode(ISD::EXTRACT_ELEMENT, DL, MVT::i8, LHS, SDValue LHShi = DAG.getNode(ISD::EXTRACT_ELEMENT, DL, MVT::i8, LHS,
DAG.getIntPtrConstant(1, DL)); DAG.getIntPtrConstant(1, DL));
SDValue RHSlo = (Imm & 0xff) == 0
? DAG.getRegister(Subtarget.getZeroRegister(), MVT::i8)
: DAG.getNode(ISD::EXTRACT_ELEMENT, DL, MVT::i8, RHS,
DAG.getIntPtrConstant(0, DL));
SDValue RHShi = (Imm & 0xff00) == 0
? DAG.getRegister(Subtarget.getZeroRegister(), MVT::i8)
: DAG.getNode(ISD::EXTRACT_ELEMENT, DL, MVT::i8, RHS,
DAG.getIntPtrConstant(1, DL));
Cmp = DAG.getNode(AVRISD::CMP, DL, MVT::Glue, LHSlo, RHSlo);
Cmp = DAG.getNode(AVRISD::CMPC, DL, MVT::Glue, LHShi, RHShi, Cmp);
} else if (RHS.getSimpleValueType() == MVT::i16 && isa<ConstantSDNode>(LHS)) {
// Generate a CPI/CPC pair if LHS is a 16-bit constant. Use the zero
// register for the constant LHS if its lower or higher byte is zero.
uint64_t Imm = cast<ConstantSDNode>(LHS)->getZExtValue();
SDValue LHSlo = (Imm & 0xff) == 0
? DAG.getRegister(Subtarget.getZeroRegister(), MVT::i8)
: DAG.getNode(ISD::EXTRACT_ELEMENT, DL, MVT::i8, LHS,
benshi001AuthorUnsubmitted
Done
ReplyInline Actions

Only a zero constant with be put at left by previous DAG transform, all other non-zero constant are dropped as RHS. So we have to handle it specially.

benshi001: Only a zero constant with be put at left by previous DAG transform, all other non-zero constant…
DAG.getIntPtrConstant(0, DL));
SDValue LHShi = (Imm & 0xff00) == 0
? DAG.getRegister(Subtarget.getZeroRegister(), MVT::i8)
: DAG.getNode(ISD::EXTRACT_ELEMENT, DL, MVT::i8, LHS,
DAG.getIntPtrConstant(1, DL));
SDValue RHSlo = DAG.getNode(ISD::EXTRACT_ELEMENT, DL, MVT::i8, RHS, SDValue RHSlo = DAG.getNode(ISD::EXTRACT_ELEMENT, DL, MVT::i8, RHS,
DAG.getIntPtrConstant(0, DL)); DAG.getIntPtrConstant(0, DL));
SDValue RHShi = DAG.getNode(ISD::EXTRACT_ELEMENT, DL, MVT::i8, RHS, SDValue RHShi = DAG.getNode(ISD::EXTRACT_ELEMENT, DL, MVT::i8, RHS,
DAG.getIntPtrConstant(1, DL)); DAG.getIntPtrConstant(1, DL));
Cmp = DAG.getNode(AVRISD::CMP, DL, MVT::Glue, LHSlo, RHSlo); Cmp = DAG.getNode(AVRISD::CMP, DL, MVT::Glue, LHSlo, RHSlo);
Cmp = DAG.getNode(AVRISD::CMPC, DL, MVT::Glue, LHShi, RHShi, Cmp); Cmp = DAG.getNode(AVRISD::CMPC, DL, MVT::Glue, LHShi, RHShi, Cmp);
} else { } else {
// Generate ordinary 16-bit comparison. // Generate ordinary 16-bit comparison.
▲ Show 20 Lines • Show All 2,139 Lines • Show Last 20 Lines

llvm/test/CodeGen/AVR/cmp.ll

; RUN: llc < %s -march=avr | FileCheck %s ; RUN: llc < %s -march=avr | FileCheck %s
; RUN: llc < %s -mtriple=avr -mcpu=attiny10 | FileCheck --check-prefix=TINY %s
declare void @f1(i8) declare void @f1(i8)
declare void @f2(i8) declare void @f2(i8)
define void @cmp8(i8 %a, i8 %b) { define void @cmp8(i8 %a, i8 %b) {
; CHECK-LABEL: cmp8: ; CHECK-LABEL: cmp8:
; CHECK: cp ; CHECK: cp
; CHECK-NOT: cpc ; CHECK-NOT: cpc
%cmp = icmp eq i8 %a, %b %cmp = icmp eq i8 %a, %b
▲ Show 20 Lines • Show All 187 Lines • ▼ Show 20 Linesif.then:
tail call void @f9() tail call void @f9()
br label %if.end br label %if.end
if.else: if.else:
tail call void @f10() tail call void @f10()
br label %if.end br label %if.end
if.end: if.end:
ret void ret void
} }
define i16 @cmp_i16_gt_0(i16 %0) {
; CHECK-LABEL: cmp_i16_gt_0:
; CHECK: ; %bb.0:
; CHECK-NEXT: ldi r18, 1
; CHECK-NEXT: cp r1, r24
; CHECK-NEXT: cpc r1, r25
; CHECK-NEXT: brlt .LBB11_2
benshi001AuthorUnsubmitted
Done
ReplyInline Actions

x > 0 is compiled to 0 < x, and the low 0 value byte and the high 0 value byte are represented by the zero register.

benshi001: `x > 0` is compiled to `0 < x`, and the low 0 value byte and the high 0 value byte are…
; CHECK-NEXT: ; %bb.1:
; CHECK-NEXT: mov r18, r1
; CHECK-NEXT: .LBB11_2:
; CHECK-NEXT: mov r24, r18
; CHECK-NEXT: clr r25
; CHECK-NEXT: ret
;
; TINY-LABEL: cmp_i16_gt_0:
; TINY: ; %bb.0:
; TINY-NEXT: ldi r20, 1
; TINY-NEXT: cp r17, r24
; TINY-NEXT: cpc r17, r25
; TINY-NEXT: brlt .LBB11_2
; TINY-NEXT: ; %bb.1:
; TINY-NEXT: mov r20, r17
; TINY-NEXT: .LBB11_2:
; TINY-NEXT: mov r24, r20
; TINY-NEXT: clr r25
; TINY-NEXT: ret
benshi001AuthorUnsubmitted
Done
ReplyInline Actions

x > 126 is compiled to x >= 127, the low byte value is 127, and the high byte valued zero is represented via the zero register.

benshi001: `x > 126` is compiled to `x >= 127`, the low byte value is 127, and the high byte valued zero…
%2 = icmp sgt i16 %0, 0
%3 = zext i1 %2 to i16
ret i16 %3
}
define i16 @cmp_i16_gt_126(i16 %0) {
; CHECK-LABEL: cmp_i16_gt_126:
; CHECK: ; %bb.0:
; CHECK-NEXT: ldi r18, 1
; CHECK-NEXT: cpi r24, 127
; CHECK-NEXT: cpc r25, r1
benshi001AuthorUnsubmitted
Done
ReplyInline Actions

x > 126 is compiled to x >= 127, the low byte value is 127, and the high byte valued zero is represented via the zero register.

benshi001: x > 126 is compiled to x >= 127, the low byte value is 127, and the high byte valued zero is…
; CHECK-NEXT: brge .LBB12_2
; CHECK-NEXT: ; %bb.1:
; CHECK-NEXT: mov r18, r1
; CHECK-NEXT: .LBB12_2:
; CHECK-NEXT: mov r24, r18
; CHECK-NEXT: clr r25
; CHECK-NEXT: ret
;
; TINY-LABEL: cmp_i16_gt_126:
benshi001AuthorUnsubmitted
Done
ReplyInline Actions

x > 1023 is compiled to x >= 1024, the high byte is 4, and the low byte is represented via the zero register.

benshi001: `x > 1023` is compiled to `x >= 1024`, the high byte is 4, and the low byte is represented via…
; TINY: ; %bb.0:
; TINY-NEXT: ldi r20, 1
; TINY-NEXT: cpi r24, 127
; TINY-NEXT: cpc r25, r17
; TINY-NEXT: brge .LBB12_2
; TINY-NEXT: ; %bb.1:
; TINY-NEXT: mov r20, r17
; TINY-NEXT: .LBB12_2:
; TINY-NEXT: mov r24, r20
; TINY-NEXT: clr r25
; TINY-NEXT: ret
%2 = icmp sgt i16 %0, 126
%3 = zext i1 %2 to i16
ret i16 %3
}
define i16 @cmp_i16_gt_1023(i16 %0) {
; CHECK-LABEL: cmp_i16_gt_1023:
; CHECK: ; %bb.0:
; CHECK-NEXT: ldi r19, 4
; CHECK-NEXT: ldi r18, 1
; CHECK-NEXT: cp r24, r1
; CHECK-NEXT: cpc r25, r19
; CHECK-NEXT: brge .LBB13_2
; CHECK-NEXT: ; %bb.1:
; CHECK-NEXT: mov r18, r1
; CHECK-NEXT: .LBB13_2:
; CHECK-NEXT: mov r24, r18
; CHECK-NEXT: clr r25
; CHECK-NEXT: ret
;
; TINY-LABEL: cmp_i16_gt_1023:
; TINY: ; %bb.0:
; TINY-NEXT: ldi r21, 4
; TINY-NEXT: ldi r20, 1
; TINY-NEXT: cp r24, r17
; TINY-NEXT: cpc r25, r21
benshi001AuthorUnsubmitted
Done
ReplyInline Actions

x > 1023 is compiled to x >= 1024, the high byte is 4, and the low byte is represented via the zero register.

benshi001: x > 1023 is compiled to x >= 1024, the high byte is 4, and the low byte is represented via the…
; TINY-NEXT: brge .LBB13_2
; TINY-NEXT: ; %bb.1:
; TINY-NEXT: mov r20, r17
; TINY-NEXT: .LBB13_2:
; TINY-NEXT: mov r24, r20
; TINY-NEXT: clr r25
; TINY-NEXT: ret
%2 = icmp sgt i16 %0, 1023
%3 = zext i1 %2 to i16
ret i16 %3
}