[PATCH] D82290: [CodeMoverUtils][WIP] Isolate checks strictly related to the code motion candidate instruction

[Whitney Tsang via Phabricator via llvm-commits]

Whitney added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/CodeMoverUtils.cpp:308
+bool llvm::isSafeToMove(Instruction &I) {
+  if (!(isa<LoadInst>(I) || isa<StoreInst>(I) || isa<CallInst>(I) ||
+        isa<FenceInst>(I) || isa<CastInst>(I) || isa<UnaryOperator>(I) ||
----------------
RithikSharma wrote:
> These conditions are inherited from LICM, is this a generalized safe assumption? Is there any very obvious exception to this?
Not sure how this list is generated in LICM. Is it trying to exclude PHINode and terminators (e.g. BranchInst, SwitchInst, ...)? If so, then those was excluded in `isSafeToMoveBefore` originally. 
Any test cases failed without this check? 


================
Comment at: llvm/lib/Transforms/Utils/CodeMoverUtils.cpp:323
+      return reportInvalidCandidate(I, NotMovedDebugInfo);
+    if (CI->mayThrow())
+      return reportInvalidCandidate(I, NotMovedThrowCall);
----------------
This is covered in line 380.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82290