[PATCH] D151419: [DebugInfo][RemoveDIs] Eliminate some debug-intrinsics-affect-codegen errors

[Felipe de Azevedo Piovezan via Phabricator via llvm-commits]

fdeazeve added inline comments.


================
Comment at: llvm/lib/IR/BasicBlock.cpp:423-425
   DebugLoc Loc = I->getDebugLoc();
+  if (isa<DbgInfoIntrinsic>(&*I))
+    Loc = I->getNextNonDebugInstruction()->getDebugLoc();
----------------
jryans wrote:
> It might be to structure this as an `if ... else ...` so we always test `isa<DbgInfoIntrinsic>(&*I)` first, instead of unconditionally calling `I->getDebugLoc()` and then potentially replacing it...? (If you agree, then the same comment applies to quite a few other changes here.)
Not to turn this into a bike-shedding discussion but this seems like an important enough pattern that some helper function could live alongside Intrusction::getNextNonDebugInstruction and friends? Something similar to BasicBlock::skipDebugIntrinsics.

I only mention this because an `if/else` pattern would necessarily imply declaring an uninitialized loc or having a big if/else block, both of which are undesirable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151419