ebevhan added inline comments.
================
Comment at: llvm/include/llvm/IR/FixedPointBuilder.h:126
+ /// \p Ty, or a floating point type with a larger exponent than Ty.
+ Type *getAccommodatingFloatType(Type *Ty, const FixedPointSemantics &Sema) {
+ const fltSemantics *FloatSema = &Ty->getFltSemantics();
----------------
rjmccall wrote:
> I like this method name. Does this have the same problem as I asked about in
> the other patch about really needing to be about whether the *unscaled*
> fixed-point type fits in the given type?
It should be the same case here, yes. Both the integral range and the scale
matter when determining this.
================
Comment at: llvm/include/llvm/IR/FixedPointBuilder.h:131
+ // There's seemingly no way to convert fltSemantics to Type.
+ return ConstantFP::get(Ty->getContext(), APFloat(*FloatSema))->getType();
+ }
----------------
rjmccall wrote:
> Could you just extract that code out of `llvm::ConstantFP::get` and put it on
> `llvm::Type`? Might be better as a separate patch.
>
> While you're at it, there's also a `TypeToFloatSemantics` function in
> Constants.cpp that's completely redundant with `llvm::Type::getFltSemantics`.
Done in D87512.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86632/new/
https://reviews.llvm.org/D86632
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits