[PATCH] D62555: [TailDuplicator] prevent tail duplication for INLINEASM_BR

Fri May 31 05:30:56 PDT 2019

peter.smith added a comment.

In D62555#1523932 <https://reviews.llvm.org/D62555#1523932>, @nickdesaulniers wrote:

> > so probably a bug in if conversion. Maybe the same issue as https://bugs.llvm.org/show_bug.cgi?id=41121 .
>
> Interesting, and does look related.  While I try to make If Converter more robust in the face of INLINEASM_BR, I still think curtailing Tail Duplicator from duplicating INLINEASM_BR (and INLINEASM) is probably also worthwhile, and kind of makes robustness changes to If Converter less important.  I'm going to spend the rest of the day trying to improve If Converter, but ultimately this is holding up asm goto from working perfectly (which is more important to me), so if I don't have anything working by EOD, then tomorrow I will make the suggested edits here and focus on this patch.

I don't have a strong opinion here. I'm happy to disable the duplication for INLINEASM_BR at this point if we can capture the information that we could remove the restriction with a more sophisticated if conversion.

================
Comment at: llvm/lib/CodeGen/TailDuplicator.cpp:630
+    // case INLINEASM_BR.
+    if (MI.getOpcode() == TargetOpcode::INLINEASM_BR)
+      return false;
----------------
Is it worth something like "FIXME: improve if conversion to not consider blocks with INLINEASM_BR to remove this restriction." ? To record that improvements are possible.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62555