[PATCH] D95745: Support unwinding from inline assembly

[Amanieu d'Antras via Phabricator via llvm-commits]

Amanieu added a comment.

The new operand to InlineAsm needs to be handled in llvm/lib/Transforms/Utils/ValueMapper.cpp otherwise you will end up with a bug similar to https://bugs.llvm.org/show_bug.cgi?id=45291.

Some tests with other unwinding models such as SJLJ and SEH would be nice.



================
Comment at: clang/lib/CodeGen/CGStmt.cpp:2522
+  assert(!(HasUnwindClobber && IsGCCAsmGoto) &&
+         "unwind clobber can't be used with asm goto");
+
----------------
This should be a compiler error diagnostic in SemaAsmStmt.cpp rather than an assert.


================
Comment at: llvm/lib/AsmParser/LLParser.cpp:5471
                        (ID.UIntVal >> 1) & 1,
-                       (InlineAsm::AsmDialect(ID.UIntVal >> 2)));
+                       (InlineAsm::AsmDialect(ID.UIntVal >> 2)),
+                       (ID.UIntVal >> 3) & 1);
----------------



================
Comment at: llvm/lib/Bitcode/Reader/BitcodeReader.cpp:2857
+      bool IsAlignStack = (Record[0] >> 1) & 1;
+      unsigned AsmDialect = Record[0] >> 2;
+      bool CanThrow = (Record[0] >> 3) & 1;
----------------



================
Comment at: llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp:2446
+    if (!IA->canThrow()) {
+      // Fast path without emitting EH_LABELs.
+
----------------
Is this fast path actually useful? The frontend will almost never emit an invoke instruction for inline asm that can't unwind.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:8347
+    Chain = DAG.getEHLabel(getCurSDLoc(), Chain, BeginLabel);
+  }
+
----------------
This code is likely to get out of sync with the one in visitInvokable. It would be nice to share the code in a single place, but if that is not practical then at least add a comment in visitInvokable to remind anyone modifying that code to apply the same changes here as well.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95745/new/

https://reviews.llvm.org/D95745