[llvm] 9b2fece - [BuildLibCalls][RISCV] Sign extend return value of bcmp on riscv64.
Craig Topper via llvm-commits
llvm-commits at lists.llvm.org
Tue Dec 20 11:47:30 PST 2022
Author: Craig Topper
Date: 2022-12-20T11:47:04-08:00
New Revision: 9b2fecec406d6a6bcda9fbb9251db2ae202c7400
URL: https://github.com/llvm/llvm-project/commit/9b2fecec406d6a6bcda9fbb9251db2ae202c7400
DIFF: https://github.com/llvm/llvm-project/commit/9b2fecec406d6a6bcda9fbb9251db2ae202c7400.diff
LOG: [BuildLibCalls][RISCV] Sign extend return value of bcmp on riscv64.
riscv64 wants callees to sign extend signed and unsigned int returns.
The caller can use this to avoid a sign extend if the result is
used by a comparison since riscv64 only has 64-bit compares.
InstCombine/SimplifyLibCalls aggressively turn memcmps that are only
used by an icmp eq 0 into bcmp, but we lose the signext attribute that
would have been present on the memcmp. This causes an unneeded sext.w
in the generated assembly.
This looks even sillier if bcmp is implemented alias to memcmp. In
that case, not only did we not get any savings by using bcmp, we added
an instruction.
This probably applies to other functions, this just happens to be
the one I noticed so far.
See also the discussion here https://discourse.llvm.org/t/can-we-preserve-signext-return-attribute-when-converting-memcmp-to-bcmp/67126
Reviewed By: efriedma
Differential Revision: https://reviews.llvm.org/D139901
Added:
llvm/test/Transforms/InstCombine/RISCV/memcmp.ll
Modified:
llvm/include/llvm/Analysis/TargetLibraryInfo.h
llvm/lib/Analysis/TargetLibraryInfo.cpp
llvm/lib/Transforms/Utils/BuildLibCalls.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/Analysis/TargetLibraryInfo.h b/llvm/include/llvm/Analysis/TargetLibraryInfo.h
index cb9da3eccd6d0..87cead103f585 100644
--- a/llvm/include/llvm/Analysis/TargetLibraryInfo.h
+++ b/llvm/include/llvm/Analysis/TargetLibraryInfo.h
@@ -52,7 +52,7 @@ class TargetLibraryInfoImpl {
unsigned char AvailableArray[(NumLibFuncs+3)/4];
DenseMap<unsigned, std::string> CustomNames;
static StringLiteral const StandardNames[NumLibFuncs];
- bool ShouldExtI32Param, ShouldExtI32Return, ShouldSignExtI32Param;
+ bool ShouldExtI32Param, ShouldExtI32Return, ShouldSignExtI32Param, ShouldSignExtI32Return;
unsigned SizeOfInt;
enum AvailabilityState {
@@ -189,6 +189,12 @@ class TargetLibraryInfoImpl {
ShouldSignExtI32Param = Val;
}
+ /// Set to true iff i32 results from library functions should have signext
+ /// attribute if they correspond to C-level int or unsigned int.
+ void setShouldSignExtI32Return(bool Val) {
+ ShouldSignExtI32Return = Val;
+ }
+
/// Returns the size of the wchar_t type in bytes or 0 if the size is unknown.
/// This queries the 'wchar_size' metadata.
unsigned getWCharSize(const Module &M) const;
@@ -401,6 +407,8 @@ class TargetLibraryInfo {
Attribute::AttrKind getExtAttrForI32Return(bool Signed = true) const {
if (Impl->ShouldExtI32Return)
return Signed ? Attribute::SExt : Attribute::ZExt;
+ if (Impl->ShouldSignExtI32Return)
+ return Attribute::SExt;
return Attribute::None;
}
diff --git a/llvm/lib/Analysis/TargetLibraryInfo.cpp b/llvm/lib/Analysis/TargetLibraryInfo.cpp
index 270fa9ccca01f..35811f7b04f17 100644
--- a/llvm/lib/Analysis/TargetLibraryInfo.cpp
+++ b/llvm/lib/Analysis/TargetLibraryInfo.cpp
@@ -169,7 +169,7 @@ static void initialize(TargetLibraryInfoImpl &TLI, const Triple &T,
TLI.setUnavailable(LibFunc_fgets_unlocked);
bool ShouldExtI32Param = false, ShouldExtI32Return = false,
- ShouldSignExtI32Param = false;
+ ShouldSignExtI32Param = false, ShouldSignExtI32Return = false;
// PowerPC64, Sparc64, SystemZ need signext/zeroext on i32 parameters and
// returns corresponding to C-level ints and unsigned ints.
if (T.isPPC64() || T.getArch() == Triple::sparcv9 ||
@@ -182,9 +182,15 @@ static void initialize(TargetLibraryInfoImpl &TLI, const Triple &T,
if (T.isMIPS() || T.isRISCV64()) {
ShouldSignExtI32Param = true;
}
+ // riscv64 needs signext on i32 returns corresponding to both signed and
+ // unsigned ints.
+ if (T.isRISCV64()) {
+ ShouldSignExtI32Return = true;
+ }
TLI.setShouldExtI32Param(ShouldExtI32Param);
TLI.setShouldExtI32Return(ShouldExtI32Return);
TLI.setShouldSignExtI32Param(ShouldSignExtI32Param);
+ TLI.setShouldSignExtI32Return(ShouldSignExtI32Return);
// Let's assume by default that the size of int is 32 bits, unless the target
// is a 16-bit architecture because then it most likely is 16 bits. If that
@@ -882,6 +888,7 @@ TargetLibraryInfoImpl::TargetLibraryInfoImpl(const TargetLibraryInfoImpl &TLI)
: CustomNames(TLI.CustomNames), ShouldExtI32Param(TLI.ShouldExtI32Param),
ShouldExtI32Return(TLI.ShouldExtI32Return),
ShouldSignExtI32Param(TLI.ShouldSignExtI32Param),
+ ShouldSignExtI32Return(TLI.ShouldSignExtI32Return),
SizeOfInt(TLI.SizeOfInt) {
memcpy(AvailableArray, TLI.AvailableArray, sizeof(AvailableArray));
VectorDescs = TLI.VectorDescs;
@@ -893,6 +900,7 @@ TargetLibraryInfoImpl::TargetLibraryInfoImpl(TargetLibraryInfoImpl &&TLI)
ShouldExtI32Param(TLI.ShouldExtI32Param),
ShouldExtI32Return(TLI.ShouldExtI32Return),
ShouldSignExtI32Param(TLI.ShouldSignExtI32Param),
+ ShouldSignExtI32Return(TLI.ShouldSignExtI32Return),
SizeOfInt(TLI.SizeOfInt) {
std::move(std::begin(TLI.AvailableArray), std::end(TLI.AvailableArray),
AvailableArray);
@@ -905,6 +913,7 @@ TargetLibraryInfoImpl &TargetLibraryInfoImpl::operator=(const TargetLibraryInfoI
ShouldExtI32Param = TLI.ShouldExtI32Param;
ShouldExtI32Return = TLI.ShouldExtI32Return;
ShouldSignExtI32Param = TLI.ShouldSignExtI32Param;
+ ShouldSignExtI32Return = TLI.ShouldSignExtI32Return;
SizeOfInt = TLI.SizeOfInt;
memcpy(AvailableArray, TLI.AvailableArray, sizeof(AvailableArray));
return *this;
@@ -915,6 +924,7 @@ TargetLibraryInfoImpl &TargetLibraryInfoImpl::operator=(TargetLibraryInfoImpl &&
ShouldExtI32Param = TLI.ShouldExtI32Param;
ShouldExtI32Return = TLI.ShouldExtI32Return;
ShouldSignExtI32Param = TLI.ShouldSignExtI32Param;
+ ShouldSignExtI32Return = TLI.ShouldSignExtI32Return;
SizeOfInt = TLI.SizeOfInt;
std::move(std::begin(TLI.AvailableArray), std::end(TLI.AvailableArray),
AvailableArray);
diff --git a/llvm/lib/Transforms/Utils/BuildLibCalls.cpp b/llvm/lib/Transforms/Utils/BuildLibCalls.cpp
index beb3351dddf14..1e21a2f85446d 100644
--- a/llvm/lib/Transforms/Utils/BuildLibCalls.cpp
+++ b/llvm/lib/Transforms/Utils/BuildLibCalls.cpp
@@ -1240,6 +1240,13 @@ static void setArgExtAttr(Function &F, unsigned ArgNo,
F.addParamAttr(ArgNo, ExtAttr);
}
+static void setRetExtAttr(Function &F,
+ const TargetLibraryInfo &TLI, bool Signed = true) {
+ Attribute::AttrKind ExtAttr = TLI.getExtAttrForI32Return(Signed);
+ if (ExtAttr != Attribute::None && !F.hasRetAttribute(ExtAttr))
+ F.addRetAttr(ExtAttr);
+}
+
// Modeled after X86TargetLowering::markLibCallAttributes.
static void markRegisterParameterAttributes(Function *F) {
if (!F->arg_size() || F->isVarArg())
@@ -1315,6 +1322,8 @@ FunctionCallee llvm::getOrInsertLibFunc(Module *M, const TargetLibraryInfo &TLI,
// on any target: A size_t argument (which may be an i32 on some targets)
// should not trigger the assert below.
case LibFunc_bcmp:
+ setRetExtAttr(*F, TLI);
+ break;
case LibFunc_calloc:
case LibFunc_fwrite:
case LibFunc_malloc:
diff --git a/llvm/test/Transforms/InstCombine/RISCV/memcmp.ll b/llvm/test/Transforms/InstCombine/RISCV/memcmp.ll
new file mode 100644
index 0000000000000..7447677ea4e75
--- /dev/null
+++ b/llvm/test/Transforms/InstCombine/RISCV/memcmp.ll
@@ -0,0 +1,19 @@
+; RUN: opt %s -passes=instcombine -mtriple=riscv64-unknown-linux-gnu -S | FileCheck %s
+
+declare signext i32 @memcmp(ptr, ptr, i64)
+
+; Make sure we use signext attribute for the bcmp result.
+define signext i32 @test_bcmp(ptr %mem1, ptr %mem2, i64 %size) {
+; CHECK-LABEL: define {{[^@]+}}@test_bcmp(
+; CHECK-NEXT: [[BCMP:%.*]] = call i32 @bcmp(ptr [[MEM1:%.*]], ptr [[MEM2:%.*]], i64 [[SIZE:%.*]])
+; CHECK-NEXT: [[CMP:%.*]] = icmp eq i32 [[BCMP]], 0
+; CHECK-NEXT: [[ZEXT:%.*]] = zext i1 [[CMP]] to i32
+; CHECK-NEXT: ret i32 [[ZEXT]]
+;
+ %call = call signext i32 @memcmp(ptr %mem1, ptr %mem2, i64 %size)
+ %cmp = icmp eq i32 %call, 0
+ %zext = zext i1 %cmp to i32
+ ret i32 %zext
+}
+
+; CHECK: declare signext i32 @bcmp(ptr nocapture, ptr nocapture, i64)
More information about the llvm-commits
mailing list