[HeterogeneousDwarf] Improve -O1 debugging support

Squashed commit, fixes SWDEV-463925.

This is a combination of 5 commits.
This is the 1st commit message:

[AMDGPU] Remove Dwarf encodings for subregisters

Previously, registers and subregisters mapped to the same Dwarf
encoding. We don't really have any way to refer to subregisters directly
from Dwarf, the expression emitter should instead use DW_OPs to stencil
out the subregister from the whole register. This was also confusing
tools that need to map back to the llvm reg (e.g. dwarfdump), since
getLLVMRegNum() would arbitrarily return the _LO16 register.

This is a cherry-pick of github.com/llvm/pull/117891 with
test fixes.

Change-Id: I155bce592c7d556c01a7e3048bb8b251109dd51d

This is the commit message #2:

[HeterogeneousDwarf] Support poisoned fragments in DIExpression

This improves fragment emission, since we previously had to assume that
a poisoned expression clobbered all other live variable fragments. Now,
it only clobbers the region described by it's fragment.

This commit also canonicalizes any DIExpression containing a poison to
only the ops DW_OP_LLVM_poisoned and (optionally) DW_OP_LLVM_fragment.
The other ops don't really serve any purpose since we can't rely on any
invariants (e.g. number of location ops) of a poisoned expression. This
also cleans up the dwarf output for posioned exprs.

Change-Id: I97bd9513a81b30f290ef70f958dcc8ca4c79e489

This is the commit message #3:

[HeterogeneousDwarf] Support Dwarf register emission of subregs/sequences

This commit adds support for emitting subregisters and register
sequences. This is needed for debugging -O1, since DIExpressions can now
refer to these registers.

Change-Id: Ic7b468a01855d3f8dc675dce4b2280625bf68574

This is the commit message #4:

[SelectionDAG] Add debug info salvaging for bitcast operations

This is needed to retain debug info for 64 bit kernel parameters.

This is the commit message #5:

[SROA] Fix DIOp-DIExpression fragment handling

Change-Id: If1f73e941c227d74ebe563c2857f5df2d60e9225

Change-Id: Id3914345ef7a434355f36e45d3ea0bf6c7ae29aa

Author: epilk

llvm/include/llvm/IR/DebugInfoMetadata.h

