-
Notifications
You must be signed in to change notification settings - Fork 10.8k
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.
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
[SDAG] Count call argument attributes to reduce unnecessary extension #73501
Conversation
@llvm/pr-subscribers-llvm-selectiondag Author: ZengZhijin (zengdage) ChangesFull diff: https://github.com/llvm/llvm-project/pull/73501.diff 2 Files Affected:
diff --git a/llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp b/llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
index 1d0a03ccfcdc6ae..7ca6a8310b033e2 100644
--- a/llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp
@@ -69,6 +69,20 @@ static ISD::NodeType getPreferredExtendForValue(const Instruction *I) {
NumOfSigned += CI->isSigned();
NumOfUnsigned += CI->isUnsigned();
}
+ if (const auto *CallI = dyn_cast<CallInst>(U)) {
+ for (unsigned op = 0, Eop = CallI->arg_size(); op < Eop; ++op) {
+ if (CallI->getArgOperand(op) == I) {
+ const AttributeList &PAL = CallI->getAttributes();
+ AttributeSet Attrs = PAL.getParamAttrs(op);
+ if (Attrs.hasAttributes()) {
+ for (const auto &Attr : Attrs) {
+ NumOfUnsigned += Attr.hasAttribute(Attribute::ZExt);
+ NumOfSigned += Attr.hasAttribute(Attribute::SExt);
+ }
+ }
+ }
+ }
+ }
}
if (NumOfSigned > NumOfUnsigned)
ExtendKind = ISD::SIGN_EXTEND;
diff --git a/llvm/test/CodeGen/RISCV/reduce-unnecessary-extension.ll b/llvm/test/CodeGen/RISCV/reduce-unnecessary-extension.ll
new file mode 100644
index 000000000000000..8723bf68572825d
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/reduce-unnecessary-extension.ll
@@ -0,0 +1,127 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv64 -mattr=+zbb -verify-machineinstrs < %s \
+; RUN: | FileCheck -check-prefix=RV64I %s
+
+@PL_reg_match_utf8 = external dso_local local_unnamed_addr global i8, align 1
+
+declare dso_local signext i32 @test1(i8 noundef signext)
+
+declare dso_local signext i32 @test2(i8 noundef signext)
+
+declare dso_local signext i32 @test3(i8 noundef signext)
+
+define dso_local signext i32 @test() {
+; RV64I-LABEL: test:
+; RV64I: # %bb.0:
+; RV64I-NEXT: addi sp, sp, -16
+; RV64I-NEXT: .cfi_def_cfa_offset 16
+; RV64I-NEXT: sd ra, 8(sp) # 8-byte Folded Spill
+; RV64I-NEXT: sd s0, 0(sp) # 8-byte Folded Spill
+; RV64I-NEXT: .cfi_offset ra, -8
+; RV64I-NEXT: .cfi_offset s0, -16
+; RV64I-NEXT: lui a0, %hi(PL_reg_match_utf8)
+; RV64I-NEXT: lb s0, %lo(PL_reg_match_utf8)(a0)
+; RV64I-NEXT: beqz s0, .LBB0_2
+; RV64I-NEXT: # %bb.1:
+; RV64I-NEXT: mv a0, s0
+; RV64I-NEXT: call test1
+; RV64I-NEXT: mv a0, s0
+; RV64I-NEXT: call test2
+; RV64I-NEXT: mv a0, s0
+; RV64I-NEXT: call test3
+; RV64I-NEXT: j .LBB0_3
+; RV64I-NEXT: .LBB0_2:
+; RV64I-NEXT: li a0, 0
+; RV64I-NEXT: call test2
+; RV64I-NEXT: .LBB0_3:
+; RV64I-NEXT: li a0, 0
+; RV64I-NEXT: ld ra, 8(sp) # 8-byte Folded Reload
+; RV64I-NEXT: ld s0, 0(sp) # 8-byte Folded Reload
+; RV64I-NEXT: addi sp, sp, 16
+; RV64I-NEXT: ret
+ %1 = load i8, ptr @PL_reg_match_utf8, align 1
+ %2 = icmp eq i8 %1, 0
+ br i1 %2, label %7, label %3
+
+3:
+ %4 = tail call signext i32 @test1(i8 noundef signext %1)
+ %5 = tail call signext i32 @test2(i8 noundef signext %1)
+ %6 = tail call signext i32 @test3(i8 noundef signext %1)
+ br label %9
+
+7:
+ %8 = tail call signext i32 @test2(i8 noundef signext 0)
+ br label %9
+
+9:
+ ret i32 0
+}
+
+
+define signext i32 @test_loop() {
+; RV64I-LABEL: test_loop:
+; RV64I: # %bb.0:
+; RV64I-NEXT: addi sp, sp, -32
+; RV64I-NEXT: .cfi_def_cfa_offset 32
+; RV64I-NEXT: sd ra, 24(sp) # 8-byte Folded Spill
+; RV64I-NEXT: sd s0, 16(sp) # 8-byte Folded Spill
+; RV64I-NEXT: sd s1, 8(sp) # 8-byte Folded Spill
+; RV64I-NEXT: sd s2, 0(sp) # 8-byte Folded Spill
+; RV64I-NEXT: .cfi_offset ra, -8
+; RV64I-NEXT: .cfi_offset s0, -16
+; RV64I-NEXT: .cfi_offset s1, -24
+; RV64I-NEXT: .cfi_offset s2, -32
+; RV64I-NEXT: li s1, -16
+; RV64I-NEXT: lui s2, %hi(PL_reg_match_utf8)
+; RV64I-NEXT: j .LBB1_2
+; RV64I-NEXT: .LBB1_1: # in Loop: Header=BB1_2 Depth=1
+; RV64I-NEXT: mv a0, s0
+; RV64I-NEXT: call test2
+; RV64I-NEXT: addiw s1, s1, 1
+; RV64I-NEXT: beqz s1, .LBB1_4
+; RV64I-NEXT: .LBB1_2: # =>This Inner Loop Header: Depth=1
+; RV64I-NEXT: lb s0, %lo(PL_reg_match_utf8)(s2)
+; RV64I-NEXT: beqz s0, .LBB1_1
+; RV64I-NEXT: # %bb.3: # in Loop: Header=BB1_2 Depth=1
+; RV64I-NEXT: mv a0, s0
+; RV64I-NEXT: call test1
+; RV64I-NEXT: mv a0, s0
+; RV64I-NEXT: call test2
+; RV64I-NEXT: mv a0, s0
+; RV64I-NEXT: call test3
+; RV64I-NEXT: addiw s1, s1, 1
+; RV64I-NEXT: bnez s1, .LBB1_2
+; RV64I-NEXT: .LBB1_4:
+; RV64I-NEXT: li a0, 0
+; RV64I-NEXT: ld ra, 24(sp) # 8-byte Folded Reload
+; RV64I-NEXT: ld s0, 16(sp) # 8-byte Folded Reload
+; RV64I-NEXT: ld s1, 8(sp) # 8-byte Folded Reload
+; RV64I-NEXT: ld s2, 0(sp) # 8-byte Folded Reload
+; RV64I-NEXT: addi sp, sp, 32
+; RV64I-NEXT: ret
+ br label %1
+
+1:
+ %2 = phi i32 [ 16, %0 ], [ %12, %11 ]
+ %3 = load i8, ptr @PL_reg_match_utf8, align 1
+ %4 = icmp eq i8 %3, 0
+ br i1 %4, label %9, label %5
+
+5:
+ %6 = tail call signext i32 @test1(i8 noundef signext %3)
+ %7 = tail call signext i32 @test2(i8 noundef signext %3)
+ %8 = tail call signext i32 @test3(i8 noundef signext %3)
+ br label %11
+
+9:
+ %10 = tail call signext i32 @test2(i8 noundef signext %3)
+ br label %11
+
+11:
+ %12 = add nsw i32 %2, -1
+ %13 = icmp eq i32 %12, 0
+ br i1 %13, label %14, label %1
+
+14:
+ ret i32 0
+}
|
I have updated the patch based on reviewer comments. 1) switch the loop to be over |
NumOfSigned += CI->isSigned(); | ||
NumOfUnsigned += CI->isUnsigned(); | ||
} | ||
if (const auto *CallI = dyn_cast<CallBase>(U.getUser())) { | ||
const AttributeList &PAL = CallI->getAttributes(); | ||
AttributeSet Attrs = PAL.getParamAttrs(U.getOperandNo()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to make sure the use is one of the call operands and not the callee of an indirect call? Not sure how we determine what Instructions getPreferredExtendForValue
is called for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think need to make sure the use is one of the call operands. So I am going to add if (!CallI->isArgOperand(&U)) continue;
between line 72 and line 73. Do you think that's okay?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added if (!CallI->isArgOperand(&U)) continue;
in new commit. Please help to review. Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hi, are you still here? Is anything else need to modify?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Signed-off-by: Zhijin Zeng <zhijin.zeng@spacemit.com>
No description provided.