[Hexagon] Implement shouldConvertConstantLoadToIntImm #146452
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This will convert loads of constant strings to immediate values. Put this behind a flag that is enabled by default so that we can toggle it if need be.
|
@llvm/pr-subscribers-backend-hexagon Author: Sudharsan Veeravalli (svs-quic) ChangesThis will convert loads of constant strings to immediate values. Put this behind a flag that is enabled by default so that we can toggle it if need be. Full diff: https://github.com/llvm/llvm-project/pull/146452.diff 3 Files Affected:
diff --git a/llvm/lib/Target/Hexagon/HexagonISelLowering.cpp b/llvm/lib/Target/Hexagon/HexagonISelLowering.cpp
index 6534ae938fede..e7d0ec6ee0fe5 100644
--- a/llvm/lib/Target/Hexagon/HexagonISelLowering.cpp
+++ b/llvm/lib/Target/Hexagon/HexagonISelLowering.cpp
@@ -102,6 +102,10 @@ static cl::opt<int>
MaxStoresPerMemsetOptSizeCL("max-store-memset-Os", cl::Hidden, cl::init(4),
cl::desc("Max #stores to inline memset"));
+static cl::opt<bool>
+ ConstantLoadsToImm("constant-loads-to-imm", cl::Hidden, cl::init(true),
+ cl::desc("Convert constant loads to immediate values."));
+
static cl::opt<bool> AlignLoads("hexagon-align-loads",
cl::Hidden, cl::init(false),
cl::desc("Rewrite unaligned loads as a pair of aligned loads"));
@@ -3607,6 +3611,18 @@ bool HexagonTargetLowering::isFPImmLegal(const APFloat &Imm, EVT VT,
return true;
}
+/// Returns true if it is beneficial to convert a load of a constant
+/// to just the constant itself.
+bool HexagonTargetLowering::shouldConvertConstantLoadToIntImm(const APInt &Imm,
+ Type *Ty) const {
+ if (!ConstantLoadsToImm)
+ return false;
+
+ assert(Ty->isIntegerTy());
+ unsigned BitSize = Ty->getPrimitiveSizeInBits();
+ return (BitSize > 0 && BitSize <= 64);
+}
+
/// isLegalAddressingMode - Return true if the addressing mode represented by
/// AM is legal for this target, for a load/store of the specified type.
bool HexagonTargetLowering::isLegalAddressingMode(const DataLayout &DL,
diff --git a/llvm/lib/Target/Hexagon/HexagonISelLowering.h b/llvm/lib/Target/Hexagon/HexagonISelLowering.h
index 1321bee44a295..a2c9b57d04caa 100644
--- a/llvm/lib/Target/Hexagon/HexagonISelLowering.h
+++ b/llvm/lib/Target/Hexagon/HexagonISelLowering.h
@@ -342,6 +342,11 @@ class HexagonTargetLowering : public TargetLowering {
SDValue getPICJumpTableRelocBase(SDValue Table, SelectionDAG &DAG)
const override;
+ /// Returns true if it is beneficial to convert a load of a constant
+ /// to just the constant itself.
+ bool shouldConvertConstantLoadToIntImm(const APInt &Imm,
+ Type *Ty) const override;
+
bool shouldReduceLoadWidth(SDNode *Load, ISD::LoadExtType ExtTy, EVT NewVT,
std::optional<unsigned> ByteOffset) const override;
diff --git a/llvm/test/CodeGen/Hexagon/hexagon-strcpy.ll b/llvm/test/CodeGen/Hexagon/hexagon-strcpy.ll
new file mode 100644
index 0000000000000..b23366bc11aca
--- /dev/null
+++ b/llvm/test/CodeGen/Hexagon/hexagon-strcpy.ll
@@ -0,0 +1,57 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
+; RUN: llc -march=hexagon -verify-machineinstrs < %s | FileCheck %s
+
+@.str = private unnamed_addr constant [31 x i8] c"DHRYSTONE PROGRAM, 3'RD STRING\00", align 1
+@.str1 = private unnamed_addr constant [3 x i8] c"%s\00", align 1
+
+; Function Attrs: nounwind
+declare i32 @printf(i8* nocapture readonly, ...)
+
+; Function Attrs: nounwind
+define i32 @main() {
+; CHECK-LABEL: main:
+; CHECK: .cfi_startproc
+; CHECK-NEXT: // %bb.0: // %entry
+; CHECK-NEXT: .cfi_def_cfa r30, 8
+; CHECK-NEXT: .cfi_offset r31, -4
+; CHECK-NEXT: .cfi_offset r30, -8
+; CHECK-NEXT: {
+; CHECK-NEXT: r0 = ##.L.str1
+; CHECK-NEXT: r3:2 = CONST64(#2325073635944967245)
+; CHECK-NEXT: allocframe(r29,#40):raw
+; CHECK-NEXT: }
+; CHECK-NEXT: {
+; CHECK-NEXT: r1 = add(r29,#8)
+; CHECK-NEXT: r7:6 = CONST64(#4706902966564560965)
+; CHECK-NEXT: r5:4 = CONST64(#5642821575076104260)
+; CHECK-NEXT: }
+; CHECK-NEXT: {
+; CHECK-NEXT: memb(r29+#38) = #0
+; CHECK-NEXT: memw(r29+#0) = r1
+; CHECK-NEXT: }
+; CHECK-NEXT: {
+; CHECK-NEXT: memd(r29+#24) = r3:2
+; CHECK-NEXT: memd(r29+#16) = r7:6
+; CHECK-NEXT: }
+; CHECK-NEXT: {
+; CHECK-NEXT: memd(r29+#8) = r5:4
+; CHECK-NEXT: memh(r29+#36) = ##18254
+; CHECK-NEXT: }
+; CHECK-NEXT: {
+; CHECK-NEXT: call printf
+; CHECK-NEXT: memw(r29+#32) = ##1230132307
+; CHECK-NEXT: }
+; CHECK-NEXT: {
+; CHECK-NEXT: r0 = #0
+; CHECK-NEXT: dealloc_return
+; CHECK-NEXT: }
+entry:
+ %blah = alloca [30 x i8], align 8
+ %arraydecay = getelementptr inbounds [30 x i8], [30 x i8]* %blah, i32 0, i32 0
+ call void @llvm.memcpy.p0i8.p0i8.i32(i8* %arraydecay, i8* getelementptr inbounds ([31 x i8], [31 x i8]* @.str, i32 0, i32 0), i32 31, i32 1, i1 false)
+ %call2 = call i32 (i8*, ...) @printf(i8* getelementptr inbounds ([3 x i8], [3 x i8]* @.str1, i32 0, i32 0), i8* %arraydecay)
+ ret i32 0
+}
+
+; Function Attrs: nounwind
+declare void @llvm.memcpy.p0i8.p0i8.i32(i8* nocapture, i8* nocapture readonly, i32, i32, i1)
|
aankit-ca
approved these changes
Jul 1, 2025
yandalur
approved these changes
Jul 1, 2025
rlavaee
pushed a commit
to rlavaee/llvm-project
that referenced
this pull request
Jul 1, 2025
This will convert loads of constant strings to immediate values. Put this behind a flag that is enabled by default so that we can toggle it if need be.
rlavaee
pushed a commit
to rlavaee/llvm-project
that referenced
this pull request
Jul 1, 2025
This will convert loads of constant strings to immediate values. Put this behind a flag that is enabled by default so that we can toggle it if need be.
|
Hi, this commit currently leads to a test fail with 21-rc1 and -rc2 on Alpine Linux edge on most architectures: Do you understand whats going on here? |
|
It's been almost a month since this got merged. It looks like there is a change in the order of the cfi instructions emitted in the test case. Do you have a buildbot link for this? I'm not sure if there was another patch that caused this change. |
|
I have a gitlab CI build link for the build log https://gitlab.alpinelinux.org/alpine/aports/-/jobs/1953786 (raw: https://gitlab.alpinelinux.org/alpine/aports/-/jobs/1953786/raw, relevant error at line 7461). For -rc1 i simply reverted this patch and it succeeded, i'm currently trying to do the same for -rc2. |
|
I've pushed #151293 to remove the cfi information from the test checks. Can you let me know if the failure goes away with that? |
|
Yes indeed that fixes the test, thank you! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This will convert loads of constant strings to immediate values. Put this behind a flag that is enabled by default so that we can toggle it if need be.