[Autobackout][FunctionalRegression]Revert of change: d08d9ce: [DebugInfo] Only emit stack value at the end of SIMD32 exprs

Since our emitter splits SIMD32 programs into SIMD16 subprograms, we should
    adapt the debug info by only emitting `DW_OP_stack_value` at the end of the
    source variable's DI Block, i.e. after the merge point that follows the upper
    SIMD16 register.

    The approach is to note the information about the split in an
    `IGC::DbgVariable` instance when generating the upper register variable.
    Potentially, we could also consider checking for the presence of `DW_OP_skip`
    when evaluating the DI expression, however it would seem less future-proof in
    case of future use cases for skips.

    The change is accompanied by minor in-place refactoring where appropriate.

Author: sys-igc

IGC/DebugInfo/DwarfCompileUnit.cpp

@@ -2257,7 +2257,7 @@ IGC::DIE *CompileUnit::constructVariableDIE(DbgVariable &DV,
 
   // Check if variable is described by a DBG_VALUE instruction.
   const Instruction *pDbgInst = DV.getDbgInst();
-  if (!pDbgInst || !DV.currentLocationIsInlined()) {
+  if (!pDbgInst || !DV.isLocationInlined) {
     DV.setDIE(VariableDie);
     LLVM_DEBUG(dbgs() << " done. No dbg.inst assotiated\n");
     return VariableDie;
@@ -2603,7 +2603,8 @@ bool CompileUnit::buildFpBasedLoc(const DbgVariable &var, IGC::DIEBlock *Block,
 }
 
 bool CompileUnit::buildSlicedLoc(
-    DbgVariable &var, IGC::DIEBlock *Block, const VISAVariableLocation &loc,
+    const DbgVariable &var, IGC::DIEBlock *Block,
+    const VISAVariableLocation &loc,
     const std::vector<DbgDecoder::LiveIntervalsVISA> *vars) {
   LLVM_DEBUG(dbgs() << "  sliced variable, pushing lane \n");
   // DW_OP_push_simd_lane
@@ -2627,7 +2628,7 @@ bool CompileUnit::buildSlicedLoc(
   unsigned int offsetNotTaken = Block->ComputeSizeOnTheFly(Asm);
 
   // Emit first register
-  if (!buildValidVar(var, Block, loc, vars, DbgRegisterType::FirstHalf))
+  if (!buildValidVar(var, Block, loc, vars, true))
     return false;
 
   // Emit second half register
@@ -2644,16 +2645,16 @@ bool CompileUnit::buildSlicedLoc(
   // register in buildValidVar(), which always processes the 1st register only.
   VISAVariableLocation second_loc(loc);
   second_loc.SetRegister(loc.GetSecondReg());
-  if (!buildValidVar(var, Block, second_loc, vars, DbgRegisterType::SecondHalf))
+  if (!buildValidVar(var, Block, second_loc, vars, false))
     return false;
 
   return true;
 }
 
 bool CompileUnit::buildValidVar(
-    DbgVariable &var, IGC::DIEBlock *Block, const VISAVariableLocation &loc,
-    const std::vector<DbgDecoder::LiveIntervalsVISA> *vars,
-    DbgRegisterType regType) {
+    const DbgVariable &var, IGC::DIEBlock *Block,
+    const VISAVariableLocation &loc,
+    const std::vector<DbgDecoder::LiveIntervalsVISA> *vars, bool firstHalf) {
   const DbgDecoder::VarInfo *VarInfo = nullptr;
   const auto *VISAMod = loc.GetVISAModule();
 
@@ -2670,26 +2671,11 @@ bool CompileUnit::buildValidVar(
       LLVM_DEBUG(dbgs() << "  warning: could not get vISA Variable info\n");
   }
 
-  const bool isSecondHalf = regType == DbgRegisterType::SecondHalf;
-  const unsigned NumVarsExpected = isSecondHalf ? 2 : 1;
-  // TODO: If neither condition is fulfilled, should we do an early
-  // 'return false' as in "invalid variable"? In that case, we could improve
-  // the logic in the following way:
-  //
-  //  DbgDecoder::LiveIntervalsVISA *lrToUse = nullptr;
-  //  if (vars->size() >= NumVarsExpected)
-  //    lrToUse = vars->at(LRIndex);
-  //  else if (VarInfo)
-  //    lrToUse = VarInfo->lrs.front();
-  //  if (!lrToUse)
-  //    return false;
-  //  /* remaining code from the if block */
-  if (VarInfo || vars->size() >= NumVarsExpected) {
-    const unsigned LRIndex = isSecondHalf ? 1 : 0;
-    const auto &lrToUse = vars ? vars->at(LRIndex) : VarInfo->lrs.front();
+  if (VarInfo || (vars && vars->size() >= (firstHalf ? 1u : 2u))) {
+    const auto &lrToUse =
+        vars ? vars->at(firstHalf ? 0 : 1) : VarInfo->lrs.front();
     LLVM_DEBUG(dbgs() << "  emitting variable location at LR: <";
                lrToUse.print(dbgs()); dbgs() << ">\n");
-    var.setLocationRegisterType(regType);
     emitLocation = true;
     if (lrToUse.isGRF()) {
       if (loc.IsVectorized() == false) {
@@ -2716,7 +2702,7 @@ bool CompileUnit::buildValidVar(
                      SimdOffset < MaxUI16);
           if (loc.IsRegister())
             addSimdLane(Block, var, loc, &lrToUse, (uint16_t)(SimdOffset),
-                        false, isSecondHalf);
+                        false, !firstHalf);
         }
       }
     } else if (lrToUse.isSpill()) {
@@ -2752,7 +2738,7 @@ bool CompileUnit::buildValidVar(
                              static_cast<int32_t>(VectorOffset));
           addBE_FP(Block);
           addSimdLane(Block, var, loc, &lrToUse, 0, false,
-                      isSecondHalf); // Emit SIMD lane for spill (unpacked)
+                      !firstHalf); // Emit SIMD lane for spill (unpacked)
         }
       }
     } else {
@@ -2765,7 +2751,7 @@ bool CompileUnit::buildValidVar(
 }
 
 IGC::DIEBlock *CompileUnit::buildGeneral(
-    DbgVariable &var, const VISAVariableLocation &loc,
+    const DbgVariable &var, const VISAVariableLocation &loc,
     const std::vector<DbgDecoder::LiveIntervalsVISA> *vars,
     IGC::DIE *VariableDie) {
   IGC::DIEBlock *Block = new (DIEValueAllocator) IGC::DIEBlock();
@@ -2802,7 +2788,7 @@ IGC::DIEBlock *CompileUnit::buildGeneral(
     if (loc.HasLocationSecondReg()) {
       buildSlicedLoc(var, Block, loc, vars);
     } else {
-      buildValidVar(var, Block, loc, vars, DbgRegisterType::Regular);
+      buildValidVar(var, Block, loc, vars, true);
     }
   }
 

IGC/DebugInfo/DwarfCompileUnit.hpp

@@ -524,7 +524,7 @@ class CompileUnit {
   // buildSLM - Build expression for location described as offset in SLM memory.
   DIEBlock *buildSLM(const DbgVariable &, const VISAVariableLocation &,
                      IGC::DIE *);
-  DIEBlock *buildGeneral(DbgVariable &, const VISAVariableLocation &,
+  DIEBlock *buildGeneral(const DbgVariable &, const VISAVariableLocation &,
                          const std::vector<DbgDecoder::LiveIntervalsVISA> *,
                          IGC::DIE *);
 
@@ -533,13 +533,12 @@ class CompileUnit {
                                 const VISAVariableLocation &);
   bool buildFpBasedLoc(const DbgVariable &, IGC::DIEBlock *,
                        const VISAVariableLocation &);
-  bool buildSlicedLoc(DbgVariable &, IGC::DIEBlock *,
+  bool buildSlicedLoc(const DbgVariable &, IGC::DIEBlock *,
                       const VISAVariableLocation &,
                       const std::vector<DbgDecoder::LiveIntervalsVISA> *);
-  bool buildValidVar(DbgVariable &, IGC::DIEBlock *,
+  bool buildValidVar(const DbgVariable &, IGC::DIEBlock *,
                      const VISAVariableLocation &,
-                     const std::vector<DbgDecoder::LiveIntervalsVISA> *,
-                     DbgRegisterType);
+                     const std::vector<DbgDecoder::LiveIntervalsVISA> *, bool);
 
   // Variables, used in buildGeneral-algorithm:
   bool emitLocation = false;

IGC/DebugInfo/DwarfDebug.cpp

@@ -211,19 +211,14 @@ void DbgVariable::emitExpression(CompileUnit *CU, IGC::DIEBlock *Block) const {
     }
     I->appendToVector(Elements);
   }
-  const bool isSimpleIndirect = currentLocationIsSimpleIndirectValue();
-  if (isSimpleIndirect)
-    // drop OP_deref
+  bool isStackValueNeeded = false;
+  if (currentLocationIsSimpleIndirectValue()) {
+    // drop OP_deref and don't emit DW_OP_stack_value.
     Elements.erase(Elements.begin());
-  bool shouldResetStackValue = currentLocationIsImplicit();
-  if (shouldResetStackValue && !Elements.empty() &&
-      *Elements.rbegin() == dwarf::DW_OP_stack_value) {
-    Elements.pop_back();
+  } else if (!currentLocationIsMemoryAddress() &&
+             !currentLocationIsImplicit() && !currentLocationIsVector()) {
+    isStackValueNeeded = true;
   }
-  const bool isFirstHalf = this->RegType == DbgRegisterType::FirstHalf;
-  bool isStackValueNeeded = !isSimpleIndirect &&
-                            !currentLocationIsMemoryAddress() &&
-                            !currentLocationIsVector() && !isFirstHalf;
 
   for (auto elem : Elements) {
     auto BF = DIEInteger::BestForm(false, elem);
@@ -1653,7 +1648,7 @@ void DwarfDebug::collectVariableInfo(
                                   (pInst->getMetadata("StorageOffset") ||
                                    Loc.HasSurface() || Loc.IsSLM()))) {
           RegVar->setDbgInst(pInst);
-          RegVar->setLocationInlined(true);
+          RegVar->isLocationInlined = true;
           break;
         }
       }

IGC/DebugInfo/DwarfDebug.hpp

@@ -123,15 +123,6 @@ class DotDebugLocEntry {
   void setSymbol(llvm::MCSymbol *S) { Symbol = S; }
 };
 
-//===----------------------------------------------------------------------===//
-/// \brief This enum is used to describe whether a register represents one of
-/// the SIMD32 register halves.
-enum class DbgRegisterType : uint8_t {
-  Regular = 0,   // Represents all SIMD channels for a source variable, no slice
-  FirstHalf = 1, // SIMD32 sliced - lower channels
-  SecondHalf = 2 // SIMD32 sliced - upper channels
-};
-
 //===----------------------------------------------------------------------===//
 /// \brief This class is used to track local variable information.
 class DbgVariable {
@@ -152,12 +143,6 @@ class DbgVariable {
   // DBG_VALUE instruction of the variable
   const llvm::DbgVariableIntrinsic *m_pDbgInst = nullptr;
 
-  // isLocationInlined is true when we expect location to be inlined in
-  // DW_AT_location.
-  bool isLocationInlined = false;
-
-  DbgRegisterType RegType = DbgRegisterType::Regular;
-
 public:
   // AbsVar may be NULL.
   DbgVariable(const llvm::DILocalVariable *V,
@@ -211,16 +196,6 @@ class DbgVariable {
   bool currentLocationIsSimpleIndirectValue() const;
   bool currentLocationIsVector() const;
 
-  bool currentLocationIsInlined() const { return isLocationInlined; }
-  void setLocationInlined(bool isInlined = true) {
-    isLocationInlined = isInlined;
-  }
-
-  DbgRegisterType getLocationRegisterType() const { return RegType; }
-  void setLocationRegisterType(DbgRegisterType RegType) {
-    this->RegType = RegType;
-  }
-
   void emitExpression(CompileUnit *CU, IGC::DIEBlock *Block) const;
 
   // Translate tag to proper Dwarf tag.
@@ -248,6 +223,10 @@ class DbgVariable {
     return false;
   }
 
+  // isLocationInlined is true when we expect location to be inlined in
+  // DW_AT_location.
+  bool isLocationInlined = false;
+
   bool isBlockByrefVariable() const;
 
   llvm::DIType *getType() const;