[PATCH] D136497: [Clang] support for outputs along indirect edges of asm goto

[Nick Desaulniers via Phabricator via cfe-commits]

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