stefanp requested changes to this revision. stefanp added inline comments. This revision now requires changes to proceed.
================ Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.h:1321 bool convertToNonDenormSingle(APFloat &ArgAPFloat); + bool checkNonDenormCannotConvertToSingle(APInt &ArgAPInt); + bool checkNonDenormCannotConvertToSingle(APFloat &ArgAPFloat); ---------------- Is the APInt version of this function used anywere? ================ Comment at: llvm/lib/Target/PowerPC/PPCInstrInfo.td:412 + APFloat APFloatOfN = N->getValueAPF(); + checkNonDenormCannotConvertToSingle(APFloatOfN); + uint32_t Hi = (uint32_t)((APFloatOfN.bitcastToAPInt().getZExtValue() & ---------------- Why are we running this here? We don't check the return of the function so we must assume that it returns true. In that case the value of `APFloatOfN` won't change because `convertToNonDenormSingle` will only change the value of the parameter if it returns true. But `checkNonDenormCannotConvertToSingle` only returns true if `convertToNonDenormSingle` return false. ================ Comment at: llvm/test/CodeGen/PowerPC/constant-pool.ll:44 +; CHECK-NEXT: xxsplti32dx vs1, 0, 56623104 +; CHECK-NEXT: xxsplti32dx vs1, 1, -609716532 +; CHECK-NEXT: # kill: def $f1 killed $f1 killed $vsl1 ---------------- I'm looking to understand this test case. We are trying to materialize a special PowerPC long double (double-double). It seems that we have materialized one half of it and not the other half. Is it because the first half is a denormal? Why are we avoiding denormals anyway? It seems like we can completely specify a 64 bit double with two `xxsplti32dx` instructions. ================ Comment at: llvm/test/CodeGen/PowerPC/constant-pool.ll:363 +; CHECK-NEXT: stxv vs3, 32(r1) # 16-byte Folded Spill +; CHECK-NEXT: xxsplti32dx vs3, 1, -343597384 +; CHECK-NEXT: # kill: def $f3 killed $f3 killed $vsl3 ---------------- What is going on here? It almost looks like we are spilling `vs3` half way through materializing a constant. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95458/new/ https://reviews.llvm.org/D95458 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
