SuccessChanges

Summary

  1. [DebugInfo] Stop describing imms in TargetInstrInfo's (details)
Commit 74a72e684849d00dbe5cc784cf05d20fd8873cdb by david.stenberg
[DebugInfo] Stop describing imms in TargetInstrInfo's
describeLoadedValue() impl
Summary: The default implementation of the describeLoadedValue() hook
uses the MoveImm property to determine if an instruction moves an
immediate. If an instruction has that property the function returns the
second operand, assuming that that is the immediate value the
instruction moves. As far as I can tell, the MoveImm property does not
imply that the second operand is the immediate value, nor that any other
operand necessarily holds the immediate value; it just means that the
instruction moves some immediate value.
One example where the second operand is not the immediate is SystemZ's
LZER instruction, which moves a zero immediate implicitly: $f0S = LZER.
That case triggered an out-of-bound assertion when getting the operand.
I have added a test case for that instruction.
Another example is ARM's MVN instruction, which holds the logical
bitwise NOT'd value of the immediate that is moved. For the following
reproducer:
  extern void foo(int);
int main() { foo(-11); }
an incorrect call site value would be emitted:
  $ clang --target=arm foo.c -O1 -g -Xclang -femit-debug-entry-values \
     -c -o - | ./build/bin/llvm-dwarfdump  - | \
     grep -A2 call_site_parameter
  0x00000058:       DW_TAG_GNU_call_site_parameter
                     DW_AT_location (DW_OP_reg0 R0)
                     DW_AT_GNU_call_site_value (DW_OP_lit10)
Another example is the A2_combineii instruction on Hexagon which moves
two immediates to a super-register: $d0 = A2_combineii 20, 10.
Perhaps these are rare exceptions, and most MoveImm instructions hold
the immediate in the second operand, but in my opinion the default
implementation of the hook should only describe values that it can, by
some contract, guarantee are safe to describe, rather than leaving it up
to the targets to override the exceptions, as that can silently result
in incorrect call site values.
This patch adds X86's relevant move immediate instructions to the
target's hook implementation, so this commit should be a NFC for that
target. We need to do the same for ARM and AArch64.
Reviewers: djtodoro, NikolaPrica, aprantl, vsk
Reviewed By: vsk
Subscribers: kristof.beyls, hiraditya, llvm-commits
Tags: #debug-info, #llvm
Differential Revision: https://reviews.llvm.org/D69109
The file was modifiedllvm/lib/CodeGen/TargetInstrInfo.cpp
The file was modifiedllvm/lib/Target/X86/X86InstrInfo.cpp
The file was addedllvm/test/DebugInfo/MIR/SystemZ/call-site-lzer.mir
The file was addedllvm/test/DebugInfo/MIR/SystemZ/lit.local.cfg