Skip to content
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.

Sign up for GitHub

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

[CIR][CIRGen] Add support for __fp16 type #950

Merged
merged 1 commit into from
Oct 9, 2024
Merged

[CIR][CIRGen] Add support for __fp16 type #950

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 56 additions & 8 deletions clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -502,16 +502,25 @@ class ScalarExprEmitter : public StmtVisitor<ScalarExprEmitter, mlir::Value> {
// TODO(cir): CGFPOptionsRAII
assert(!MissingFeatures::CGFPOptionsRAII());

if (type->isHalfType() && !CGF.getContext().getLangOpts().NativeHalfType)
llvm_unreachable("__fp16 type NYI");
if (type->isHalfType() &&
!CGF.getContext().getLangOpts().NativeHalfType) {
// Another special case: half FP increment should be done via float
if (CGF.getContext().getTargetInfo().useFP16ConversionIntrinsics()) {
llvm_unreachable("cast via llvm.convert.from.fp16 is NYI");
} else {
value = Builder.createCast(CGF.getLoc(E->getExprLoc()),
mlir::cir::CastKind::floating, input,
CGF.CGM.FloatTy);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have a real world case for this else branch to test?
I'm thinking about how we lower this CastOp in current LLVM Lowering.
it looks like it may exercise this lowering code
rewriter.replaceOpWithNewOp<mlir::LLVM::FPExtOp>
And mlir::LLVM::FPExtOp is expecting floating type arg,
But your type converter converts cir.f16.storage into MLIR::IntegerType.....

Copy link
Member Author

@Lancern Lancern Oct 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This else branch is the normal case and it is hit when input is of type !cir.f16. When input is of type !cir.f16.storage, the then branch would be taken instead.

Do we have a real world case for this else branch to test?

Well since the else branch is the normal one, let's consider instead the then branch. I searched in the upstream clang tests with llvm.convert.from.fp16 and remarkably I found nothing. For most targets the useFP16ConversionIntrinsics() function call would return false, which indicates for most targets the else branch would be taken. But there are indeed some exceptions, for example mips-unknown-linux-gnu: https://godbolt.org/z/feGvPYGxE , unfortunately CIR now supports none of these exceptions.

Anyway for now I think I'm going to remove the !cir.f16.storage stuff and make it assert on relevant paths, including the then branch here.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, you are right, the else branch is the most common case, and it should takes existing cir.f16 type as input. And lowering supports it without problem.

}
}

if (mlir::isa<mlir::cir::SingleType, mlir::cir::DoubleType>(
value.getType())) {
// Create the inc/dec operation.
// NOTE(CIR): clang calls CreateAdd but folds this to a unary op
auto kind =
(isInc ? mlir::cir::UnaryOpKind::Inc : mlir::cir::UnaryOpKind::Dec);
value = buildUnaryOp(E, kind, input);
value = buildUnaryOp(E, kind, value);
} else {
// Remaining types are Half, Bfloat16, LongDouble, __ibm128 or
// __float128. Convert from float.
Expand All @@ -537,8 +546,16 @@ class ScalarExprEmitter : public StmtVisitor<ScalarExprEmitter, mlir::Value> {
value = Builder.createBinop(value, mlir::cir::BinOpKind::Add, amt);
}

if (type->isHalfType() && !CGF.getContext().getLangOpts().NativeHalfType)
llvm_unreachable("NYI");
if (type->isHalfType() &&
!CGF.getContext().getLangOpts().NativeHalfType) {
if (CGF.getContext().getTargetInfo().useFP16ConversionIntrinsics()) {
llvm_unreachable("cast via llvm.convert.to.fp16 is NYI");
} else {
value = Builder.createCast(CGF.getLoc(E->getExprLoc()),
mlir::cir::CastKind::floating, value,
input.getType());
}
}

} else if (type->isFixedPointType()) {
llvm_unreachable("no fixed point inc/dec yet");
Expand Down Expand Up @@ -1043,7 +1060,23 @@ class ScalarExprEmitter : public StmtVisitor<ScalarExprEmitter, mlir::Value> {
// Cast from half through float if half isn't a native type.
if (SrcType->isHalfType() &&
!CGF.getContext().getLangOpts().NativeHalfType) {
llvm_unreachable("not implemented");
// Cast to FP using the intrinsic if the half type itself isn't supported.
if (mlir::isa<mlir::cir::CIRFPTypeInterface>(DstTy)) {
if (CGF.getContext().getTargetInfo().useFP16ConversionIntrinsics())
llvm_unreachable("cast via llvm.convert.from.fp16 is NYI");
} 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()) {
llvm_unreachable("cast via llvm.convert.from.fp16 is NYI");
} else {
Src = Builder.createCast(
CGF.getLoc(Loc), mlir::cir::CastKind::floating, Src, CGF.FloatTy);
}
SrcType = CGF.getContext().FloatTy;
SrcTy = CGF.FloatTy;
}
}

// TODO(cir): LLVM codegen ignore conversions like int -> uint,
Expand Down Expand Up @@ -1098,13 +1131,28 @@ class ScalarExprEmitter : public StmtVisitor<ScalarExprEmitter, mlir::Value> {
// Cast to half through float if half isn't a native type.
if (DstType->isHalfType() &&
!CGF.getContext().getLangOpts().NativeHalfType) {
llvm_unreachable("NYI");
// Make sure we cast in a single step if from another FP type.
if (mlir::isa<mlir::cir::CIRFPTypeInterface>(SrcTy)) {
// 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())
llvm_unreachable("cast via llvm.convert.to.fp16 is NYI");
// If the half type is supported, just use an fptrunc.
return Builder.createCast(CGF.getLoc(Loc),
mlir::cir::CastKind::floating, Src, DstTy);
}
DstTy = CGF.FloatTy;
}

Res = buildScalarCast(Src, SrcType, DstType, SrcTy, DstTy, Opts);

if (DstTy != ResTy) {
llvm_unreachable("NYI");
if (CGF.getContext().getTargetInfo().useFP16ConversionIntrinsics()) {
llvm_unreachable("cast via llvm.convert.to.fp16 is NYI");
} else {
Res = Builder.createCast(CGF.getLoc(Loc), mlir::cir::CastKind::floating,
Res, ResTy);
}
}

if (Opts.EmitImplicitIntegerTruncationChecks)
Expand Down
7 changes: 5 additions & 2 deletions clang/lib/CIR/CodeGen/CIRGenTypes.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -469,8 +469,11 @@ mlir::Type CIRGenTypes::ConvertType(QualType T) {
ResultType = CGM.FP16Ty;
break;
case BuiltinType::Half:
// Should be the same as above?
assert(0 && "not implemented");
if (Context.getLangOpts().NativeHalfType ||
!Context.getTargetInfo().useFP16ConversionIntrinsics())
ResultType = CGM.FP16Ty;
else
llvm_unreachable("NYI");
break;
case BuiltinType::BFloat16:
ResultType = CGM.BFloat16Ty;
Expand Down
Loading