Add debug_value [trace] attribute.

atrick · atrick · commit 87d72371814f · 2022-09-25T22:44:15.000-07:00
This lets us write optimizer unit tests and selectively debug the
optimizer in general. We'll be able trace analyses and control
optimization selectively for certain values.

Adding a trace flag to debug_value is the easiest way to start using
it experimentally and develop the rest of the infrastructure. If this
takes off, then we can consider a new `trace_value`
instruction. For now, reusing debug_value is the least intrusive way to
start writing liveness unit tests.
diff --git a/include/swift/SIL/SILBuilder.h b/include/swift/SIL/SILBuilder.h
@@ -957,10 +957,12 @@ class SILBuilder {
   DebugValueInst *createDebugValue(SILLocation Loc, SILValue src,
                                    SILDebugVariable Var,
                                    bool poisonRefs = false,
-                                   bool wasMoved = false);
+                                   bool wasMoved = false,
+                                   bool trace = false);
   DebugValueInst *createDebugValueAddr(SILLocation Loc, SILValue src,
                                        SILDebugVariable Var,
-                                       bool wasMoved = false);
+                                       bool wasMoved = false,
+                                       bool trace = false);
 
   /// Create a debug_value according to the type of \p src
   SILInstruction *emitDebugDescription(SILLocation Loc, SILValue src,
diff --git a/include/swift/SIL/SILCloner.h b/include/swift/SIL/SILCloner.h
@@ -1283,7 +1283,7 @@ SILCloner<ImplClass>::visitDebugValueInst(DebugValueInst *Inst) {
   getBuilder().setCurrentDebugScope(getOpScope(Inst->getDebugScope()));
   auto *NewInst = getBuilder().createDebugValue(
       Inst->getLoc(), getOpValue(Inst->getOperand()), VarInfo,
-      Inst->poisonRefs(), Inst->getWasMoved());
+      Inst->poisonRefs(), Inst->getWasMoved(), Inst->hasTrace());
   remapDebugVarInfo(DebugVarCarryingInst(NewInst));
   recordClonedInstruction(Inst, NewInst);
 }
diff --git a/include/swift/SIL/SILInstruction.h b/include/swift/SIL/SILInstruction.h
@@ -4832,13 +4832,15 @@ class DebugValueInst final
   USE_SHARED_UINT8;
 
   DebugValueInst(SILDebugLocation DebugLoc, SILValue Operand,
-                 SILDebugVariable Var, bool poisonRefs, bool operandWasMoved);
+                 SILDebugVariable Var, bool poisonRefs, bool operandWasMoved,
+                 bool trace);
   static DebugValueInst *create(SILDebugLocation DebugLoc, SILValue Operand,
                                 SILModule &M, SILDebugVariable Var,
-                                bool poisonRefs, bool operandWasMoved);
+                                bool poisonRefs, bool operandWasMoved,
+                                bool trace);
   static DebugValueInst *createAddr(SILDebugLocation DebugLoc, SILValue Operand,
                                     SILModule &M, SILDebugVariable Var,
-                                    bool operandWasMoved);
+                                    bool operandWasMoved, bool trace);
 
   SIL_DEBUG_VAR_SUPPLEMENT_TRAILING_OBJS_IMPL()
 
@@ -4915,6 +4917,12 @@ class DebugValueInst final
   void setPoisonRefs(bool poisonRefs = true) {
     sharedUInt8().DebugValueInst.poisonRefs = poisonRefs;
   }
+
+  bool hasTrace() const { return sharedUInt8().DebugValueInst.trace; }
+
+  void setTrace(bool trace = true) {
+    sharedUInt8().DebugValueInst.trace = trace;
+  }
 };
 
 /// An abstract class representing a load from some kind of reference storage.
