[PATCH] D48024: [DebugInfo] Check size of variable in ConvertDebugDeclareToDebugValue

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added inline comments.


================
Comment at: include/llvm/IR/IntrinsicInst.h:98
+    /// that is described.
+    Optional<uint64_t> getFragmentSize() const;
+
----------------
`getFragmentSizeInBits()` ?

I'm still curious whether it makes sense to support the case where this returns None, or if we could add a Verifier check that is effectively `assert_di(getFragmentSize())`. Doesn't need to be in this commit though.


================
Comment at: lib/Transforms/Utils/Local.cpp:1233
+/// fragment of the variable) described by \p DII.
+static bool ValueCoversEntireFragment(Type *ValTy, DbgInfoIntrinsic *DII) {
+  const DataLayout &DL = DII->getModule()->getDataLayout();
----------------
Are all functions in this file starting with an upper case letter? Otherwise it would be better to follow the LLVM naming conventions. Perhaps also just doing this in a later NFC commit.


================
Comment at: lib/Transforms/Utils/Local.cpp:1310
+  assert(ValueCoversEntireFragment(LI->getType(), DII) &&
+         "Assuming that the load is loading the full variable.");
+
----------------
I think we usually spell out the error, not the assertion: `load is not loading the full variable fragment`


Repository:
  rL LLVM

https://reviews.llvm.org/D48024