[PATCH] D133293: [Assignment Tracking][10/*] salvageDebugInfo for dbg.assign intrinsics

[Nuno Lopes via Phabricator via llvm-commits]

nlopes added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/Local.cpp:1769
+  } else {
+    DAI->setAddress(UndefValue::get(I->getType()));
+  }
----------------
Orlando wrote:
> nlopes wrote:
> > Please use PoisonValue for placeholder/dummy values. We are trying to get rid of undef.
> > Thank you!
> Oh right, I wasn't aware of that - do you have a link to a discussion on this?
> 
> FWIW, this `Undef` is wrapped in `ValueAsMetadata` and is used by a debug intrinsic, if that makes a difference. This is currently standard practice for debug intrinsics to denote that the value of a variable is unknown.
> 
> I posted a question on discourse that might be relevant / you may be interested in - [[ https://discourse.llvm.org/t/auto-undef-debug-uses-of-a-deleted-value/65019 | here ]].
Traditionally all placeholders were undef, because there was no poison value.
But now that we have poison, we are trying to move away from undef. Since PoisonValue inherits from UndefValue, all matchers (e.g., isa<>) will work fine. So upgrades can be done incrementally.

I do have a rule in the review system to alert me when someone is trying to add a new use of UndefValue::get, as we want to avoid that.


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

https://reviews.llvm.org/D133293