IR: Remove llvm.convert.to.fp16 and llvm.convert.from.fp16 intrinsics #174484
+607
−2,569
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
These are long overdue for removal. These were originally a hack to support loading half values before there was any / decent support for the half type through the backend. There's no reason to continue supporting these, they're equivalent to fpext/fptrunc with a bitcast. SelectionDAG stopped translating these directly, and used the bitcast + fp cast since f7a02c1, so there's been no reason to use these since 2014.
Contributor
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Member
|
@llvm/pr-subscribers-clangir @llvm/pr-subscribers-llvm-ir Author: Matt Arsenault (arsenm) ChangesThese are long overdue for removal. These were originally a hack SelectionDAG stopped translating these directly, and used the Patch is 150.71 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/174484.diff 53 Files Affected:
diff --git a/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h
index 4ff77bb64cf1c..f1aeeb13dcb57 100644
--- a/clang/include/clang/Basic/TargetInfo.h
+++ b/clang/include/clang/Basic/TargetInfo.h
@@ -1013,14 +1013,6 @@ class TargetInfo : public TransferrableTargetInfo,
return ComplexLongDoubleUsesFP2Ret;
}
- /// Check whether llvm intrinsics such as llvm.convert.to.fp16 should be used
- /// to convert to and from __fp16.
- /// FIXME: This function should be removed once all targets stop using the
- /// conversion intrinsics.
- virtual bool useFP16ConversionIntrinsics() const {
- return true;
- }
-
/// Specify if mangling based on address space map should be used or
/// not for language specific address spaces
bool useAddressSpaceMapMangling() const {
diff --git a/clang/lib/Basic/Targets/AArch64.h b/clang/lib/Basic/Targets/AArch64.h
index 2d3b8d2a8d950..2d2d469a5c7d6 100644
--- a/clang/lib/Basic/Targets/AArch64.h
+++ b/clang/lib/Basic/Targets/AArch64.h
@@ -154,10 +154,6 @@ class LLVM_LIBRARY_VISIBILITY AArch64TargetInfo : public TargetInfo {
llvm::APInt getFMVPriority(ArrayRef<StringRef> Features) const override;
- bool useFP16ConversionIntrinsics() const override {
- return false;
- }
-
void getTargetDefinesARMV81A(const LangOptions &Opts,
MacroBuilder &Builder) const;
void getTargetDefinesARMV82A(const LangOptions &Opts,
diff --git a/clang/lib/Basic/Targets/AMDGPU.h b/clang/lib/Basic/Targets/AMDGPU.h
index 0076f822c02a1..0ec4c50bde8c4 100644
--- a/clang/lib/Basic/Targets/AMDGPU.h
+++ b/clang/lib/Basic/Targets/AMDGPU.h
@@ -268,8 +268,6 @@ class LLVM_LIBRARY_VISIBILITY AMDGPUTargetInfo final : public TargetInfo {
llvm::SmallVector<Builtin::InfosShard> getTargetBuiltins() const override;
- bool useFP16ConversionIntrinsics() const override { return false; }
-
void getTargetDefines(const LangOptions &Opts,
MacroBuilder &Builder) const override;
diff --git a/clang/lib/Basic/Targets/ARM.h b/clang/lib/Basic/Targets/ARM.h
index 43c4718f4735b..00c2918bd2f23 100644
--- a/clang/lib/Basic/Targets/ARM.h
+++ b/clang/lib/Basic/Targets/ARM.h
@@ -177,10 +177,6 @@ class LLVM_LIBRARY_VISIBILITY ARMTargetInfo : public TargetInfo {
bool setFPMath(StringRef Name) override;
- bool useFP16ConversionIntrinsics() const override {
- return false;
- }
-
void getTargetDefinesARMV81A(const LangOptions &Opts,
MacroBuilder &Builder) const;
void getTargetDefinesARMV82A(const LangOptions &Opts,
diff --git a/clang/lib/Basic/Targets/DirectX.h b/clang/lib/Basic/Targets/DirectX.h
index 7589b4309ebf5..ca470e126795f 100644
--- a/clang/lib/Basic/Targets/DirectX.h
+++ b/clang/lib/Basic/Targets/DirectX.h
@@ -68,7 +68,6 @@ class LLVM_LIBRARY_VISIBILITY DirectXTargetInfo : public TargetInfo {
resetDataLayout();
TheCXXABI.set(TargetCXXABI::GenericItanium);
}
- bool useFP16ConversionIntrinsics() const override { return false; }
void getTargetDefines(const LangOptions &Opts,
MacroBuilder &Builder) const override;
diff --git a/clang/lib/Basic/Targets/LoongArch.h b/clang/lib/Basic/Targets/LoongArch.h
index 31afd3eed96f9..45c0bd2896993 100644
--- a/clang/lib/Basic/Targets/LoongArch.h
+++ b/clang/lib/Basic/Targets/LoongArch.h
@@ -105,8 +105,6 @@ class LLVM_LIBRARY_VISIBILITY LoongArchTargetInfo : public TargetInfo {
bool hasBFloat16Type() const override { return true; }
- bool useFP16ConversionIntrinsics() const override { return false; }
-
bool handleTargetFeatures(std::vector<std::string> &Features,
DiagnosticsEngine &Diags) override;
diff --git a/clang/lib/Basic/Targets/NVPTX.h b/clang/lib/Basic/Targets/NVPTX.h
index 6338a4f2f9036..b928fc800ecaa 100644
--- a/clang/lib/Basic/Targets/NVPTX.h
+++ b/clang/lib/Basic/Targets/NVPTX.h
@@ -81,8 +81,6 @@ class LLVM_LIBRARY_VISIBILITY NVPTXTargetInfo : public TargetInfo {
llvm::SmallVector<Builtin::InfosShard> getTargetBuiltins() const override;
- bool useFP16ConversionIntrinsics() const override { return false; }
-
bool
initFeatureMap(llvm::StringMap<bool> &Features, DiagnosticsEngine &Diags,
StringRef CPU,
diff --git a/clang/lib/Basic/Targets/RISCV.h b/clang/lib/Basic/Targets/RISCV.h
index 685735b54a45b..9441204e02bc8 100644
--- a/clang/lib/Basic/Targets/RISCV.h
+++ b/clang/lib/Basic/Targets/RISCV.h
@@ -113,10 +113,6 @@ class RISCVTargetInfo : public TargetInfo {
CallingConvCheckResult checkCallingConvention(CallingConv CC) const override;
- bool useFP16ConversionIntrinsics() const override {
- return false;
- }
-
bool isValidCPUName(StringRef Name) const override;
void fillValidCPUList(SmallVectorImpl<StringRef> &Values) const override;
bool isValidTuneCPUName(StringRef Name) const override;
diff --git a/clang/lib/Basic/Targets/SPIR.h b/clang/lib/Basic/Targets/SPIR.h
index d5374602caeaa..d05c0f01603d3 100644
--- a/clang/lib/Basic/Targets/SPIR.h
+++ b/clang/lib/Basic/Targets/SPIR.h
@@ -168,10 +168,6 @@ class LLVM_LIBRARY_VISIBILITY BaseSPIRTargetInfo : public TargetInfo {
}
public:
- // SPIR supports the half type and the only llvm intrinsic allowed in SPIR is
- // memcpy as per section 3 of the SPIR spec.
- bool useFP16ConversionIntrinsics() const override { return false; }
-
llvm::SmallVector<Builtin::InfosShard> getTargetBuiltins() const override {
return {};
}
diff --git a/clang/lib/Basic/Targets/SystemZ.h b/clang/lib/Basic/Targets/SystemZ.h
index 2bcf75deb0a91..80032da545e53 100644
--- a/clang/lib/Basic/Targets/SystemZ.h
+++ b/clang/lib/Basic/Targets/SystemZ.h
@@ -106,8 +106,6 @@ class LLVM_LIBRARY_VISIBILITY SystemZTargetInfo : public TargetInfo {
unsigned getMinGlobalAlign(uint64_t Size, bool HasNonWeakDef) const override;
- bool useFP16ConversionIntrinsics() const override { return false; }
-
void getTargetDefines(const LangOptions &Opts,
MacroBuilder &Builder) const override;
diff --git a/clang/lib/Basic/Targets/WebAssembly.h b/clang/lib/Basic/Targets/WebAssembly.h
index 3634330ec6698..ca8e3990cc54d 100644
--- a/clang/lib/Basic/Targets/WebAssembly.h
+++ b/clang/lib/Basic/Targets/WebAssembly.h
@@ -110,7 +110,6 @@ class LLVM_LIBRARY_VISIBILITY WebAssemblyTargetInfo : public TargetInfo {
StringRef getABI() const override;
bool setABI(const std::string &Name) override;
- bool useFP16ConversionIntrinsics() const override { return !HasFP16; }
protected:
void getTargetDefines(const LangOptions &Opts,
diff --git a/clang/lib/Basic/Targets/X86.h b/clang/lib/Basic/Targets/X86.h
index 922e32906cd04..d00f0acdee666 100644
--- a/clang/lib/Basic/Targets/X86.h
+++ b/clang/lib/Basic/Targets/X86.h
@@ -325,10 +325,6 @@ class LLVM_LIBRARY_VISIBILITY X86TargetInfo : public TargetInfo {
return "";
}
- bool useFP16ConversionIntrinsics() const override {
- return false;
- }
-
void getTargetDefines(const LangOptions &Opts,
MacroBuilder &Builder) const override;
diff --git a/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp b/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp
index ecb65d901de54..f797973683805 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExprConstant.cpp
@@ -1793,18 +1793,10 @@ mlir::Attribute ConstantEmitter::tryEmitPrivate(const APValue &value,
return cir::IntAttr::get(ty, value.getInt());
}
case APValue::Float: {
- const llvm::APFloat &init = value.getFloat();
- if (&init.getSemantics() == &llvm::APFloat::IEEEhalf() &&
- !cgm.getASTContext().getLangOpts().NativeHalfType &&
- cgm.getASTContext().getTargetInfo().useFP16ConversionIntrinsics()) {
- cgm.errorNYI("ConstExprEmitter::tryEmitPrivate half");
- return {};
- }
-
mlir::Type ty = cgm.convertType(destType);
assert(mlir::isa<cir::FPTypeInterface>(ty) &&
"expected floating-point type");
- return cir::FPAttr::get(ty, init);
+ return cir::FPAttr::get(ty, value.getFloat());
}
case APValue::Array: {
const ArrayType *arrayTy = cgm.getASTContext().getAsArrayType(destType);
diff --git a/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp b/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
index a06f1f1dc1784..4b2572ce4858d 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
@@ -924,19 +924,12 @@ class ScalarExprEmitter : public StmtVisitor<ScalarExprEmitter, mlir::Value> {
if (srcType->isHalfType() &&
!cgf.getContext().getLangOpts().NativeHalfType) {
// Cast to FP using the intrinsic if the half type itself isn't supported.
- if (mlir::isa<cir::FPTypeInterface>(mlirDstType)) {
- if (cgf.getContext().getTargetInfo().useFP16ConversionIntrinsics())
- cgf.getCIRGenModule().errorNYI(loc,
- "cast via llvm.convert.from.fp16");
- } else {
- // Cast to other types through float, using either the intrinsic or
- // FPExt, depending on whether the half type itself is supported (as
- // opposed to operations on half, available with NativeHalfType).
- if (cgf.getContext().getTargetInfo().useFP16ConversionIntrinsics())
- cgf.getCIRGenModule().errorNYI(loc,
- "cast via llvm.convert.from.fp16");
- // FIXME(cir): For now lets pretend we shouldn't use the conversion
- // intrinsics and insert a cast here unconditionally.
+ if (!mlir::isa<cir::FPTypeInterface>(mlirDstType)) {
+ // Cast to other types through float, using FPExt, depending on whether
+ // the half type itself is supported (as opposed to operations on half,
+ // available with NativeHalfType). FIXME(cir): For now lets pretend we
+ // shouldn't use the conversion intrinsics and insert a cast here
+ // unconditionally.
src = builder.createCast(cgf.getLoc(loc), cir::CastKind::floating, src,
cgf.floatTy);
srcType = cgf.getContext().FloatTy;
@@ -994,11 +987,6 @@ class ScalarExprEmitter : public StmtVisitor<ScalarExprEmitter, mlir::Value> {
res = emitScalarCast(src, srcType, dstType, mlirSrcType, mlirDstType, opts);
if (mlirDstType != resTy) {
- if (cgf.getContext().getTargetInfo().useFP16ConversionIntrinsics()) {
- cgf.getCIRGenModule().errorNYI(loc, "cast via llvm.convert.to.fp16");
- }
- // FIXME(cir): For now we never use FP16 conversion intrinsics even if
- // required by the target. Change that once this is implemented
res = builder.createCast(cgf.getLoc(loc), cir::CastKind::floating, res,
resTy);
}
diff --git a/clang/lib/CIR/CodeGen/CIRGenTypes.cpp b/clang/lib/CIR/CodeGen/CIRGenTypes.cpp
index cddc849180971..23875d100ac0e 100644
--- a/clang/lib/CIR/CodeGen/CIRGenTypes.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenTypes.cpp
@@ -396,13 +396,7 @@ mlir::Type CIRGenTypes::convertType(QualType type) {
resultType = cgm.fP16Ty;
break;
case BuiltinType::Half:
- if (astContext.getLangOpts().NativeHalfType ||
- !astContext.getTargetInfo().useFP16ConversionIntrinsics()) {
- resultType = cgm.fP16Ty;
- } else {
- cgm.errorNYI(SourceLocation(), "processing of built-in type", type);
- resultType = cgm.sInt32Ty;
- }
+ resultType = cgm.fP16Ty;
break;
case BuiltinType::BFloat16:
resultType = cgm.bFloat16Ty;
diff --git a/clang/lib/CodeGen/CGExprConstant.cpp b/clang/lib/CodeGen/CGExprConstant.cpp
index 0eec4dba4824a..26cb3f39bfbd0 100644
--- a/clang/lib/CodeGen/CGExprConstant.cpp
+++ b/clang/lib/CodeGen/CGExprConstant.cpp
@@ -2469,16 +2469,8 @@ ConstantEmitter::tryEmitPrivate(const APValue &Value, QualType DestType,
llvm::StructType::get(Complex[0]->getType(), Complex[1]->getType());
return llvm::ConstantStruct::get(STy, Complex);
}
- case APValue::Float: {
- const llvm::APFloat &Init = Value.getFloat();
- if (&Init.getSemantics() == &llvm::APFloat::IEEEhalf() &&
- !CGM.getContext().getLangOpts().NativeHalfType &&
- CGM.getContext().getTargetInfo().useFP16ConversionIntrinsics())
- return llvm::ConstantInt::get(CGM.getLLVMContext(),
- Init.bitcastToAPInt());
- else
- return llvm::ConstantFP::get(CGM.getLLVMContext(), Init);
- }
+ case APValue::Float:
+ return llvm::ConstantFP::get(CGM.getLLVMContext(), Value.getFloat());
case APValue::ComplexFloat: {
llvm::Constant *Complex[2];
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index 84421fef9f524..1b3486c86d493 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -1596,23 +1596,11 @@ Value *ScalarExprEmitter::EmitScalarConversion(Value *Src, QualType SrcType,
// Cast from half through float if half isn't a native type.
if (SrcType->isHalfType() && !CGF.getContext().getLangOpts().NativeHalfType) {
// Cast to FP using the intrinsic if the half type itself isn't supported.
- if (DstTy->isFloatingPointTy()) {
- if (CGF.getContext().getTargetInfo().useFP16ConversionIntrinsics())
- return Builder.CreateCall(
- CGF.CGM.getIntrinsic(llvm::Intrinsic::convert_from_fp16, DstTy),
- Src);
- } else {
+ if (!DstTy->isFloatingPointTy()) {
// Cast to other types through float, using either the intrinsic or FPExt,
// depending on whether the half type itself is supported
// (as opposed to operations on half, available with NativeHalfType).
- if (CGF.getContext().getTargetInfo().useFP16ConversionIntrinsics()) {
- Src = Builder.CreateCall(
- CGF.CGM.getIntrinsic(llvm::Intrinsic::convert_from_fp16,
- CGF.CGM.FloatTy),
- Src);
- } else {
- Src = Builder.CreateFPExt(Src, CGF.CGM.FloatTy, "conv");
- }
+ Src = Builder.CreateFPExt(Src, CGF.CGM.FloatTy, "conv");
SrcType = CGF.getContext().FloatTy;
SrcTy = CGF.FloatTy;
}
@@ -1723,11 +1711,6 @@ Value *ScalarExprEmitter::EmitScalarConversion(Value *Src, QualType SrcType,
if (DstType->isHalfType() && !CGF.getContext().getLangOpts().NativeHalfType) {
// Make sure we cast in a single step if from another FP type.
if (SrcTy->isFloatingPointTy()) {
- // Use the intrinsic if the half type itself isn't supported
- // (as opposed to operations on half, available with NativeHalfType).
- if (CGF.getContext().getTargetInfo().useFP16ConversionIntrinsics())
- return Builder.CreateCall(
- CGF.CGM.getIntrinsic(llvm::Intrinsic::convert_to_fp16, SrcTy), Src);
// If the half type is supported, just use an fptrunc.
return Builder.CreateFPTrunc(Src, DstTy);
}
@@ -1737,14 +1720,7 @@ Value *ScalarExprEmitter::EmitScalarConversion(Value *Src, QualType SrcType,
Res = EmitScalarCast(Src, SrcType, DstType, SrcTy, DstTy, Opts);
if (DstTy != ResTy) {
- if (CGF.getContext().getTargetInfo().useFP16ConversionIntrinsics()) {
- assert(ResTy->isIntegerTy(16) && "Only half FP requires extra conversion");
- Res = Builder.CreateCall(
- CGF.CGM.getIntrinsic(llvm::Intrinsic::convert_to_fp16, CGF.CGM.FloatTy),
- Res);
- } else {
- Res = Builder.CreateFPTrunc(Res, ResTy, "conv");
- }
+ Res = Builder.CreateFPTrunc(Res, ResTy, "conv");
}
if (Opts.EmitImplicitIntegerTruncationChecks)
@@ -3399,14 +3375,7 @@ ScalarExprEmitter::EmitScalarPrePostIncDec(const UnaryOperator *E, LValue LV,
if (type->isHalfType() && !CGF.getContext().getLangOpts().NativeHalfType) {
// Another special case: half FP increment should be done via float
- if (CGF.getContext().getTargetInfo().useFP16ConversionIntrinsics()) {
- value = Builder.CreateCall(
- CGF.CGM.getIntrinsic(llvm::Intrinsic::convert_from_fp16,
- CGF.CGM.FloatTy),
- input, "incdec.conv");
- } else {
- value = Builder.CreateFPExt(input, CGF.CGM.FloatTy, "incdec.conv");
- }
+ value = Builder.CreateFPExt(input, CGF.CGM.FloatTy, "incdec.conv");
}
if (value->getType()->isFloatTy())
@@ -3439,14 +3408,7 @@ ScalarExprEmitter::EmitScalarPrePostIncDec(const UnaryOperator *E, LValue LV,
value = Builder.CreateFAdd(value, amt, isInc ? "inc" : "dec");
if (type->isHalfType() && !CGF.getContext().getLangOpts().NativeHalfType) {
- if (CGF.getContext().getTargetInfo().useFP16ConversionIntrinsics()) {
- value = Builder.CreateCall(
- CGF.CGM.getIntrinsic(llvm::Intrinsic::convert_to_fp16,
- CGF.CGM.FloatTy),
- value, "incdec.conv");
- } else {
- value = Builder.CreateFPTrunc(value, input->getType(), "incdec.conv");
- }
+ value = Builder.CreateFPTrunc(value, input->getType(), "incdec.conv");
}
// Fixed-point types.
diff --git a/clang/lib/CodeGen/CodeGenTypes.cpp b/clang/lib/CodeGen/CodeGenTypes.cpp
index 4239552d1299e..080082a8dc652 100644
--- a/clang/lib/CodeGen/CodeGenTypes.cpp
+++ b/clang/lib/CodeGen/CodeGenTypes.cpp
@@ -471,10 +471,8 @@ llvm::Type *CodeGenTypes::ConvertType(QualType T) {
case BuiltinType::Half:
// Half FP can either be storage-only (lowered to i16) or native.
- ResultType = getTypeForFormat(
- getLLVMContext(), Context.getFloatTypeSemantics(T),
- Context.getLangOpts().NativeHalfType ||
- !Context.getTargetInfo().useFP16ConversionIntrinsics());
+ ResultType = getTypeForFormat(getLLVMContext(),
+ Context.getFloatTypeSemantics(T), true);
break;
case BuiltinType::LongDouble:
LongDoubleReferenced = true;
diff --git a/clang/lib/CodeGen/TargetBuiltins/NVPTX.cpp b/clang/lib/CodeGen/TargetBuiltins/NVPTX.cpp
index a4486965a851a..ee911f5258554 100644
--- a/clang/lib/CodeGen/TargetBuiltins/NVPTX.cpp
+++ b/clang/lib/CodeGen/TargetBuiltins/NVPTX.cpp
@@ -375,23 +375,8 @@ static Value *MakeCpAsync(unsigned IntrinsicID, unsigned IntrinsicIDS,
CGF.EmitScalarExpr(E->getArg(1))});
}
-static bool EnsureNativeHalfSupport(unsigned BuiltinID, const CallExpr *E,
- CodeGenFunction &CGF) {
- auto &C = CGF.CGM.getContext();
- if (!C.getLangOpts().NativeHalfType &&
- C.getTargetInfo().useFP16ConversionIntrinsics()) {
- CGF.CGM.Error(E->getExprLoc(), C.BuiltinInfo.getQuotedName(BuiltinID) +
- " requires native half type support.");
- return false;
- }
- return true;
-}
-
static Value *MakeHalfType(Function *Intrinsic, unsigned BuiltinID,
const CallExpr *E, CodeGenFunction &CGF) {
- if (!EnsureNativeHalfSupport(BuiltinID, E, CGF))
- return nullptr;
-
SmallVector<Value *, 16> Args;
auto *FTy = Intrinsic->getFunctionType();
unsigned ICEArguments = 0;
@@ -1069,13 +1054,10 @@ Value *CodeGenFunction::EmitNVPTXBuiltinExpr(unsigned BuiltinID,
EmitScalarExpr(E->getArg(0)));
case NVPTX::BI__nvvm_ldg_h:
case NVPTX::BI__nvvm_ldg_h2:
- return EnsureNativeHalfSupport(BuiltinID, E, *this) ? MakeLdg(*this, E)
- : nullptr;
+ return MakeLdg(*this, E);
case NVPTX::BI__nvvm_ldu_h:
case NVPTX::BI__nvvm_ldu_h2:
- return EnsureNativeHalfSupport(BuiltinID, E, *this)
- ? MakeLdu(Intrinsic::nvvm_ldu_global_f, *this, E)
- : nullptr;
+ return MakeLdu(Intrinsic::nvvm_ldu_global_f, *this, E);
case NVPTX::BI__nvvm_cp_async_ca_shared_global_4:
return MakeCpAsync(Intrinsic::nvvm_cp_async_ca_shared_global_4,
Intrinsic::nvvm_cp_async_ca_shared_global_4_s, *this, E,
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index ba4b25961d70d..daddeef808f73 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -5763,22 +5763,12 @@ bool Sema::BuiltinFPClassification(CallExpr *TheCall, unsigned NumArgs,
if (OrigArg->isTypeDependent())
return false;
- // Usual Unary Conversions will convert half to ...
[truncated]
|
arsenm
requested review from
RKSimon,
TNorthover,
davemgreen,
fhahn,
nikic and
topperc
arsenm
requested review from
andykaylor,
bcardosolopes,
lanza and
xlauko
as code owners
llvmbot
added
backend:ARM
backend:AArch64
backend:AMDGPU
backend:Hexagon
backend:MIPS
backend:RISC-V
backend:PowerPC
backend:SystemZ
backend:WebAssembly
backend:X86
clang:frontend
llvmbot
added
backend:loongarch
llvm:SelectionDAG
nikic
reviewed
Jan 5, 2026
Contributor
There was a problem hiding this comment.
Stopping use of these intrinsics in Clang should be split from the actual removal.
IIRC the last time I tried this this resulted incorrect codegen for targets that still don't use soft promote half float, which is why I did not submit it at the time. Did you confirm that ABI/codegen are unchanged e.g. on wasm?
Edit: I think the issue was that this implicitly also changes the ABI from passing in GPR to passing in FPR, because that's the default for half without soft promote half, which differs from what happens with the previous i16 use.
Contributor
Author
Given the codegen tests mostly passed and didn't strictly need to be removed, I'd say yes. For whatever reason we do run intrinsic auto upgrade on text inputs. There was one diff in a wasm test that looked cosmetic.
There can't be any change to the conversion libcall ABI because SelectionDAG hasn't been directly consuming these intrinsics since 2014. It was already legalizing based on FP_ROUND/FP_EXTEND. If there's an ABI problem it's on the clang side (one oddity was some of the FP builtin tests were emitting the f32 version of llvm.is.fpclass instead of f16 depending on whether the intrinsics were used or not). |
Contributor
|
The ABI problem is on the clang side in the sense that this changes the used type from |
arsenm
added a commit
that referenced
this pull request
Jan 5, 2026
There is no reason to use these over fpext/fptrunc and bitcast. Split out from #174484.
Contributor
Author
That's supposed to be controlled by NativeHalfType. I found a diff when using -Xclang -fnative-half-arguments-and-returns; some places were checking if the conversion intrinsics were used when the correct property was probably intended to be NativeHalfType |
arsenm
added a commit
that referenced
this pull request
Jan 6, 2026
There is no reason to use these over fpext/fptrunc and bitcast. Split out from #174484. The test coverage is also shockingly bad, so adds a new wasm test which shows different contexts the intrinsics are used. I've also reverted this to a more conservative version that leaves the useFP16ConversionIntrinsics configuration in place, and only replaces the exact intrinsic usage. This should be removed, but it seems to have turned into a buggy ABI option. Some contexts which probably meant to check NativeHalfType or NativeHalfArgsAndReturns were relying on this instead. Additionally, some of the SVE intrinsics appear to be using __fp16 but really expect _Float16 treatment.
arsenm
added a commit
that referenced
this pull request
Jan 6, 2026
There is no reason to use these over fpext/fptrunc and bitcast. Split out from #174484. The test coverage is also shockingly bad, so adds a new wasm test which shows different contexts the intrinsics are used. I've also reverted this to a more conservative version that leaves the useFP16ConversionIntrinsics configuration in place, and only replaces the exact intrinsic usage. This should be removed, but it seems to have turned into a buggy ABI option. Some contexts which probably meant to check NativeHalfType or NativeHalfArgsAndReturns were relying on this instead. Additionally, some of the SVE intrinsics appear to be using __fp16 but really expect _Float16 treatment.
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.
These are long overdue for removal. These were originally a hack
to support loading half values before there was any / decent support
for the half type through the backend. There's no reason to continue
supporting these, they're equivalent to fpext/fptrunc with a bitcast.
SelectionDAG stopped translating these directly, and used the
bitcast + fp cast since f7a02c1, so there's been no reason
to use these since 2014.