Use DIExpression::foldConstantMath() at the result of an append() #71719
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-debuginfo @llvm/pr-subscribers-llvm-ir Author: Shubham Sandeep Rastogi (rastogishubham) ChangesThis patch uses This is part of a stack of patches and comes after: Patch is 30.65 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/71719.diff 6 Files Affected:
diff --git a/llvm/include/llvm/IR/DebugInfoMetadata.h b/llvm/include/llvm/IR/DebugInfoMetadata.h
index b347664883fd9f2..354ae48c3d676c1 100644
--- a/llvm/include/llvm/IR/DebugInfoMetadata.h
+++ b/llvm/include/llvm/IR/DebugInfoMetadata.h
@@ -3044,6 +3044,11 @@ class DIExpression : public MDNode {
/// expression and constant on failure.
std::pair<DIExpression *, const ConstantInt *>
constantFold(const ConstantInt *CI);
+
+ /// Try to shorten an expression with constand math operations that can be
+ /// evaulated at compile time. Returns a new expression on success, or the old
+ /// expression if there is nothing to be reduced.
+ DIExpression *foldConstantMath();
};
inline bool operator==(const DIExpression::FragmentInfo &A,
@@ -3073,6 +3078,86 @@ template <> struct DenseMapInfo<DIExpression::FragmentInfo> {
static bool isEqual(const FragInfo &A, const FragInfo &B) { return A == B; }
};
+/// 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;
+ }
+
+ std::optional<DIExpression::ExprOperand> peekNextN(unsigned N) const {
+ if (Start == End)
+ return std::nullopt;
+ DIExpression::expr_op_iterator Nth = Start;
+ for (unsigned I = 0; I < N; I++) {
+ Nth = Nth.getNext();
+ if (Nth == End)
+ return std::nullopt;
+ }
+ return *Nth;
+ }
+
+ void assignNewExpr(ArrayRef<uint64_t> Expr) {
+ DIExpression::expr_op_iterator NewStart(Expr.begin());
+ DIExpression::expr_op_iterator NewEnd(Expr.end());
+ this->Start = NewStart;
+ this->End = NewEnd;
+ }
+
+ /// 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..2dee2e5e5448555 100644
--- a/llvm/lib/IR/DebugInfoMetadata.cpp
+++ b/llvm/lib/IR/DebugInfoMetadata.cpp
@@ -1857,11 +1857,10 @@ DIExpression *DIExpression::append(const DIExpression *Expr,
}
Op.appendToVector(NewOps);
}
-
NewOps.append(Ops.begin(), Ops.end());
auto *result = DIExpression::get(Expr->getContext(), NewOps);
assert(result->isValid() && "concatenated expression is not valid");
- return result;
+ return result->foldConstantMath();
}
DIExpression *DIExpression::appendToStack(const DIExpression *Expr,
@@ -1998,6 +1997,288 @@ DIExpression::constantFold(const ConstantInt *CI) {
ConstantInt::get(getContext(), NewInt)};
}
+static bool isConstOperation(uint64_t Op) {
+ return Op == dwarf::DW_OP_constu || Op == dwarf::DW_OP_consts;
+}
+
+static bool operationIsNoOp(uint64_t Op, uint64_t Val) {
+ switch (Op) {
+ case dwarf::DW_OP_plus:
+ case dwarf::DW_OP_minus:
+ case dwarf::DW_OP_shl:
+ case dwarf::DW_OP_shr:
+ return Val == 0;
+ case dwarf::DW_OP_mul:
+ case dwarf::DW_OP_div:
+ return Val == 1;
+ default:
+ return false;
+ }
+}
+
+static std::optional<uint64_t>
+foldOperationIfPossible(uint64_t Op, uint64_t Operand1, uint64_t Operand2) {
+ switch (Op) {
+ case dwarf::DW_OP_plus:
+ return Operand1 + Operand2;
+ case dwarf::DW_OP_minus:
+ return Operand1 - Operand2;
+ case dwarf::DW_OP_shl:
+ return Operand1 << Operand2;
+ case dwarf::DW_OP_shr:
+ return Operand1 >> Operand2;
+ case dwarf::DW_OP_mul:
+ return Operand1 * Operand2;
+ case dwarf::DW_OP_div:
+ return Operand1 / Operand2;
+ default:
+ return std::nullopt;
+ }
+}
+
+static bool operationsAreFoldableAndCommutative(uint64_t Op1, uint64_t Op2) {
+ if (Op1 != Op2)
+ return false;
+ switch (Op1) {
+ case dwarf::DW_OP_plus:
+ case dwarf::DW_OP_mul:
+ return true;
+ default:
+ return false;
+ }
+}
+
+static void moveCursorAndLocPos(DIExpressionCursor &Cursor, uint64_t &Loc,
+ const DIExpression::ExprOperand &Op) {
+ Cursor.consume(1);
+ Loc = Loc + Op.getSize();
+}
+
+DIExpression *DIExpression::foldConstantMath() {
+
+ SmallVector<uint64_t, 8> WorkingOps;
+ WorkingOps.append(Elements.begin(), Elements.end());
+ uint64_t Loc = 0;
+ DIExpressionCursor Cursor(WorkingOps);
+
+ while (Loc < WorkingOps.size()) {
+
+ auto Op1 = Cursor.peek();
+ // Expression has no operations, exit
+ if (!Op1)
+ break;
+ auto Op1Raw = Op1->getOp();
+ auto Op1Arg = Op1->getArg(0);
+
+ // {DW_OP_plus_uconst, 0} -> {}
+ if (Op1Raw == dwarf::DW_OP_plus_uconst && Op1Arg == 0) {
+ WorkingOps.erase(WorkingOps.begin() + Loc, WorkingOps.begin() + Loc + 2);
+ moveCursorAndLocPos(Cursor, Loc, *Op1);
+ continue;
+ }
+
+ if (!isConstOperation(Op1Raw) && Op1Raw != dwarf::DW_OP_plus_uconst) {
+ // Early exit, all of the following patterns start with a constant value
+ moveCursorAndLocPos(Cursor, Loc, *Op1);
+ continue;
+ }
+
+ auto Op2 = Cursor.peekNext();
+ // All following patterns require at least 2 Operations, exit
+ if (!Op2)
+ break;
+ auto Op2Raw = Op2->getOp();
+
+ // {DW_OP_const[u, s], 0, DW_OP_[plus, minus, shl, shr]} -> {}
+ // {DW_OP_const[u, s], 1, DW_OP_[mul, div]} -> {}
+ if (isConstOperation(Op1Raw) && operationIsNoOp(Op2Raw, Op1Arg)) {
+ WorkingOps.erase(WorkingOps.begin() + Loc, WorkingOps.begin() + Loc + 3);
+ Cursor.assignNewExpr(WorkingOps);
+ Loc = 0;
+ continue;
+ }
+
+ auto Op2Arg = Op2->getArg(0);
+
+ // {DW_OP_plus_uconst, Const1, DW_OP_plus_uconst, Const2} ->
+ // {DW_OP_plus_uconst, Const1 + Const2}
+ if (Op1Raw == dwarf::DW_OP_plus_uconst &&
+ Op2Raw == dwarf::DW_OP_plus_uconst) {
+ auto Result = Op1Arg + Op2Arg;
+ WorkingOps.erase(WorkingOps.begin() + Loc + 2,
+ WorkingOps.begin() + Loc + 4);
+ WorkingOps[Loc + 1] = Result;
+ Cursor.assignNewExpr(WorkingOps);
+ Loc = 0;
+ continue;
+ }
+
+ // {DW_OP_const[u, s], Const1, DW_OP_plus_uconst Const2} -> {DW_OP_constu,
+ // Const1 + Const2}
+ if (isConstOperation(Op1Raw) && Op2Raw == dwarf::DW_OP_plus_uconst) {
+ auto Result = Op1Arg + Op2Arg;
+ WorkingOps.erase(WorkingOps.begin() + Loc + 2,
+ WorkingOps.begin() + Loc + 4);
+ WorkingOps[Loc] = dwarf::DW_OP_constu;
+ WorkingOps[Loc + 1] = Result;
+ Cursor.assignNewExpr(WorkingOps);
+ Loc = 0;
+ continue;
+ }
+
+ auto Op3 = Cursor.peekNextN(2);
+ // Op2 could still match a pattern, skip iteration
+ if (!Op3) {
+ moveCursorAndLocPos(Cursor, Loc, *Op1);
+ continue;
+ }
+ auto Op3Raw = Op3->getOp();
+
+ // {DW_OP_const[u, s], Const1, DW_OP_const[u, s], Const2, DW_OP_[plus,
+ // minus, mul, div, shl, shr] -> {DW_OP_constu, Const1 [+, -, *, /, <<, >>]
+ // Const2}
+ if (isConstOperation(Op1Raw) && isConstOperation(Op2Raw)) {
+ auto Result = foldOperationIfPossible(Op3Raw, Op1Arg, Op2Arg);
+ if (!Result) {
+ moveCursorAndLocPos(Cursor, Loc, *Op1);
+ continue;
+ }
+ WorkingOps.erase(WorkingOps.begin() + Loc + 2,
+ WorkingOps.begin() + Loc + 5);
+ WorkingOps[Loc] = dwarf::DW_OP_constu;
+ WorkingOps[Loc + 1] = *Result;
+ Cursor.assignNewExpr(WorkingOps);
+ Loc = 0;
+ continue;
+ }
+
+ // {DW_OP_plus_uconst, Const1, DW_OP_const[u, s], Const1, DW_OP_plus} ->
+ // {DW_OP_plus_uconst, Const1 + Const2}
+ if (Op1Raw == dwarf::DW_OP_plus_uconst && isConstOperation(Op2Raw) &&
+ Op3Raw == dwarf::DW_OP_plus) {
+ auto Result = Op1Arg + Op2Arg;
+ WorkingOps.erase(WorkingOps.begin() + Loc + 2,
+ WorkingOps.begin() + Loc + 5);
+ WorkingOps[Loc + 1] = Result;
+ Cursor.assignNewExpr(WorkingOps);
+ Loc = 0;
+ continue;
+ }
+
+ auto Op3Arg = Op3->getArg(0);
+ // {DW_OP_const[u, s], Const1, DW_OP_plus, DW_OP_plus_uconst, Const2} ->
+ // {DW_OP_plus_uconst, Const1 + Const2}
+ if (isConstOperation(Op1Raw) && Op2Raw == dwarf::DW_OP_plus &&
+ Op3Raw == dwarf::DW_OP_plus_uconst) {
+ auto Result = Op1Arg + Op3Arg;
+ WorkingOps.erase(WorkingOps.begin() + Loc + 2,
+ WorkingOps.begin() + Loc + 5);
+ WorkingOps[Loc] = dwarf::DW_OP_plus_uconst;
+ WorkingOps[Loc + 1] = Result;
+ Cursor.assignNewExpr(WorkingOps);
+ Loc = 0;
+ continue;
+ }
+
+ auto Op4 = Cursor.peekNextN(3);
+ // Op2 and Op3 could still match a pattern, skip iteration
+ if (!Op4) {
+ moveCursorAndLocPos(Cursor, Loc, *Op1);
+ continue;
+ }
+ auto Op4Raw = Op4->getOp();
+
+ // {DW_OP_const[u, s], Const1, DW_OP_[plus, mul], Const2, DW_OP_[plus, mul]}
+ // -> {DW_OP_constu, Const1 [+, *] Const2, DW_OP_[plus, mul]}
+ if (isConstOperation(Op1Raw) && isConstOperation(Op3Raw) &&
+ operationsAreFoldableAndCommutative(Op2Raw, Op4Raw)) {
+ auto Result = foldOperationIfPossible(Op2Raw, Op1Arg, Op3Arg);
+ if (!Result)
+ llvm_unreachable(
+ "Something went very wrong here! This should always work");
+ WorkingOps.erase(WorkingOps.begin() + Loc + 3,
+ WorkingOps.begin() + Loc + 6);
+ WorkingOps[Loc] = dwarf::DW_OP_constu;
+ WorkingOps[Loc + 1] = *Result;
+ Cursor.assignNewExpr(WorkingOps);
+ Loc = 0;
+ continue;
+ }
+ auto Op5 = Cursor.peekNextN(4);
+ if (!Op5) {
+ moveCursorAndLocPos(Cursor, Loc, *Op1);
+ continue;
+ }
+ auto Op5Raw = Op5->getOp();
+ auto Op5Arg = Op5->getArg(0);
+
+ // {DW_OP_const[u, s], Const1, DW_OP_plus, DW_OP_LLVM_arg, Arg1, DW_OP_plus,
+ // DW_OP_plus_uconst, Const2} -> {DW_OP_constu, Const1 + Const2, DW_OP_plus,
+ // DW_OP_LLVM_arg, Arg1, DW_OP_plus}
+ if (isConstOperation(Op1Raw) && Op3Raw == dwarf::DW_OP_LLVM_arg &&
+ Op5Raw == dwarf::DW_OP_plus_uconst && Op2Raw == Op4Raw &&
+ Op4Raw == dwarf::DW_OP_plus) {
+ auto Result = Op1Arg + Op5Arg;
+ WorkingOps.erase(WorkingOps.begin() + Loc + 6,
+ WorkingOps.begin() + Loc + 8);
+ WorkingOps[Loc] = dwarf::DW_OP_constu;
+ WorkingOps[Loc + 1] = Result;
+ Cursor.assignNewExpr(WorkingOps);
+ Loc = 0;
+ continue;
+ }
+
+ auto Op4Arg = Op4->getArg(0);
+
+ // {DW_OP_plus_uconst, Const1, DW_OP_LLVM_arg, Arg1, DW_OP_plus,
+ // DW_OP_const[u, s], Const2, DW_OP_plus} -> {DW_OP_plus_uconst, Const1 +
+ // Const2, DW_OP_LLVM_arg, Arg1, DW_OP_plus}
+ if (Op1Raw == dwarf::DW_OP_plus_uconst && Op2Raw == dwarf::DW_OP_LLVM_arg &&
+ Op3Raw == Op5Raw && Op3Raw == dwarf::DW_OP_plus &&
+ isConstOperation(Op4Raw)) {
+ auto Result = Op1Arg + Op4Arg;
+ WorkingOps.erase(WorkingOps.begin() + Loc + 5,
+ WorkingOps.begin() + Loc + 8);
+ WorkingOps[Loc + 1] = Result;
+ Cursor.assignNewExpr(WorkingOps);
+ Loc = 0;
+ continue;
+ }
+
+ auto Op6 = Cursor.peekNextN(5);
+ if (!Op6) {
+ moveCursorAndLocPos(Cursor, Loc, *Op1);
+ continue;
+ }
+ auto Op6Raw = Op6->getOp();
+ // {DW_OP_const[u, s], Const1, DW_OP_[plus, mul], DW_OP_LLVM_arg, Arg1,
+ // DW_OP_[plus, mul], DW_OP_const[u, s], Const2, DW_OP_[plus, mul]} ->
+ // {DW_OP_constu, Const1 [+, *] Const2, DW_OP_[plus, mul], DW_OP_LLVM_arg,
+ // Arg1, DW_OP_[plus, mul]}
+ if (isConstOperation(Op1Raw) && Op3Raw == dwarf::DW_OP_LLVM_arg &&
+ isConstOperation(Op5Raw) &&
+ operationsAreFoldableAndCommutative(Op2Raw, Op4Raw) &&
+ operationsAreFoldableAndCommutative(Op4Raw, Op6Raw)) {
+ auto Result = foldOperationIfPossible(Op2Raw, Op1Arg, Op5Arg);
+ if (!Result)
+ llvm_unreachable(
+ "Something went very wrong here! This should always work");
+ WorkingOps.erase(WorkingOps.begin() + Loc + 6,
+ WorkingOps.begin() + Loc + 9);
+ WorkingOps[Loc] = dwarf::DW_OP_constu;
+ WorkingOps[Loc + 1] = *Result;
+ Cursor.assignNewExpr(WorkingOps);
+ Loc = 0;
+ continue;
+ }
+
+ moveCursorAndLocPos(Cursor, Loc, *Op1);
+ }
+ auto *Result = DIExpression::get(getContext(), WorkingOps);
+ assert(Result->isValid() && "concatenated expression is not valid");
+ return Result;
+}
+
uint64_t DIExpression::getNumLocationOperands() const {
uint64_t Result = 0;
for (auto ExprOp : expr_ops())
diff --git a/llvm/test/Bitcode/upgrade-dbg-addr.ll b/llvm/test/Bitcode/upgrade-dbg-addr.ll
index 40fd7db18948b57..1943c35b0059fb1 100644
--- a/llvm/test/Bitcode/upgrade-dbg-addr.ll
+++ b/llvm/test/Bitcode/upgrade-dbg-addr.ll
@@ -8,7 +8,7 @@ entry:
%num.addr = alloca i32, align 4
store i32 %num, ptr %num.addr, align 4
; CHECK-NOT: call void @llvm.dbg.addr
- ; CHECK: call void @llvm.dbg.value(metadata ptr %num.addr, metadata ![[#]], metadata !DIExpression(DW_OP_plus_uconst, 0, DW_OP_deref))
+ ; CHECK: call void @llvm.dbg.value(metadata ptr %num.addr, metadata ![[#]], metadata !DIExpression(DW_OP_deref))
call void @llvm.dbg.addr(metadata ptr %num.addr, metadata !16, metadata !DIExpression(DW_OP_plus_uconst, 0)), !dbg !17
%0 = load i32, ptr %num.addr, align 4
ret i32 %0
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..f233d2ff5d25269 100644
--- a/llvm/unittests/IR/MetadataTest.cpp
+++ b/llvm/unittests/IR/MetadataTest.cpp
@@ -3120,6 +3120,369 @@ TEST_F(DIExpressionTest, get) {
EXPECT_EQ(N0WithPrependedOps, N2);
}
+TEST_F(DIExpressionTest, Fold) {
+
+ // Remove a No-op DW_OP_plus_uconst from an expression.
+ SmallVector<uint64_t, 8> Ops = {dwarf::DW_OP_plus_uconst, 0};
+ auto *Expr = DIExpression::get(Context, Ops);
+ auto *E = Expr->foldConstantMath();
+ SmallVector<uint64_t, 8> ResOps;
+ auto *ResExpr = DIExpression::get(Context, ResOps);
+ EXPECT_EQ(E, ResExpr);
+
+ // Remove a No-op add from an expression.
+ Ops[0] = dwarf::DW_OP_constu;
+ Ops[1] = 0;
+ Ops.push_back(dwarf::DW_OP_plus);
+ Expr = DIExpression::get(Context, Ops);
+ E = Expr->foldConstantMath();
+ ResExpr = DIExpression::get(Context, ResOps);
+ EXPECT_EQ(E, ResExpr);
+
+ // Remove a No-op subtract from an expression.
+ Ops[2] = dwarf::DW_OP_minus;
+ Expr = DIExpression::get(Context, Ops);
+ E = Expr->foldConstantMath();
+ ResExpr = DIExpression::get(Context, ResOps);
+ EXPECT_EQ(E, ResExpr);
+
+ // Remove a No-op shift left from an expression.
+ Ops[2] = dwarf::DW_OP_shl;
+ Expr = DIExpression::get(Context, Ops);
+ E = Expr->foldConstantMath();
+ ResExpr = DIExpression::get(Context, ResOps);
+ EXPECT_EQ(E, ResExpr);
+
+ // Remove a No-op shift right from an expression.
+ Ops[2] = dwarf::DW_OP_shr;
+ Expr = DIExpression::get(Context, Ops);
+ E = Expr->foldConstantMath();
+ ResExpr = DIExpression::get(Context, ResOps);
+ EXPECT_EQ(E, ResExpr);
+
+ // Remove a No-op multiply from an expression.
+ Ops[2] = dwarf::DW_OP_mul;
+ Ops[1] = 1;
+ Expr = DIExpression::get(Context, Ops);
+ E = Expr->foldConstantMath();
+ ResExpr = DIExpression::get(Context, ResOps);
+ EXPECT_EQ(E, ResExpr);
+
+ // Remove a No-op divide from an expression.
+ Ops[2] = dwarf::DW_OP_div;
+ Expr = DIExpression::get(Context, Ops);
+ E = Expr->foldConstantMath();
+ ResExpr = DIExpression::get(Context, ResOps);
+ EXPECT_EQ(E, ResExpr);
+
+ // Test fold {DW_OP_plus_uconst, Const1, DW_OP_plus_uconst, Const2} ->
+ // {DW_OP_plus_uconst, Const1 + Const2}
+ Ops[0] = dwarf::DW_OP_plus_uconst;
+ Ops[1] = 2;
+ Ops[2] = dwarf::DW_OP_plus_uconst;
+ Ops.push_back(3);
+ Expr = DIExpression::get(Context, Ops);
+ E = Expr->foldConstantMath();
+ ResOps.push_back(dwarf::DW_OP_plus_uconst);
+ ResOps.push_back(5);
+ ResExpr = DIExpression::get(Context, ResOps);
+ EXPECT_EQ(E, ResExpr);
+
+ // Test {DW_OP_constu, Const1, DW_OP_plus_uconst, Const2} -> {DW_OP...
[truncated]
|
rastogishubham
requested review from
MaskRay,
jmorse,
adrian-prantl,
dwblaikie and
SLTozer
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
rastogishubham
force-pushed
the
DIExpressionAppend
branch
from
e37ff7f
to
f29e07c
Compare
jmorse
approved these changes
Nov 14, 2023
llvm/unittests/IR/MetadataTest.cpp
Outdated
Comment on lines
3432
to
3433
| OpsRes[2] = dwarf::DW_OP_constu; | ||
| OpsRes[3] = Ops[3]; |
There was a problem hiding this comment.
Nit: working out what's in OpsRes between each test is going to place a reading-burden on someone who has to come and read this test, IMO it's better to re-declare a new vector and initialize-list it, rather than let state drift across EXPECT sections.
rastogishubham
force-pushed
the
DIExpressionAppend
branch
from
f29e07c
to
324f23c
Compare
|
@jmorse I have updated this patch, I am not aware of how to push to the compile time tracker, can you please help me out |
|
@adrian-prantl ping |
rastogishubham
force-pushed
the
DIExpressionAppend
branch
3 times, most recently
from
3676abd
to
0ccd27c
Compare
rastogishubham
force-pushed
the
DIExpressionAppend
branch
from
0ccd27c
to
1ad7409
Compare
I can submit your branch to the compile time tracker, but it looks like the final commit and at least some of the preceding commits fail when building, so there's no measurement right now. If you get the patches building successfully with tests passing, I can resubmit it to get compile time stats. |
rastogishubham
force-pushed
the
DIExpressionAppend
branch
3 times, most recently
from
62feaf5
to
9219203
Compare
rastogishubham
force-pushed
the
DIExpressionAppend
branch
from
9219203
to
c095fbb
Compare
rastogishubham
force-pushed
the
DIExpressionAppend
branch
3 times, most recently
from
d0f9195
to
8f7b790
Compare
rastogishubham
force-pushed
the
DIExpressionAppend
branch
3 times, most recently
from
31dc7c0
to
84a8122
Compare
|
|
||
| using namespace llvm; | ||
|
|
||
| static std::optional<uint64_t> isConstantVal(DIExpression::ExprOperand Op) { |
There was a problem hiding this comment.
It looks like you accidentally dropped all the Doxygen comments from the static functions?
|
|
||
| using namespace llvm; | ||
|
|
||
| static std::optional<uint64_t> isConstantVal(DIExpression::ExprOperand Op) { |
There was a problem hiding this comment.
Should this be isUnsignedConstant?
| return std::nullopt; | ||
| } | ||
|
|
||
| static bool isNeutralElement(uint64_t Op, uint64_t Val) { |
There was a problem hiding this comment.
Can the first argument be of type dwarf::LocationAtom for more clarity?
| } | ||
| } | ||
|
|
||
| static std::optional<uint64_t> foldOperationIfPossible( |
There was a problem hiding this comment.
This function needs a few comments explaining at a high level what's happening. Especially the inout bool looks suspicious from a design perspective.
rastogishubham
force-pushed
the
DIExpressionAppend
branch
2 times, most recently
from
ef82cc9
to
a9383f7
Compare
rastogishubham
force-pushed
the
DIExpressionAppend
branch
from
a9383f7
to
c083167
Compare
alanzhao1
pushed a commit
to alanzhao1/llvm-project
that referenced
this pull request
May 29, 2024
…sion (llvm#71721) This patch uses `DIExpression::foldConstantMath()` at the result of a Salvaged expression, that is, it runs the folding optimizations after an expression has been salvaged completely, to reduce how many times the fold optimization function is called. Which should help in reducing the size of DIExpressions that grow because of salvaging debug info After checking the size of the dSYM with and without this change, I saw a decrease of about 300KB, where the debug_loc section is about 1.6 GB in size. Where the debug loc section reduced in size by 212KB and it is 193MB in size, the rest comes from the debug_info section This is part of a stack of patches and comes after: llvm#69768 llvm#71717 llvm#71718 llvm#71719
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch uses
DIExpression::foldConstantMath()at the end of aDIExpression::append(). Which should help in reducing the size of DIExpressions that grow because of salvaging debug infoThis is part of a stack of patches and comes after:
#69768
#71717
#71718