[PATCH] D79190: llvm rejects DWARF operator DW_OP_lit[1-31].
Alok Kumar Sharma via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri May 1 01:48:32 PDT 2020
alok marked 8 inline comments as done.
alok added a comment.
In D79190#2013650 <https://reviews.llvm.org/D79190#2013650>, @probinson wrote:
> If I take your C input program and compile it to a .o file, I see that we do emit (DW_OP_lit13; DW_OP_lit0; DW_OP_mul; DW_OP_stack_value) for "var2".
>
> I think what you are saying is, we do not accept DW_OP_litN as *input* in the IR. Is that correct? If so, please make sure the commit log states that.
Yes correct. The C program works because clang generates "DW_OP_constu, 5", which is optimized to DW_OP_lit5, but it doesnt accept DW_OP_lit5 in IR. I missed to state that clearly in commit log. I shall update the commit log. Thanks for your review.
In D79190#2014425 <https://reviews.llvm.org/D79190#2014425>, @aprantl wrote:
> I think this may actually be to some degree intentional. In LLVM IR. What's the use-case for this? Currently in LLVM IR a constant is always represented by a DW_OP_const(u) which makes the expressions easier to parse. LLVM knows how to *emit* DW_OP_litX, but I'm not sure what we gain from accepting it in LLVM IR.
Actually I found this while working on debugability support for flang. It looked strange that DW_OP_lit0 works but not other values. I understand that the same can be represented by DW_OP_const(u) but the smaller representation with DW_OP_lit is tempting. Supporting the same from completeness point of view as well as avoiding unnecessary issue reporting by the new front-ends emitting DW_OP_litX looked tempting.
================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp:400
continue;
} else if (OpNum >= dwarf::DW_OP_breg0 && OpNum <= dwarf::DW_OP_breg31) {
addBReg(OpNum - dwarf::DW_OP_breg0, Op->getArg(0));
----------------
probinson wrote:
> I know you did not introduce this, so I won't insist you change it, but LLVM style says do not use `else` after a `continue`.
Thanks for pointing this out, I shall correct this.
================
Comment at: llvm/test/DebugInfo/dwarfdump-litN.ll:4
+
+; RUN: %llc_dwarf %s -O2 -filetype=obj -mtriple x86_64-unknown-linux -o %t.o
+; RUN: llvm-dwarfdump %t.o | FileCheck %s
----------------
probinson wrote:
> Remove the `-mtriple`, because you are using the %llc_dwarf substitution, which will add `-mtriple` when necessary.
Thanks for your comment. I shall incorporate this.
================
Comment at: llvm/test/DebugInfo/dwarfdump-litN.ll:14
+
+; 2. Test whetehr DW_OP_lit5 works.
+; CHECK-LABEL: DW_TAG_variable
----------------
probinson wrote:
> Typo, "whether"
Thanks for pointing this out. I shall incorporate this.
================
Comment at: llvm/test/DebugInfo/dwarfdump-litN.ll:36
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
----------------
probinson wrote:
> Remove the target triple line, so the input will work for any target.
Thanks. I shall update this in next version.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79190/new/
https://reviews.llvm.org/D79190
More information about the llvm-commits
mailing list