nickdesaulniers planned changes to this revision. nickdesaulniers added a comment.
In D136497#3889938 <https://reviews.llvm.org/D136497#3889938>, @void wrote: > It might be easier to see the main changes here if you submit the (very nice) > refactoring of `EmitAsmStores` first. D137113 <https://reviews.llvm.org/D137113> TODO(Nick): rebase on D137113 <https://reviews.llvm.org/D137113> ================ Comment at: clang/lib/CodeGen/CGStmt.cpp:2358 + // the expression, do the conversion. + if (ResultRegTypes[i] != ResultTruncRegTypes[i]) { + ---------------- void wrote: > s/ResultTruncRegTypes[i]/TruncTy/ Done in D137113 ================ Comment at: clang/lib/CodeGen/CGStmt.cpp:2362 + // a pointer. + if (TruncTy->isFloatingPointTy()) + Tmp = Builder.CreateFPTrunc(Tmp, TruncTy); ---------------- void wrote: > This looks like a direct copy from below (which is fine). I'm a bit iffy on > whether this covers *all* of the value types that could come through here... Note: this code was simply moved wholesale. If value types are missing, that is a pre-existing bug and should be a prerequisite to fix for this patch. I've broken out D137113 as a child patch to make that clearer. ================ Comment at: clang/lib/CodeGen/CGStmt.cpp:2850 assert(RegResults.size() == ResultRegTypes.size()); assert(RegResults.size() == ResultTruncRegTypes.size()); ---------------- void wrote: > Should these asserts (or some of them) be moved into `EmitAsmStores()`? Ah, yeah, let me add those to D137113. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136497/new/ https://reviews.llvm.org/D136497 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
