[PATCH] D68739: [GISel] Allow ConstantFoldBinOp to consider G_FCONSTANT binary representation for combines
Marcello Maggioni via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Oct 9 15:54:06 PDT 2019
kariddi marked an inline comment as done.
kariddi added inline comments.
================
Comment at: llvm/lib/CodeGen/GlobalISel/Utils.cpp:323-325
+ (&FPVal->getValueAPF().getSemantics() == &APFloat::IEEEdouble() ||
+ &FPVal->getValueAPF().getSemantics() == &APFloat::IEEEsingle() ||
+ &FPVal->getValueAPF().getSemantics() == &APFloat::IEEEhalf())) {
----------------
kariddi wrote:
> arsenm wrote:
> > Why do you need to whitelist these types?
> Are the types that fit a uint64 that we know of, but I guess that could be checked from "the only user" of this function instead
Actually, that's not necessarily true ... ConstantFoldBinOp used to use a function that only handled Optional<int64_t> . Now I substituted that with a function that returns APInt (for simplicity), but I wanted to keep the functionality the same as getConstantVRegVal() I guess ...
I guess I can remove that limitation for the Floats, but maintain the limitation fo the integers and then check in ConstantFoldBinOp.
I'm just scared that allowing ConstantFoldBinOp to digest things it never saw before would cause some "unexpected consequence" :-P
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68739/new/
https://reviews.llvm.org/D68739
More information about the llvm-commits
mailing list