[PATCH] D116821: [DebugInfo][InstrRef] Move instr-ref controlling flag out of TargetOptions

Jeremy Morse via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 7 10:11:39 PST 2022


jmorse created this revision.
jmorse added reviewers: Orlando, StephenTozer, djtodoro, TWeaver.
Herald added subscribers: pengfei, hiraditya.
jmorse requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Hi,

This patch moves the flag controlling instruction referencing out of TargetOptions and into a plain old LLVM option. I don't think TargetOptions is the right place for this to be, mostly because I've tried twice [0, 1] to have instruction referencing on-by-default in clang, and it's still not working out (which is highly unfortunate). Both clang, and seemingly other frontends, populate TargetOptions with their own opinions on what codegen should look like, rather than starting with the defaults from `codegen::InitTargetOptionsFromCodeGenFlags`. This doesn't make it a good place to control an *optional* extra part of LLVM. Plus, it's harder to control from LTO [2].

This patch switches the truth-test for using instruction referencing from a field in TargetOptions to the added debuginfoShouldUseDebugInstrRef method, which performs the same checks on Triple + command line flags. This will actually turn on instruction referencing for x86 on all frontends, like I intended in [0] :/. I would add an end-to-end test checking that clang does the right thing (produces DBG_INSTR_REF after SelectionDAG), but there doesn't seem to be a right place to put it (cross-project-tests maybe, but it doesn't have any proper compiler tests there). CC @dblaikie in case he has an opinion on such testing.

It's massively rubbish that this has cut out a large amount of time where I thought this was getting test exposure (possibly LTO is still testing it), I just didn't understand how TargetOptions worked. That might mean this has to be reverted / disabled on the release branch (sad faces) if there's turbulence. Too bad.

[0] D114631 <https://reviews.llvm.org/D114631>
[1] rGea22fdd120 <https://reviews.llvm.org/rGea22fdd120aeb1bbb9ea96670d70193dc02b2c5f>
[2] https://lists.llvm.org/pipermail/llvm-dev/2020-December/147090.html


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116821

Files:
  llvm/lib/CodeGen/CommandFlags.cpp
  llvm/lib/CodeGen/LiveDebugValues/LiveDebugValues.cpp
  llvm/lib/CodeGen/LiveDebugValues/LiveDebugValues.h
  llvm/lib/CodeGen/MachineFunction.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D116821.398180.patch
Type: text/x-patch
Size: 4365 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220107/32f63420/attachment.bin>


More information about the llvm-commits mailing list