[llvm] [AArch64] Fix a presumed typo in isFPImmLegal limit. NFC (PR #106716)
Marina Taylor via llvm-commits
llvm-commits at lists.llvm.org
Fri Aug 30 07:03:23 PDT 2024
https://github.com/citymarina updated https://github.com/llvm/llvm-project/pull/106716
>From 74667af109fb1f570f6884733ab079bd2206c920 Mon Sep 17 00:00:00 2001
From: Marina Taylor <marina_taylor at apple.com>
Date: Fri, 30 Aug 2024 12:36:50 +0100
Subject: [PATCH 1/2] [AArch64] Fix a presumed typo in isFPImmLegal limit. NFC
The worst possible case for a double literal goes like:
```
mov ...
movk ..., lsl #16
movk ..., lsl #32
movk ..., lsl #48
fmov ...
```
The limit of 5 in the code gives the impression that `Insn` includes
all instructions including the `fmov`, but that's not true. It only
counts the integer moves. This led me astray on some other work in
this area.
---
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
index 02390e0a85c0a5..98f6f30112a8c7 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -11463,7 +11463,8 @@ bool AArch64TargetLowering::isFPImmLegal(const APFloat &Imm, EVT VT,
// movw+movk is fused). So we limit up to 2 instrdduction at most.
SmallVector<AArch64_IMM::ImmInsnModel, 4> Insn;
AArch64_IMM::expandMOVImm(ImmInt.getZExtValue(), VT.getSizeInBits(), Insn);
- unsigned Limit = (OptForSize ? 1 : (Subtarget->hasFuseLiterals() ? 5 : 2));
+ assert(Insn.size() <= 4);
+ unsigned Limit = (OptForSize ? 1 : (Subtarget->hasFuseLiterals() ? 4 : 2));
IsLegal = Insn.size() <= Limit;
}
>From a39c48780f8b6943bb46d8dd366a07a1937ace46 Mon Sep 17 00:00:00 2001
From: Marina Taylor <marina_taylor at apple.com>
Date: Fri, 30 Aug 2024 15:02:56 +0100
Subject: [PATCH 2/2] Add message to assert
---
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
index 98f6f30112a8c7..17772e0502f652 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -11463,7 +11463,8 @@ bool AArch64TargetLowering::isFPImmLegal(const APFloat &Imm, EVT VT,
// movw+movk is fused). So we limit up to 2 instrdduction at most.
SmallVector<AArch64_IMM::ImmInsnModel, 4> Insn;
AArch64_IMM::expandMOVImm(ImmInt.getZExtValue(), VT.getSizeInBits(), Insn);
- assert(Insn.size() <= 4);
+ assert(Insn.size() <= 4 &&
+ "Should be able to build any value with at most 4 moves");
unsigned Limit = (OptForSize ? 1 : (Subtarget->hasFuseLiterals() ? 4 : 2));
IsLegal = Insn.size() <= Limit;
}
More information about the llvm-commits
mailing list