[PATCH] D53740: [globalisel][irtranslator] Verify that DILocations aren't lost in translation

[Daniel Sanders via Phabricator via llvm-commits]

dsanders added inline comments.


================
Comment at: lib/CodeGen/GlobalISel/IRTranslator.cpp:124
+           "Inserted instruction without a current MI");
+    LLVM_DEBUG(dbgs() << "Checking DILocation from " << *CurrInst
+                      << " was copied to " << MI);
----------------
aemerson wrote:
> dsanders wrote:
> > aemerson wrote:
> > > I think this debug print is probably unnecessary given it'll print for every instruction translated (and without ASSERTIONS it won't actually do anything).
> > I'm not sure I understand this comment. It sounds like you're suggesting its removal but the follow on implies the right thing to do is to wrap it in '#ifndef NDEBUG'.
> Yes I was suggesting removing it.
> 
> AFAIK NDEBUG and assertions are independent of each other, so without assertions enabled this will print a message that doesn't actually check anything. Could be wrong about that.
I'd prefer not to remove it as it was quite useful to see what DILocations were expected to go where when I was debugging the missing DILocations that are fixed in this patch. I do agree it should only print "Checking ..." when the checks are actually happening though.

> AFAIK NDEBUG and assertions are independent of each other, so without assertions enabled this will
> print a message that doesn't actually check anything. Could be wrong about that.

The effects of LLVM_ENABLE_ASSERTIONS are achieved by adding -UNDEBUG and removing any -DNDEBUG from the standard build flags. The code for that is in HandleLLVMOptions.cmake




Repository:
  rL LLVM

https://reviews.llvm.org/D53740