diff --git a/include/swift/SIL/SILNode.h b/include/swift/SIL/SILNode.h
@@ -204,7 +204,8 @@ class alignas(8) SILNode :
 
     SHARED_FIELD(DebugValueInst, uint8_t
       poisonRefs : 1,
-      operandWasMoved : 1);
+      operandWasMoved : 1,
+      trace : 1);
 
     SHARED_FIELD(AllocStackInst, uint8_t
       dynamicLifetime : 1,
diff --git a/include/swift/SIL/SILValue.h b/include/swift/SIL/SILValue.h
@@ -572,6 +572,10 @@ class ValueBase : public SILNode, public SILAllocated<ValueBase> {
     return true;
   }
 
+  /// Returns true if this value should be traced for optimization debugging
+  /// (it has a debug_value [trace] user).
+  bool hasDebugTrace() const;
+
   static bool classof(SILNodePointer node) {
     return node->getKind() >= SILNodeKind::First_ValueBase &&
            node->getKind() <= SILNodeKind::Last_ValueBase;
diff --git a/lib/SIL/IR/SILBuilder.cpp b/lib/SIL/IR/SILBuilder.cpp
@@ -589,24 +589,26 @@ void SILBuilder::emitDestructureValueOperation(
 DebugValueInst *SILBuilder::createDebugValue(SILLocation Loc, SILValue src,
                                              SILDebugVariable Var,
                                              bool poisonRefs,
-                                             bool operandWasMoved) {
+                                             bool operandWasMoved,
+                                             bool trace) {
   llvm::SmallString<4> Name;
   // Debug location overrides cannot apply to debug value instructions.
   DebugLocOverrideRAII LocOverride{*this, None};
   return insert(DebugValueInst::create(
       getSILDebugLocation(Loc), src, getModule(),
-      *substituteAnonymousArgs(Name, Var, Loc), poisonRefs, operandWasMoved));
+      *substituteAnonymousArgs(Name, Var, Loc), poisonRefs, operandWasMoved,
+      trace));
 }
 
 DebugValueInst *SILBuilder::createDebugValueAddr(SILLocation Loc, SILValue src,
                                                  SILDebugVariable Var,
-                                                 bool wasMoved) {
+                                                 bool wasMoved, bool trace) {
   llvm::SmallString<4> Name;
   // Debug location overrides cannot apply to debug addr instructions.
   DebugLocOverrideRAII LocOverride{*this, None};
   return insert(DebugValueInst::createAddr(
       getSILDebugLocation(Loc), src, getModule(),
-      *substituteAnonymousArgs(Name, Var, Loc), wasMoved));
+      *substituteAnonymousArgs(Name, Var, Loc), wasMoved, trace));
 }
 
 void SILBuilder::emitScopedBorrowOperation(SILLocation loc, SILValue original,
diff --git a/lib/SIL/IR/SILInstructions.cpp b/lib/SIL/IR/SILInstructions.cpp
@@ -351,7 +351,7 @@ SILType AllocBoxInst::getAddressType() const {
 
 DebugValueInst::DebugValueInst(SILDebugLocation DebugLoc, SILValue Operand,
                                SILDebugVariable Var, bool poisonRefs,
-                               bool wasMoved)
+                               bool wasMoved, bool trace)
     : UnaryInstructionBase(DebugLoc, Operand),
       SILDebugVariableSupplement(Var.DIExpr.getNumElements(),
                                  Var.Type.hasValue(), Var.Loc.hasValue(),
@@ -365,29 +365,30 @@ DebugValueInst::DebugValueInst(SILDebugLocation DebugLoc, SILValue Operand,
   setPoisonRefs(poisonRefs);
   if (wasMoved)
     markAsMoved();
+  setTrace(trace);
 }
 
 DebugValueInst *DebugValueInst::create(SILDebugLocation DebugLoc,
                                        SILValue Operand, SILModule &M,
                                        SILDebugVariable Var, bool poisonRefs,
-                                       bool wasMoved) {
+                                       bool wasMoved, bool trace) {
   void *buf = allocateDebugVarCarryingInst<DebugValueInst>(M, Var);
   return ::new (buf)
-      DebugValueInst(DebugLoc, Operand, Var, poisonRefs, wasMoved);
+    DebugValueInst(DebugLoc, Operand, Var, poisonRefs, wasMoved, trace);
 }
 
 DebugValueInst *DebugValueInst::createAddr(SILDebugLocation DebugLoc,
                                            SILValue Operand, SILModule &M,
                                            SILDebugVariable Var,
-                                           bool wasMoved) {
+                                           bool wasMoved, bool trace) {
   // For alloc_stack, debug_value is used to annotate the associated
   // memory location, so we shouldn't attach op_deref.
   if (!isa<AllocStackInst>(Operand))
     Var.DIExpr.prependElements(
       {SILDIExprElement::createOperator(SILDIExprOperator::Dereference)});
   void *buf = allocateDebugVarCarryingInst<DebugValueInst>(M, Var);
   return ::new (buf) DebugValueInst(DebugLoc, Operand, Var,
-                                    /*poisonRefs=*/false, wasMoved);
+                                    /*poisonRefs=*/false, wasMoved, trace);
 }
 
 bool DebugValueInst::exprStartsWithDeref() const {
diff --git a/lib/SIL/IR/SILPrinter.cpp b/lib/SIL/IR/SILPrinter.cpp
@@ -1737,6 +1737,8 @@ class SILPrinter : public SILInstructionVisitor<SILPrinter> {
       *this << "[poison] ";
     if (DVI->getWasMoved())
       *this << "[moved] ";
+    if (DVI->hasTrace())
+      *this << "[trace] ";
     *this << getIDAndType(DVI->getOperand());
     printDebugVar(DVI->getVarInfo(),
                   &DVI->getModule().getASTContext().SourceMgr);
diff --git a/lib/SIL/IR/SILValue.cpp b/lib/SIL/IR/SILValue.cpp
@@ -118,6 +118,16 @@ bool ValueBase::isLexical() const {
   return false;
 }
 
+bool ValueBase::hasDebugTrace() const {
+  for (auto *op : getUses()) {
+    if (auto *debugValue = dyn_cast<DebugValueInst>(op->getUser())) {
+      if (debugValue->hasTrace())
+        return true;
+    }
+  }
+  return false;
+}
+
 SILBasicBlock *SILNode::getParentBlock() const {
   if (auto *Inst = dyn_cast<SILInstruction>(this))
     return Inst->getParent();
diff --git a/lib/SIL/Parser/ParseSIL.cpp b/lib/SIL/Parser/ParseSIL.cpp
@@ -3347,6 +3347,7 @@ bool SILParser::parseSpecificSILInstruction(SILBuilder &B,
   case SILInstructionKind::DebugValueInst: {
     bool poisonRefs = false;
     bool wasMoved = false;
+    bool hasTrace = false;
     SILDebugVariable VarInfo;
 
     // Allow for poison and moved to be in either order.
@@ -3357,6 +3358,8 @@ bool SILParser::parseSpecificSILInstruction(SILBuilder &B,
         poisonRefs = true;
       else if (attributeName == "moved")
         wasMoved = true;
+      else if (attributeName == "trace")
+        hasTrace = true;
       else {
         P.diagnose(attributeLoc, diag::sil_invalid_attribute_for_instruction,
                    attributeName, "debug_value");
@@ -3369,7 +3372,8 @@ bool SILParser::parseSpecificSILInstruction(SILBuilder &B,
       return true;
     if (Val->getType().isAddress())
       assert(!poisonRefs && "debug_value w/ address value does not support poison");
-    ResultVal = B.createDebugValue(InstLoc, Val, VarInfo, poisonRefs, wasMoved);
+    ResultVal = B.createDebugValue(InstLoc, Val, VarInfo, poisonRefs, wasMoved,
+                                   hasTrace);
     break;
   }
 
diff --git a/test/SIL/Parser/basic.sil b/test/SIL/Parser/basic.sil
@@ -1309,6 +1309,7 @@ sil @debug_value : $@convention(thin) (Int, @in P, AnyObject) -> () {
 bb0(%0 : $Int, %1 : $*P, %2 : $AnyObject):
   debug_value %0 : $Int         // CHECK: debug_value %0 : $Int
   debug_value [poison] %2 : $AnyObject // CHECK: debug_value [poison] %2 : $AnyObject
+  debug_value [trace] %2 : $AnyObject // CHECK: debug_value [trace] %2 : $AnyObject
   debug_value %1 : $*P, expr op_deref // CHECK: debug_value %1 : $*any P, expr op_deref
   unreachable
 }
diff --git a/test/SIL/cloning.sil b/test/SIL/cloning.sil
@@ -94,3 +94,16 @@ entry(%0 : @owned $Builtin.NativeObject):
   %res = apply %callee_move_value(%0) : $@convention(thin) (@owned Builtin.NativeObject) -> @owned Builtin.NativeObject
   return %res : $Builtin.NativeObject
 }
+
+sil [ossa] @callee_debug_value : $@convention(thin) (@owned X) -> @owned X {
+entry(%0 : @owned $X):
+  debug_value [trace] %0 : $X
+  return %0 : $X
+}
+
+sil [ossa] @caller_debug_value : $@convention(thin) (@owned X) -> @owned X {
+entry(%0 : @owned $X):
+  %callee_debug_value = function_ref @callee_debug_value : $@convention(thin) (@owned X) -> @owned X
+  %res = apply %callee_debug_value(%0) : $@convention(thin) (@owned X) -> @owned X
+  return %res : $X
+}

Original file line number	Diff line number	Diff line change
`@@ -1283,7 +1283,7 @@ SILCloner<ImplClass>::visitDebugValueInst(DebugValueInst *Inst) {`
`1283`	`1283`	`getBuilder().setCurrentDebugScope(getOpScope(Inst->getDebugScope()));`
`1284`	`1284`	`auto *NewInst = getBuilder().createDebugValue(`
`1285`	`1285`	`Inst->getLoc(), getOpValue(Inst->getOperand()), VarInfo,`
`1286`		`- Inst->poisonRefs(), Inst->getWasMoved());`
	`1286`	`+ Inst->poisonRefs(), Inst->getWasMoved(), Inst->hasTrace());`
`1287`	`1287`	`remapDebugVarInfo(DebugVarCarryingInst(NewInst));`
`1288`	`1288`	`recordClonedInstruction(Inst, NewInst);`
`1289`	`1289`	`}`