[llvm] Introduce DIExpressionOptimizer in DIExpression::append (PR #69770)
via llvm-commits
llvm-commits at lists.llvm.org
Fri Oct 20 13:29:18 PDT 2023
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-debuginfo
Author: Shubham Sandeep Rastogi (rastogishubham)
<details>
<summary>Changes</summary>
This commit is used to use the new DIExpressionOptimizer class to optimize DIExpression::append calls.
---
Full diff: https://github.com/llvm/llvm-project/pull/69770.diff
5 Files Affected:
- (modified) llvm/include/llvm/IR/DebugInfoMetadata.h (+103)
- (modified) llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h (-61)
- (modified) llvm/lib/IR/DebugInfoMetadata.cpp (+140-3)
- (modified) llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-expr-chain.mir (+2-2)
- (modified) llvm/unittests/IR/MetadataTest.cpp (+75)
``````````diff
diff --git a/llvm/include/llvm/IR/DebugInfoMetadata.h b/llvm/include/llvm/IR/DebugInfoMetadata.h
index b347664883fd9f2..b9df644e44ccf36 100644
--- a/llvm/include/llvm/IR/DebugInfoMetadata.h
+++ b/llvm/include/llvm/IR/DebugInfoMetadata.h
@@ -3073,6 +3073,109 @@ template <> struct DenseMapInfo<DIExpression::FragmentInfo> {
static bool isEqual(const FragInfo &A, const FragInfo &B) { return A == B; }
};
+class DIExpressionOptimizer {
+
+ // When looking at a DIExpression such as {DW_OP_constu, 1, DW_OP_constu, 2,
+ // DW_OP_plus} and trying to append {DW_OP_consts, 3, DW_OP_minus}
+ // NewMathOperator = DW_OP_minus
+ // OperandRight = 3
+ // OperatorRight = DW_OP_consts
+ // CurrMathOperator = DW_OP_plus
+ // OperandLeft = 2
+ // OperandLeft = DW_OP_constu
+
+ /// The math operator of the new subexpression being appended to the
+ /// DIExpression.
+ uint64_t NewMathOperator = 0;
+
+ /// The operand of the new subexpression being appended to the DIExpression.
+ uint64_t OperandRight;
+
+ /// The operator attached to OperandRight.
+ uint64_t OperatorRight = 0;
+
+ /// The math operator at the end of the current DIExpression.
+ uint64_t CurrMathOperator = 0;
+
+ /// The operand at the end of the current DIExpression at the top of the
+ /// stack.
+ uint64_t OperandLeft;
+
+ /// The operator attached to OperandLeft.
+ uint64_t OperatorLeft = 0;
+
+ bool operatorIsCommutative(uint64_t Operator);
+
+ SmallVector<uint64_t> getOperatorLocations(ArrayRef<uint64_t> Ops);
+
+ void reset();
+
+public:
+ bool operatorCanBeOptimized(uint64_t Operator);
+ void optimize(SmallVectorImpl<uint64_t> &NewOps, ArrayRef<uint64_t> Ops);
+};
+
+/// Holds a DIExpression and keeps track of how many operands have been consumed
+/// so far.
+class DIExpressionCursor {
+ DIExpression::expr_op_iterator Start, End;
+
+public:
+ DIExpressionCursor(const DIExpression *Expr) {
+ if (!Expr) {
+ assert(Start == End);
+ return;
+ }
+ Start = Expr->expr_op_begin();
+ End = Expr->expr_op_end();
+ }
+
+ DIExpressionCursor(ArrayRef<uint64_t> Expr)
+ : Start(Expr.begin()), End(Expr.end()) {}
+
+ DIExpressionCursor(const DIExpressionCursor &) = default;
+
+ /// Consume one operation.
+ std::optional<DIExpression::ExprOperand> take() {
+ if (Start == End)
+ return std::nullopt;
+ return *(Start++);
+ }
+
+ /// Consume N operations.
+ void consume(unsigned N) { std::advance(Start, N); }
+
+ /// Return the current operation.
+ std::optional<DIExpression::ExprOperand> peek() const {
+ if (Start == End)
+ return std::nullopt;
+ return *(Start);
+ }
+
+ /// Return the next operation.
+ std::optional<DIExpression::ExprOperand> peekNext() const {
+ if (Start == End)
+ return std::nullopt;
+
+ auto Next = Start.getNext();
+ if (Next == End)
+ return std::nullopt;
+
+ return *Next;
+ }
+
+ /// Determine whether there are any operations left in this expression.
+ operator bool() const { return Start != End; }
+
+ DIExpression::expr_op_iterator begin() const { return Start; }
+ DIExpression::expr_op_iterator end() const { return End; }
+
+ /// Retrieve the fragment information, if any.
+ std::optional<DIExpression::FragmentInfo> getFragmentInfo() const {
+ return DIExpression::getFragmentInfo(Start, End);
+ }
+};
+
/// Global variables.
///
/// TODO: Remove DisplayName. It's always equal to Name.
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h b/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h
index 667a9efc6f6c04f..4daa78b15b8e298 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h
@@ -31,67 +31,6 @@ class DIELoc;
class TargetRegisterInfo;
class MachineLocation;
-/// Holds a DIExpression and keeps track of how many operands have been consumed
-/// so far.
-class DIExpressionCursor {
- DIExpression::expr_op_iterator Start, End;
-
-public:
- DIExpressionCursor(const DIExpression *Expr) {
- if (!Expr) {
- assert(Start == End);
- return;
- }
- Start = Expr->expr_op_begin();
- End = Expr->expr_op_end();
- }
-
- DIExpressionCursor(ArrayRef<uint64_t> Expr)
- : Start(Expr.begin()), End(Expr.end()) {}
-
- DIExpressionCursor(const DIExpressionCursor &) = default;
-
- /// Consume one operation.
- std::optional<DIExpression::ExprOperand> take() {
- if (Start == End)
- return std::nullopt;
- return *(Start++);
- }
-
- /// Consume N operations.
- void consume(unsigned N) { std::advance(Start, N); }
-
- /// Return the current operation.
- std::optional<DIExpression::ExprOperand> peek() const {
- if (Start == End)
- return std::nullopt;
- return *(Start);
- }
-
- /// Return the next operation.
- std::optional<DIExpression::ExprOperand> peekNext() const {
- if (Start == End)
- return std::nullopt;
-
- auto Next = Start.getNext();
- if (Next == End)
- return std::nullopt;
-
- return *Next;
- }
-
- /// Determine whether there are any operations left in this expression.
- operator bool() const { return Start != End; }
-
- DIExpression::expr_op_iterator begin() const { return Start; }
- DIExpression::expr_op_iterator end() const { return End; }
-
- /// Retrieve the fragment information, if any.
- std::optional<DIExpression::FragmentInfo> getFragmentInfo() const {
- return DIExpression::getFragmentInfo(Start, End);
- }
-};
-
/// Base class containing the logic for constructing DWARF expressions
/// independently of whether they are emitted into a DIE or into a .debug_loc
/// entry.
diff --git a/llvm/lib/IR/DebugInfoMetadata.cpp b/llvm/lib/IR/DebugInfoMetadata.cpp
index f7f36129ec8557c..29b8ddb7e854972 100644
--- a/llvm/lib/IR/DebugInfoMetadata.cpp
+++ b/llvm/lib/IR/DebugInfoMetadata.cpp
@@ -1734,6 +1734,143 @@ const DIExpression *DIExpression::extractAddressClass(const DIExpression *Expr,
return Expr;
}
+SmallVector<uint64_t>
+DIExpressionOptimizer::getOperatorLocations(ArrayRef<uint64_t> Ops) {
+ SmallVector<uint64_t> OpLocs;
+ uint64_t Loc = 0;
+ DIExpressionCursor Cursor(Ops);
+
+ while (Cursor.peek()) {
+ OpLocs.push_back(Loc);
+ auto Size = Cursor.peek()->getSize();
+ Loc += Size;
+ Cursor.consume(1);
+ }
+
+ return OpLocs;
+}
+
+bool DIExpressionOptimizer::operatorIsCommutative(uint64_t Operator) {
+ return Operator == dwarf::DW_OP_mul || Operator == dwarf::DW_OP_plus;
+}
+
+bool DIExpressionOptimizer::operatorCanBeOptimized(uint64_t Operator) {
+ switch (Operator) {
+ case dwarf::DW_OP_plus:
+ case dwarf::DW_OP_plus_uconst:
+ case dwarf::DW_OP_minus:
+ case dwarf::DW_OP_mul:
+ case dwarf::DW_OP_div:
+ case dwarf::DW_OP_shl:
+ case dwarf::DW_OP_shr:
+ return true;
+ default:
+ return false;
+ }
+}
+
+void DIExpressionOptimizer::reset() {
+ NewMathOperator = 0;
+ CurrMathOperator = 0;
+ OperatorLeft = 0;
+ OperatorRight = 0;
+}
+
+void DIExpressionOptimizer::optimize(SmallVectorImpl<uint64_t> &NewOps,
+ ArrayRef<uint64_t> Ops) {
+
+ if (Ops.size() == 2 && Ops[0] == dwarf::DW_OP_plus_uconst) {
+ // Convert a {DW_OP_plus_uconst, <constant value>} expression to
+ // {DW_OP_constu, <constant value>, DW_OP_plus}
+ NewMathOperator = dwarf::DW_OP_plus;
+ OperandRight = Ops[1];
+ OperatorRight = dwarf::DW_OP_constu;
+ } else if (Ops.size() == 3) {
+ NewMathOperator = Ops[2];
+ OperandRight = Ops[1];
+ OperatorRight = Ops[0];
+ } else {
+ // Currently, DIExpressionOptimizer only supports arithmetic operations
+ // such as Ops = {DW_OP_constu, 1, DW_OP_mul}
+ NewOps.append(Ops.begin(), Ops.end());
+ return;
+ }
+
+ if (OperatorRight != dwarf::DW_OP_constu &&
+ OperatorRight != dwarf::DW_OP_consts) {
+ NewOps.append(Ops.begin(), Ops.end());
+ return;
+ }
+
+ uint64_t CurrOperator;
+
+ if ((NewMathOperator == dwarf::DW_OP_mul ||
+ NewMathOperator == dwarf::DW_OP_div) &&
+ OperandRight == 1) {
+ // It is a multiply or divide by 1
+ reset();
+ return;
+ }
+ if ((NewMathOperator == dwarf::DW_OP_plus ||
+ NewMathOperator == dwarf::DW_OP_minus ||
+ NewMathOperator == dwarf::DW_OP_shl ||
+ NewMathOperator == dwarf::DW_OP_shr) &&
+ OperandRight == 0) {
+ // It is a add, subtract, shift left, or shift right with 0
+ reset();
+ return;
+ }
+
+ // Get the locations of the operators in the DIExpression
+ auto OperatorLocs = getOperatorLocations(NewOps);
+
+ for (auto It = OperatorLocs.rbegin(); It != OperatorLocs.rend(); It++) {
+ // Only look at the last two operators of the DIExpression to see if the new
+ // operators can be constant folded with them.
+ if (std::distance(OperatorLocs.rbegin(), It) > 1)
+ break;
+ CurrOperator = NewOps[*It];
+ if (CurrOperator == dwarf::DW_OP_mul || CurrOperator == dwarf::DW_OP_div ||
+ CurrOperator == dwarf::DW_OP_plus ||
+ CurrOperator == dwarf::DW_OP_minus) {
+ CurrMathOperator = CurrOperator;
+ continue;
+ }
+ if (CurrOperator == dwarf::DW_OP_plus_uconst &&
+ NewMathOperator == dwarf::DW_OP_plus) {
+ NewOps[*It + 1] = OperandRight + NewOps[*It + 1];
+ reset();
+ return;
+ }
+ if (CurrOperator == dwarf::DW_OP_constu ||
+ CurrOperator == dwarf::DW_OP_consts) {
+ OperatorLeft = CurrOperator;
+ OperandLeft = NewOps[*It + 1];
+
+ if (NewMathOperator == CurrMathOperator &&
+ operatorIsCommutative(NewMathOperator)) {
+ switch (NewMathOperator) {
+ case dwarf::DW_OP_plus:
+ NewOps[*It + 1] = OperandRight + OperandLeft;
+ break;
+ case dwarf::DW_OP_mul:
+ NewOps[*It + 1] = OperandRight * OperandLeft;
+ break;
+ default:
+ llvm_unreachable("Operator is not multiplication or addition!");
+ }
+ reset();
+ return;
+ }
+ break;
+ }
+ break;
+ }
+ NewOps.append(Ops.begin(), Ops.end());
+ reset();
+ return;
+}
+
DIExpression *DIExpression::prepend(const DIExpression *Expr, uint8_t Flags,
int64_t Offset) {
SmallVector<uint64_t, 8> Ops;
@@ -1846,19 +1983,19 @@ DIExpression *DIExpression::append(const DIExpression *Expr,
// Copy Expr's current op list.
SmallVector<uint64_t, 16> NewOps;
+ DIExpressionOptimizer Optimizer;
for (auto Op : Expr->expr_ops()) {
// Append new opcodes before DW_OP_{stack_value, LLVM_fragment}.
if (Op.getOp() == dwarf::DW_OP_stack_value ||
Op.getOp() == dwarf::DW_OP_LLVM_fragment) {
- NewOps.append(Ops.begin(), Ops.end());
+ Optimizer.optimize(NewOps, Ops);
// Ensure that the new opcodes are only appended once.
Ops = std::nullopt;
}
Op.appendToVector(NewOps);
}
-
- NewOps.append(Ops.begin(), Ops.end());
+ Optimizer.optimize(NewOps, Ops);
auto *result = DIExpression::get(Expr->getContext(), NewOps);
assert(result->isValid() && "concatenated expression is not valid");
return result;
diff --git a/llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-expr-chain.mir b/llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-expr-chain.mir
index 3e2244f76c905e4..b32f7310537daf4 100644
--- a/llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-expr-chain.mir
+++ b/llvm/test/DebugInfo/MIR/AArch64/dbgcall-site-expr-chain.mir
@@ -105,7 +105,7 @@ body: |
# CHECK: DW_TAG_GNU_call_site_parameter
# CHECK-NEXT: DW_AT_location (DW_OP_reg2 W2)
-# CHECK-NEXT: DW_AT_GNU_call_site_value (DW_OP_breg19 W19+700, DW_OP_plus_uconst 0x9, DW_OP_plus_uconst 0x50)
+# CHECK-NEXT: DW_AT_GNU_call_site_value (DW_OP_breg19 W19+789)
# CHECK: DW_TAG_GNU_call_site_parameter
# CHECK-NEXT: DW_AT_location (DW_OP_reg1 W1)
@@ -113,4 +113,4 @@ body: |
# CHECK: DW_TAG_GNU_call_site_parameter
# CHECK-NEXT: DW_AT_location (DW_OP_reg0 W0)
-# CHECK-NEXT: DW_AT_GNU_call_site_value (DW_OP_breg19 W19+100, DW_OP_plus_uconst 0x17)
+# CHECK-NEXT: DW_AT_GNU_call_site_value (DW_OP_breg19 W19+123)
diff --git a/llvm/unittests/IR/MetadataTest.cpp b/llvm/unittests/IR/MetadataTest.cpp
index 1570631287b2a78..2f64e4561a600a3 100644
--- a/llvm/unittests/IR/MetadataTest.cpp
+++ b/llvm/unittests/IR/MetadataTest.cpp
@@ -3120,6 +3120,81 @@ TEST_F(DIExpressionTest, get) {
EXPECT_EQ(N0WithPrependedOps, N2);
}
+TEST_F(DIExpressionTest, optimize) {
+ // Test appending a {dwarf::DW_OP_constu, <const>, DW_OP_plus} to a DW_OP_plus
+ // expression
+ uint64_t Ops[] = {dwarf::DW_OP_LLVM_arg, 0, dwarf::DW_OP_constu, 2,
+ dwarf::DW_OP_plus};
+ auto *Expr = DIExpression::get(Context, Ops);
+ uint64_t AppendOps[] = {dwarf::DW_OP_constu, 3, dwarf::DW_OP_plus};
+ auto *AppendExpr = DIExpression::append(Expr, AppendOps);
+ uint64_t OpsRes[] = {dwarf::DW_OP_LLVM_arg, 0, dwarf::DW_OP_constu, 5,
+ dwarf::DW_OP_plus};
+ auto *ResExpr = DIExpression::get(Context, OpsRes);
+ EXPECT_EQ(ResExpr, AppendExpr);
+
+ // Test appending a {dwarf::DW_OP_plus_uconst, <const>} to a DW_OP_plus
+ // expression
+ uint64_t PlusUConstOps[] = {dwarf::DW_OP_plus_uconst, 3};
+ AppendExpr = DIExpression::append(Expr, PlusUConstOps);
+ EXPECT_EQ(ResExpr, AppendExpr);
+
+ // Test appending a {dwarf::DW_OP_constu, 0, DW_OP_plus} to an expression
+ AppendOps[1] = 0;
+ AppendExpr = DIExpression::append(Expr, AppendOps);
+ OpsRes[3] = Ops[3];
+ ResExpr = DIExpression::get(Context, OpsRes);
+ EXPECT_EQ(ResExpr, AppendExpr);
+
+ // Test appending a {dwarf::DW_OP_constu, 0, DW_OP_minus} to an expression
+ AppendOps[2] = dwarf::DW_OP_minus;
+ AppendExpr = DIExpression::append(Expr, AppendOps);
+ OpsRes[3] = Ops[3];
+ ResExpr = DIExpression::get(Context, OpsRes);
+ EXPECT_EQ(ResExpr, AppendExpr);
+
+ // Test appending a {dwarf::DW_OP_constu, 0, DW_OP_shl} to an expression
+ AppendOps[2] = dwarf::DW_OP_shl;
+ AppendExpr = DIExpression::append(Expr, AppendOps);
+ OpsRes[3] = Ops[3];
+ ResExpr = DIExpression::get(Context, OpsRes);
+ EXPECT_EQ(ResExpr, AppendExpr);
+
+ // Test appending a {dwarf::DW_OP_constu, 0, DW_OP_shr} to an expression
+ AppendOps[2] = dwarf::DW_OP_shr;
+ AppendExpr = DIExpression::append(Expr, AppendOps);
+ OpsRes[3] = Ops[3];
+ ResExpr = DIExpression::get(Context, OpsRes);
+ EXPECT_EQ(ResExpr, AppendExpr);
+
+ // Test appending a {dwarf::DW_OP_constu, <const>, DW_OP_mul} to a DW_OP_mul
+ // expression
+ Ops[4] = dwarf::DW_OP_mul;
+ Expr = DIExpression::get(Context, Ops);
+ AppendOps[1] = 3;
+ AppendOps[2] = dwarf::DW_OP_mul;
+ AppendExpr = DIExpression::append(Expr, AppendOps);
+ OpsRes[3] = 6;
+ OpsRes[4] = dwarf::DW_OP_mul;
+ ResExpr = DIExpression::get(Context, OpsRes);
+ EXPECT_EQ(ResExpr, AppendExpr);
+
+ // Test appending a {dwarf::DW_OP_constu, 1, DW_OP_mul} to an expression
+ AppendOps[1] = 1;
+ AppendExpr = DIExpression::append(Expr, AppendOps);
+ OpsRes[3] = Ops[3];
+ ResExpr = DIExpression::get(Context, OpsRes);
+ EXPECT_EQ(ResExpr, AppendExpr);
+
+ // Test appending a {dwarf::DW_OP_constu, 1, DW_OP_div} to an expression
+ AppendOps[1] = 1;
+ AppendOps[2] = dwarf::DW_OP_div;
+ AppendExpr = DIExpression::append(Expr, AppendOps);
+ OpsRes[3] = Ops[3];
+ ResExpr = DIExpression::get(Context, OpsRes);
+ EXPECT_EQ(ResExpr, AppendExpr);
+}
+
TEST_F(DIExpressionTest, isValid) {
#define EXPECT_VALID(...) \
do { \
``````````
</details>
https://github.com/llvm/llvm-project/pull/69770
More information about the llvm-commits
mailing list