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

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

I can see how this change would prevent the TailDuplication pass from duplicating blocks with callbr instructions, however I'm not sure if that is sufficient, or if it is too restrictive.

- Is it possible to write C code that results in IR as if the TailDuplication pass has duplicated the callbr, or is there a fundamental part of asm goto that prevents this from happening?
- Do we know of other passes that might fail due to TailDuplication of callbr?
- Could this be fixed in the if conversion pass  IfConverter::ScanInstructions() by having TargetInstrInfo::isPredicable(MachineInstr&) reject TargetOpcode::INLINEASM_BR?

If there are many other things that are broken by duplicating callbr then this patch is definitely the right thing to do. If it is just if conversion then it may be best to fix it there? Apologies I'm not an expert in this area, so I'm happy to defer. May be best to try some more reviewers to take a look.



================
Comment at: llvm/lib/CodeGen/TailDuplicator.cpp:783
   if (!PredCond.empty())
     return false;
+  for (MachineInstr &I : *TailBB)
----------------
I think it will be worth a comment saying why we can't tail duplicate blocks with asm goto?


================
Comment at: llvm/lib/CodeGen/TailDuplicator.cpp:784
     return false;
+  for (MachineInstr &I : *TailBB)
+    if (I.getOpcode() == TargetOpcode::INLINEASM_BR)
----------------
Is the INLINEASM_BR more likely to terminate the basic block, if it is then maybe a reverse iterator will find them faster? Although I suspect that most basic blocks won't contain any so it won't matter much anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62555