[PATCH] D159485: [DebugInfo][RemoveDIs] Use getStableDebugLoc when picking IRBuilder source locations

[Jeremy Morse via Phabricator via llvm-commits]

jmorse created this revision.
jmorse added reviewers: Orlando, arsenm, scott.linder.
Herald added subscribers: luke, sunshaoce, frasercrmck, kerbowa, luismarques, apazos, sameer.abuasal, pengfei, s.egerton, Jim, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, zzheng, jrtc27, niosHD, sabuasal, simoncook, johnrusso, rbar, asb, hiraditya, tpr, jvesely.
Herald added a project: All.
jmorse requested review of this revision.
Herald added subscribers: llvm-commits, wangpc, MaskRay, wdng.
Herald added a project: LLVM.

Hi,

This is a patch broken out of D152468 <https://reviews.llvm.org/D152468> and D151419 <https://reviews.llvm.org/D151419> that deploys the "getStableDebugLoc" method -- this is a version of getDebugLoc that skips past any debug intrinsics to find a "real instruction" DebugLoc. Without this change, we can set real-instruction source locations to source locations that come from debug intrinsics, which aren't reliable and might vary with compiler flags. It turns out there are a few tests that are sensitive to this, the changes to which are elaborated below.

Firstly `llvm/test/CodeGen/AMDGPU/llvm.dbg.value.ll` (CC @arsenm and @scott.linder ) -- it appears that this is testing that the assembly syntax for a DWARF variable location is emitted correctly, but none of the "real" instructions have source locations. The test seems to implicitly rely on the source locations in the dbg.value leaking into the adjacent instructions, so that there's a lexical scope to cover. I've added source locations to the other IR instructions here to make the test pass again, would you agree this is the correct fix?

`llvm/test/Transforms/SROA/vector-promotion.ll` : The arithmetic instructions added were taking their source location from the dbg.value at the end of the function; they now take it from the "ret" instruction. This still makes sense, as the "ret" is the SSA Value Use that causes the instructions to be generated. The bitcast gets a different source location, corresponding to the "load" instruction in the source IR: I think this makes sense as that's what's converting the vector into an integer, through some stack store/loading.

`LoopIdiom/X86/logical-right-shift-until-zero-debuginfo.ll` and the simiarly named file: previously the generated instructions got the source location of the PHI in the loop because that's what was attached to the nearby dbg.value. Now the generated instructions get the source location of the first "real" instruction in the loop, which makes more sense to me.

`hwaddresssanitizer/alloca.ll`: All the hwasan generated instrs were getting the line number from the dbg.value, i.e. zero, which is liable to annoy developers. Now it comes from the call to use32, which I believe is what causes HWASAN to add instrumentation anyway. Added check-globals in the hope that in the future people will notice if those instructions become zero-line-numbered then someone will notice.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D159485

Files:
  llvm/include/llvm/IR/IRBuilder.h
  llvm/include/llvm/IR/Instruction.h
  llvm/lib/IR/Instruction.cpp
  llvm/test/CodeGen/AMDGPU/llvm.dbg.value.ll
  llvm/test/Instrumentation/HWAddressSanitizer/RISCV/alloca.ll
  llvm/test/Instrumentation/HWAddressSanitizer/alloca.ll
  llvm/test/Transforms/LoopIdiom/X86/arithmetic-right-shift-until-zero.ll
  llvm/test/Transforms/LoopIdiom/X86/logical-right-shift-until-zero-debuginfo.ll
  llvm/test/Transforms/SROA/vector-promotion.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D159485.556257.patch
Type: text/x-patch
Size: 63769 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230908/d013c4a9/attachment.bin>