I’ve had some time to think about and look into this more closely. I considered disallowing (verifying against) empty metadata operands and moving the auto-undef behaviour into ~Value (as mentioned in previous comments). This has the downside of working against the grain of LLVM’s infrastructure, which tends to null-out Values in a variety of circumstances which we’d need to fix on a case by case basis.
I liked the idea of using empty metadata !{} as the One True Kill Sentinel. However, there are a number of constraints I had not considered in my initial post.
undefs may occur naturally
Undef (and/or poison) is not only an intentional “kill sentinel” in debug intrinsics that marks a terminating location, but may also occur naturally when a value truly is undef (and/or poison). In my opinion we should continue to treat “naturally” undef’d debug intrinsics as kill sentinels. With that in mind, we can not eliminate undef as a “kill sentinel”.
DIArgLists require their own representation
First, replacing a DIArgList use with an empty metadata node looks like it can only be achieved in a hacky, ugly way. So right off the bat we would need a separate kill sentinel to non-variadic debug intrinsics if they use !{}.
Second, DIArgLists are set up to wrap Values rather than metadata, which means we need to do extreme hoop jumping (or re-writing) if we want to be able to represent !{} within the lists.
With these facts in mind, I believe the best course of action is:
- Treat
!{}as kill sentinels (so do not delete the intrinsics with!{}operands!) - Continue treating
undefas a kill sentinel. - Keep DIArgLits::handleChangedOperand’s conversion of nullptr → undef
- Optionally: delete ~Instruction’s auto-undefing behaviour now that
!{}is a kill sentinel.
While I’m in the area, we can also replace lots sites introducing undef to use poison instead (cc @nlopes).
I’ve written up some patches that achieve this, which I’ll post shortly.