[PATCH] D68739: [GISel] Allow ConstantFoldBinOp to consider G_FCONSTANT binary representation for combines

[Marcello Maggioni via Phabricator via llvm-commits]

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