[PATCH] D99273: [DebugInfo] Support for signed constants inside DIExpression

Jeremy Morse via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 25 04:51:33 PDT 2021


jmorse added a comment.

Sounds good overall, with a comment inline about the test for isSignedConstant. IMO, there should be some test coverage of expressions that shouldn't be recognised as a signed constant too.



================
Comment at: llvm/lib/IR/DebugInfoMetadata.cpp:1471
+  if ((getNumElements() == 2 && getElement(0) != dwarf::DW_OP_consts) ||
+      (getNumElements() == 3 && getElement(0) != dwarf::DW_OP_consts &&
+       getElement(2) != dwarf::DW_OP_stack_value))
----------------
I may have mis-read, but shouldn't the end `&&` on this line be `||`? Either the zeroth element not being consts, *or* the second element not being stack value, would mean this doesn't match the constant pattern, no?

Additionally: shouldn't there be a clause for `getNumElements() == 6` here? Otherwise, any six element expression with a fragment suffix will be identified a constant by this function, regardless of the first three elements.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99273/new/

https://reviews.llvm.org/D99273



More information about the llvm-commits mailing list