-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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.
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
Add f16
and f128
#114607
Add f16
and f128
#114607
Changes from all commits
f769f48
b47b41f
53e4ec2
b250f14
4dedc87
572af57
53c4802
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,8 +33,10 @@ pub(crate) fn scalar_to_clif_type(tcx: TyCtxt<'_>, scalar: Scalar) -> Type { | |
Integer::I64 => types::I64, | ||
Integer::I128 => types::I128, | ||
}, | ||
Primitive::F16 => unimplemented!("f16 is not yet implemented"), | ||
Primitive::F32 => types::F32, | ||
Primitive::F64 => types::F64, | ||
Primitive::F128 => unimplemented!("f128 is not yet implemented"), | ||
// FIXME(erikdesjardins): handle non-default addrspace ptr sizes | ||
Primitive::Pointer(_) => pointer_ty(tcx), | ||
} | ||
|
@@ -61,8 +63,10 @@ fn clif_type_from_ty<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> Option<types::Typ | |
}, | ||
ty::Char => types::I32, | ||
ty::Float(size) => match size { | ||
FloatTy::F16 => unimplemented!("f16 is not yet implemented"), | ||
FloatTy::F32 => types::F32, | ||
FloatTy::F64 => types::F64, | ||
FloatTy::F128 => unimplemented!("f128 is not yet implemented"), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Won't this cause panics when compiling the standard library? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was assuming not because the cranelift tests seem to still pass when I run locally. But I don't know what the exact combination of rustc, cranelift, and bootstrap settings is. Is there something better to do here? If you have a patch I will apply it during my next rebase, or I can make the change if it's something easy. @antoyo I do a similar thing for cg_gcc, what do you suggest there? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cg_clif inside the rust repo reuses the llvm built standard library but the standalone repo uses builds the standard library using cg_clif. cg_gcc can't reuse the llvm built standard library as it depends on all crates being compiled with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess if the CI for cg_gcc indeed fails, you could try using the supported types (F32, F64) and I'll fix this on my side when doing the next sync. |
||
}, | ||
ty::FnPtr(_) => pointer_ty(tcx), | ||
ty::RawPtr(TypeAndMut { ty: pointee_ty, mutbl: _ }) | ty::Ref(_, pointee_ty, _) => { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a gate here but this doesn't seem right since it can't catch inferred types. I'm not sure where to do a check after types are resolved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, you need to specify the type somewhere to be able to infer it, so the gate will apply at the definition place. I think constructing a
f128
via inference if the type is specified in a different crate is the only possible problem here, and I'm not sure how much of an issue that is since thefeature(...)
still has to exist somewhere in the crate graph.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@compiler-errors do you have any suggestion here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the gating is also incorrect the other way, it will deny valid code like
struct f16; f16;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, that is unfortunate. What is the better way to do this?