[PATCH] D44128: [GISel]: Add helpers for easy building G_FCONSTANT along with matchers

[Aditya Nandakumar via Phabricator via llvm-commits]

aditya_nandakumar marked 2 inline comments as done.
aditya_nandakumar added inline comments.


================
Comment at: lib/CodeGen/GlobalISel/MachineIRBuilder.cpp:291
+  LLT DstTy = MRI->getType(Res);
+  assert(!DstTy.isValid() || (DstTy.getSizeInBits() >= Ty.getSizeInBits()) &&
+                                 "DstReg not large enough for Ty");
----------------
volkan wrote:
> Could you check this in MachineVerifier as well?
I can do this in a separate commit as this is only adding builder helpers vs adding verifier + fixing tests.


================
Comment at: lib/CodeGen/GlobalISel/MachineIRBuilder.cpp:295
+  auto &Ctx = MF->getFunction().getContext();
+  auto *CFP = ConstantFP::get(Ctx, getAPFloatFromSize(Val, Ty.getSizeInBits()));
+  return buildFConstant(Res, *CFP);
----------------
volkan wrote:
> We already know DstTy here, can't we just use DstTy instead of Ty?
I was hoping to avoid needless getGenericVirtualReg() -> getType() lookups.
I've gone ahead and removed it for now.


================
Comment at: unittests/CodeGen/GlobalISel/PatternMatchTest.cpp:292
+  APFloat APF16(.5);
+  APF16.convert(LLTToAPFloatSemantics(s16), APFloat::rmNearestTiesToEven,
+                &Ignored);
----------------
volkan wrote:
> This can be replaced with `getAPFloatFromSize(0.5, 16)`.
I was hoping to test that part as well here (getting the correct APFloat).


https://reviews.llvm.org/D44128