[PATCH] D124691: [Transforms] Fix a wrong debug info intrinsic call in `mem2reg` pass for ref 128-bit

[Jeremy Morse via Phabricator via llvm-commits]

jmorse added a comment.

While the patch does fix the immediate issue, I'm not certain that disabling the `valueCoversEntireFragment` check in the context of pointers is a general solution. Otherwise generic code that does something strange (like breaking up allocas) would expect dbg.value's to become undef, and not doing that for certain types risks causing unexpected behaviour in the future. It's also good practice to be conservative in what input we accept here.

It seems that the "size" field of DIDerivedType nodes is optional -- I'm not sure why. Regardless, by adding "size: 64" to !10 in your test, an unpatched `opt` produces the correct output. Are you working from a source-language reproducer, and wouldn't the more important bug be whatever caused the DIDerivedType size field to either be unassigned, or dropped?

(It's also not clear to me why LLVM has chosen to not have a DIDerivedType specific getSizeInBits field, as it would be able to identify the type size from it's tag being a pointer or reference. However, it looks like the existing design expects the size to be provided in the "size" field, so we should try fixing that first).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124691