[DebugInfo] Add assert for variable size when creating fragments.

[akhuang]

Summary

Recently I added the size to the debug info for class declarations, so
that the correct fragments are created when a class is not defined.

This patch asserts that the variable size exists when creating
fragments, since I think there shouldn't be any cases where we want to
create fragments for a variable with no size.t

Bug: https://bugs.llvm.org/show_bug.cgi?id=47338

[dblaikie]

I'll leave this one to @aprantl - though I am a bit concerned about whether this should be a verifier error if SROA is going to assume it's an invariant. Some discussion on the bug about whether this SROA code is only reachable for some subset of types that do have sizes, while some other types won't have sizes. (variable length arrays being a fairly fundamental example, since they can't have a known length - but I would worry that SROA might be able to mess with known portions of them, maybe?) I mean, I'm usually all for "throw an assert in, and if it ever fails then we'll have figured out what case we didn't consider earlier", though just a bit hesitant on this at the moment. Probably no big deal, though.

[akhuang]

Ah, yeah. I'm not really sure if there could be cases where there is no size. If there are, I guess we shouldn't make any changes here.

[aprantl]

I'm expecting this to cause all sorts of non-clang frontends to trigger this assertion, since we don't require types to have sizes. In fact, I have an open bug for the Swift frontend to try removing the static sizes from types, since LLDB prefers the dynamic runtime size anyway.
If we want to make this a requirement, we should probably implement it as a verifier check instead of an assertion. But I'm not sure if we want to require this for all frontends.

[dblaikie]

In D87641#2275598, @aprantl wrote:

I'm expecting this to cause all sorts of non-clang frontends to trigger this assertion, since we don't require types to have sizes. In fact, I have an open bug for the Swift frontend to try removing the static sizes from types, since LLDB prefers the dynamic runtime size anyway.
If we want to make this a requirement, we should probably implement it as a verifier check instead of an assertion. But I'm not sure if we want to require this for all frontends.

An alternative question then: Should this constraint exist? Or should it be valid to create a fragment that is the same size as the object? (we currently assert in the verifier if the size is available and the fragment covers the whole size: AssertDI(FragSize != *VarSize, "fragment covers entire variable", Desc, &V);)

llvm/lib/IR/Verifier.cpp

 
 template <typename ValueOrMetadata>
 void Verifier::verifyFragmentExpression(const DIVariable &V,
                                         DIExpression::FragmentInfo Fragment,
                                         ValueOrMetadata *Desc) {
   // If there's no size, the type is broken, but that should be checked
   // elsewhere.
   auto VarSize = V.getSizeInBits();
-  if (!VarSize)
-    return;
+  AssertDI(VarSize, "variable with fragments must have a size", Desc, &V);
 
   unsigned FragSize = Fragment.SizeInBits;
   unsigned FragOffset = Fragment.OffsetInBits;
   AssertDI(FragSize + FragOffset <= *VarSize,
          "fragment is larger than or outside of variable", Desc, &V);
   AssertDI(FragSize != *VarSize, "fragment covers entire variable", Desc, &V);
 }
 

llvm/lib/Transforms/Scalar/SROA.cpp

for (auto Fragment : Fragments) {
         if (auto OrigFragment = FragmentExpr->getFragmentInfo()) {
           assert(Start >= OrigFragment->OffsetInBits &&
                  "new fragment is outside of original fragment");
           Start -= OrigFragment->OffsetInBits;
         }
 
         // The alloca may be larger than the variable.
         auto VarSize = DbgDeclare->getVariable()->getSizeInBits();
-        if (VarSize) {
-          if (Size > *VarSize)
-            Size = *VarSize;
-          if (Size == 0 || Start + Size > *VarSize)
-            continue;
-        }
+        assert(VarSize && "Variable with fragments must have a size");
 
         // Avoid creating a fragment expression that covers the entire variable.
-        if (!VarSize || *VarSize != Size) {
+        if (*VarSize != Size) {
           if (auto E =
                   DIExpression::createFragmentExpression(Expr, Start, Size))
             FragmentExpr = *E;
           else
             continue;
         }
       }