@@ -3228,6 +3228,29 @@ class DIExpression : public MDNode {
   static constexpr std::array<uint64_t, 1> PoisonedExpr = {
       dwarf::DW_OP_LLVM_poisoned};
 
+  DIExpression *getPoisonedFragment(unsigned OffsetInBits,
+                                    unsigned SizeInBits) const {
+    std::array<uint64_t, 4> PoisonedOps = {dwarf::DW_OP_LLVM_poisoned,
+                                           dwarf::DW_OP_LLVM_fragment,
+                                           OffsetInBits, SizeInBits};
+    return DIExpression::get(getContext(), PoisonedOps);
+  }
+
+  OldElementsRef getPoisonedElements() const {
+    std::optional<FragmentInfo> Frag = getFragmentInfo();
+    if (!Frag)
+      return PoisonedExpr;
+    return getPoisonedFragment(Frag->OffsetInBits, Frag->SizeInBits)
+        ->getElements();
+  }
+
+  DIExpression *getPoisoned() const {
+    std::optional<FragmentInfo> Frag = getFragmentInfo();
+    if (!Frag)
+      return DIExpression::get(getContext(), PoisonedExpr);
+    return getPoisonedFragment(Frag->OffsetInBits, Frag->SizeInBits);
+  }
+
   DIExpression(LLVMContext &C, StorageType Storage, ArrayRef<uint64_t> Elements)
       : MDNode(C, DIExpressionKind, Storage, std::nullopt),
         Elements(std::in_place_type<OldElements>, Elements.begin(),
@@ -3264,17 +3287,12 @@ class DIExpression : public MDNode {
                     (bool /*ignored*/, ArrayRef<DIOp::Variant> Elements),
                     (false, Elements))
 
-  static DIExpression *getPoisoned(LLVMContext &Ctx) {
-    return get(Ctx, PoisonedExpr);
-  }
-  DIExpression *getPoisoned() const { return getPoisoned(getContext()); }
-
   TempDIExpression clone() const { return cloneImpl(); }
 
   OldElementsRef getElements() const {
     if (auto *E = std::get_if<OldElements>(&Elements))
       return *E;
-    return PoisonedExpr;
+    return getPoisonedElements();
   }
 
   unsigned getNumElements() const { return getElements().size(); }

llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp

@@ -192,19 +192,25 @@ void DwarfExpression::addStackValue() {
 }
 
 void DwarfExpression::addSignedConstant(int64_t Value) {
+  if (IsPoisonedExpr)
+    return;
   assert(isImplicitLocation() || isUnknownLocation());
   LocationKind = Implicit;
   emitOp(dwarf::DW_OP_consts);
   emitSigned(Value);
 }
 
 void DwarfExpression::addUnsignedConstant(uint64_t Value) {
+  if (IsPoisonedExpr)
+    return;
   assert(isImplicitLocation() || isUnknownLocation());
   LocationKind = Implicit;
   emitConstu(Value);
 }
 
 void DwarfExpression::addUnsignedConstant(const APInt &Value) {
+  if (IsPoisonedExpr)
+    return;
   assert(isImplicitLocation() || isUnknownLocation());
   LocationKind = Implicit;
 
@@ -225,6 +231,8 @@ void DwarfExpression::addUnsignedConstant(const APInt &Value) {
 }
 
 void DwarfExpression::addConstantFP(const APFloat &APF, const AsmPrinter &AP) {
+  if (IsPoisonedExpr)
+    return;
   assert(isImplicitLocation() || isUnknownLocation());
   APInt API = APF.bitcastToAPInt();
   int NumBytes = API.getBitWidth() / 8;
@@ -255,6 +263,8 @@ bool DwarfExpression::addMachineRegExpression(const TargetRegisterInfo &TRI,
                                               DIExpressionCursor &ExprCursor,
                                               llvm::Register MachineReg,
                                               unsigned FragmentOffsetInBits) {
+  if (IsPoisonedExpr)
+    return true;
   auto Fragment = ExprCursor.getFragmentInfo();
   if (!addMachineReg(TRI, MachineReg, Fragment ? Fragment->SizeInBits : ~1U)) {
     LocationKind = Unknown;
@@ -510,6 +520,8 @@ bool DwarfExpression::addExpression(
   // and not any other parts of the following DWARF expression.
   assert(!IsEmittingEntryValue && "Can't emit entry value around expression");
 
+  IsPoisonedExpr = false;
+
   std::optional<DIExpression::ExprOperand> PrevConvertOp;
 
   while (ExprCursor) {
@@ -529,7 +541,7 @@ bool DwarfExpression::addExpression(
       emitOp(dwarf::DW_OP_LLVM_user);
       emitOp(dwarf::DW_OP_LLVM_USER_undefined);
       LocationKind = Unknown;
-      return true;
+      break;
     case dwarf::DW_OP_LLVM_arg:
       if (!InsertArg(Op->getArg(0), ExprCursor)) {
         LocationKind = Unknown;
@@ -715,10 +727,22 @@ bool DwarfExpression::addExpression(
 void DwarfExpression::addExpression(DIExpression::NewElementsRef Expr,
                                     ArrayRef<DbgValueLocEntry> ArgLocEntries,
                                     const TargetRegisterInfo *TRI) {
+  assert(!IsPoisonedExpr && "poisoned exprs should have old elements");
   this->ArgLocEntries = ArgLocEntries;
   this->TRI = TRI;
+  std::optional<DIOp::Fragment> FragOp;
+  for (DIOp::Variant Op : Expr) {
+    if (auto *Frag = std::get_if<DIOp::Fragment>(&Op)) {
+      FragOp = *Frag;
+      IsFragment = true;
+      break;
+    }
+  }
   buildAST(Expr);
   traverse(ASTRoot.get(), ValueKind::LocationDesc);
+  if (FragOp)
+    addOpPiece(FragOp->getBitSize());
+  IsFragment = false;
   ASTRoot.reset();
   this->TRI = nullptr;
   this->ArgLocEntries = std::nullopt;
@@ -756,7 +780,13 @@ void DwarfExpression::finalize() {
 }
 
 void DwarfExpression::addFragmentOffset(const DIExpression *Expr) {
-  if (!Expr || !Expr->isFragment())
+  if (!Expr)
+    return;
+
+  if (Expr->holdsOldElements() && Expr->isPoisoned())
+    IsPoisonedExpr = true;
+
+  if (!Expr->isFragment())
     return;
 
   uint64_t FragmentOffset = Expr->getFragmentInfo()->OffsetInBits;
@@ -806,6 +836,8 @@ void DwarfExpression::emitLegacyZExt(unsigned FromBits) {
 }
 
 void DwarfExpression::addWasmLocation(unsigned Index, uint64_t Offset) {
+  if (IsPoisonedExpr)
+    return;
   emitOp(dwarf::DW_OP_WASM_location);
   emitUnsigned(Index == 4/*TI_LOCAL_INDIRECT*/ ? 0/*TI_LOCAL*/ : Index);
   emitUnsigned(Offset);
@@ -912,16 +944,72 @@ std::optional<NewOpResult> DwarfExpression::traverse(DIOp::Arg Arg,
   }
 
   if (Entry.isLocation()) {
-    auto DWARFRegister = TRI->getDwarfRegNum(Entry.getLoc().getReg(), false);
-    if (DWARFRegister < 0) {
+    assert(DwarfRegs.empty() && "unconsumed registers?");
+    if (!addMachineReg(*TRI, Entry.getLoc().getReg())) {
+      DwarfRegs.clear();
       return std::nullopt;
     }
-    addReg(DWARFRegister);
+
+    // addMachineReg sets DwarfRegs and SubRegister{Size,Offset}InBits. Collect
+    // them here and reset the fields to avoid hitting any asserts.
+    decltype(DwarfRegs) Regs;
+    std::swap(Regs, DwarfRegs);
+    unsigned SubRegOffset = SubRegisterOffsetInBits;
+    unsigned SubRegSize = SubRegisterSizeInBits;
+    SubRegisterOffsetInBits = SubRegisterSizeInBits = 0;
+    if (SubRegOffset % 8 || SubRegSize % 8)
+      return std::nullopt;
+    SubRegOffset /= 8;
+    SubRegSize /= 8;
+
+    if (Regs.size() == 1) {
+      addReg(Regs[0].DwarfRegNo, Regs[0].Comment);
+
+      if (SubRegOffset) {
+        emitUserOp(dwarf::DW_OP_LLVM_USER_offset_uconst);
+        emitUnsigned(SubRegOffset);
+      }
+
+      if (SubRegSize) {
+        emitOp(dwarf::DW_OP_deref_size);
+        emitData1(SubRegSize);
+        return NewOpResult{Arg.getResultType(), ValueKind::Value};
+      }
+
+      return NewOpResult{Arg.getResultType(), ValueKind::LocationDesc};
+    }
+
+    assert(SubRegOffset == 0 && SubRegSize == 0 &&
+           "register piece cannot apply to multiple registers");
+
+    // When emitting fragments, the top element on the stack might be an
+    // incomplete composite. Push/drop a lit0 so that we don't add the registers
+    // to the larger composite.
+    if (IsFragment)
+      emitOp(dwarf::DW_OP_lit0);
+
+    unsigned RegSize = 0;
+    for (auto &Reg : Regs) {
+      if (Reg.SubRegSize % 8)
+        return std::nullopt;
+      RegSize += Reg.SubRegSize;
+      if (Reg.DwarfRegNo >= 0)
+        addReg(Reg.DwarfRegNo, Reg.Comment);
+      emitOp(dwarf::DW_OP_piece);
+      emitUnsigned(Reg.SubRegSize / 8);
+    }
+    emitUserOp(dwarf::DW_OP_LLVM_USER_piece_end);
+
+    if (IsFragment) {
+      emitOp(dwarf::DW_OP_swap);
+      emitOp(dwarf::DW_OP_drop);
+    }
+
     return NewOpResult{Arg.getResultType(), ValueKind::LocationDesc};
   }
 
   if (Entry.isInt()) {
-    addUnsignedConstant(Entry.getInt());
+    emitConstu(Entry.getInt());
   } else if (Entry.isConstantFP()) {
     // DwarfExpression does not support arguments wider than 64 bits
     // (see PR52584).
@@ -930,12 +1018,12 @@ std::optional<NewOpResult> DwarfExpression::traverse(DIOp::Arg Arg,
     APInt RawBytes = Entry.getConstantFP()->getValueAPF().bitcastToAPInt();
     if (RawBytes.getBitWidth() > 64)
       return std::nullopt;
-    addUnsignedConstant(RawBytes.getZExtValue());
+    emitConstu(RawBytes.getZExtValue());
   } else if (Entry.isConstantInt()) {
     APInt RawBytes = Entry.getConstantInt()->getValue();
     if (RawBytes.getBitWidth() > 64)
       return std::nullopt;
-    addUnsignedConstant(RawBytes.getZExtValue());
+    emitConstu(RawBytes.getZExtValue());
   } else if (Entry.isTargetIndexLocation()) {
     return std::nullopt;
   } else {
@@ -955,9 +1043,10 @@ std::optional<NewOpResult> DwarfExpression::traverse(DIOp::Constant Constant,
     return std::nullopt;
 
   if (isUnsigned(IntLiteralValue)) {
-    addUnsignedConstant(IntLiteralValue->getZExtValue());
+    emitConstu(IntLiteralValue->getZExtValue());
   } else {
-    addSignedConstant(IntLiteralValue->getSExtValue());
+    emitOp(dwarf::DW_OP_consts);
+    emitSigned(IntLiteralValue->getSExtValue());
   }
 
   return NewOpResult{IntLiteralValue->getType(), ValueKind::Value};
@@ -1056,7 +1145,7 @@ std::optional<NewOpResult> DwarfExpression::traverse(DIOp::Deref Deref,
 
   emitOp(dwarf::DW_OP_deref_size);
   emitData1(PointerSizeInBytes);
-  addUnsignedConstant(*PointerDWARFAddrSpace);
+  emitConstu(*PointerDWARFAddrSpace);
   emitUserOp(dwarf::DW_OP_LLVM_USER_form_aspace_address);
 
   // FIXME(KZHURAVL): Is the following result type correct?
@@ -1153,7 +1242,7 @@ void DwarfExpression::readToValue(Type *Ty) {
 
   if (NeedsMask) {
     uint64_t Mask = (1ULL << PrimitiveSizeInBits) - 1ULL;
-    addUnsignedConstant(Mask);
+    emitConstu(Mask);
     emitOp(dwarf::DW_OP_and);
   }
 }

llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h

@@ -361,6 +361,13 @@ class DwarfExpression {
   // the expression lowers as expected. If the lowering is not supported, it
   // is terminated by a DW_OP_LLVM_undefined operation.
   bool IsImplemented = true;
+  bool IsFragment = false;
+
+  /// Set when emitting a fragment/non-fragment expression that contains a
+  /// DW_OP_LLVM_poison operation. This matters for correctness in the fragment
+  /// case, since we need to ensure that we don't add any registers or constants
+  /// onto the stack. In the non-fragment case it's simply an optimization.
+  bool IsPoisonedExpr = false;
 
   void buildAST(DIExpression::NewElementsRef Elements);
 

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

@@ -15402,8 +15402,10 @@ SDValue DAGCombiner::visitBITCAST(SDNode *N) {
   }
 
   // (conv (conv x, t1), t2) -> (conv x, t2)
-  if (N0.getOpcode() == ISD::BITCAST)
+  if (N0.getOpcode() == ISD::BITCAST) {
+    DAG.salvageDebugInfo(*N0.getNode());
     return DAG.getBitcast(VT, N0.getOperand(0));
+  }
 
   // fold (conv (logicop (conv x), (c))) -> (logicop x, (conv c))
   // iff the current bitwise logicop type isn't legal

llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp

@@ -11273,6 +11273,35 @@ void SelectionDAG::salvageDebugInfo(SDNode &N) {
                  dbgs() << " into " << *DbgExpression << '\n');
       break;
     }
+    case ISD::BITCAST: {
+      DIExpression *Expr = DV->getExpression();
+      if (Expr->holdsOldElements())
+        break;
+
+      SDValue N0 = N.getOperand(0);
+      auto NewLocOps = DV->copyLocationOps();
+      bool Changed = false;
+      for (size_t i = 0; i < NewLocOps.size(); ++i) {
+        if (NewLocOps[i].getKind() != SDDbgOperand::SDNODE ||
+            NewLocOps[i].getSDNode() != &N)
+          continue;
+        NewLocOps[i] = SDDbgOperand::fromNode(N0.getNode(), N0.getResNo());
+        Changed = true;
+      }
+      assert(Changed && "Salvage target doesn't use N");
+      (void)Changed;
+
+      SDDbgValue *Clone =
+          getDbgValueList(DV->getVariable(), Expr, NewLocOps,
+                          DV->getAdditionalDependencies(), DV->isIndirect(),
+                          DV->getDebugLoc(), DV->getOrder(), DV->isVariadic());
+      ClonedDVs.push_back(Clone);
+      DV->setIsInvalidated();
+      DV->setIsEmitted();
+      LLVM_DEBUG(dbgs() << "SALVAGE: Rewriting"; N0.getNode()->dumprFull(this);
+                 dbgs() << " into " << *Expr << '\n');
+      break;
+    }
     }
   }
 

llvm/lib/IR/DebugInfoMetadata.cpp

@@ -1386,6 +1386,28 @@ DIExpression *DIExpression::getImpl(LLVMContext &Context,
 DIExpression *DIExpression::getImpl(LLVMContext &Context,
                                     OldElementsRef Elements,
                                     StorageType Storage, bool ShouldCreate) {
+  // If Elements is any expression containing DW_OP_LLVM_poisoned and an
+  // optional fragment then canonicalize, the other ops aren't doing anything.
+  SmallVector<uint64_t, 4> CanonicalizedPoisonOps;
+  for (unsigned Idx = 0; Idx < Elements.size();) {
+    ExprOperand Op(&Elements[Idx]);
+
+    if (CanonicalizedPoisonOps.empty()) {
+      if (Op.getOp() == dwarf::DW_OP_LLVM_poisoned)
+        CanonicalizedPoisonOps.push_back(Op.getOp());
+    } else if (Op.getOp() == dwarf::DW_OP_LLVM_fragment &&
+               Idx + 2 < Elements.size()) {
+      CanonicalizedPoisonOps.push_back(Op.getOp());
+      CanonicalizedPoisonOps.push_back(Op.getArg(0));
+      CanonicalizedPoisonOps.push_back(Op.getArg(1));
+    }
+
+    // Have to handle invalid exprs.
+    Idx += Op.getSize();
+  }
+  if (!CanonicalizedPoisonOps.empty())
+    Elements = CanonicalizedPoisonOps;
+
   DEFINE_GETIMPL_LOOKUP(DIExpression, (Elements));
   DEFINE_GETIMPL_STORE_NO_OPS(DIExpression, (Elements));
 }
@@ -2368,13 +2390,6 @@ std::optional<DIExpression *> DIExpression::createFragmentExpression(
   if (Expr->holdsNewElements())
     return createNewFragmentExpression(Expr, OffsetInBits, SizeInBits);
 
-  // FIXME(diexpression-poison): Is it safe to handle each fragment
-  // independently? If a fragment gets poisoned we lose the fragment info, so
-  // can't locate it correctly. Conservatively we can just have it cover the
-  // whole variable.
-  if (Expr->isPoisoned())
-    return Expr->getPoisoned();
-
   SmallVector<uint64_t, 8> Ops;
   // Track whether it's safe to split the value at the top of the DWARF stack,
   // assuming that it'll be used as an implicit location value.