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

Fri Jan 7 10:35:14 PST 2022

dblaikie added a comment.

In D116821#3227990 <https://reviews.llvm.org/D116821#3227990>, @jmorse wrote:

> In D116821#3227958 <https://reviews.llvm.org/D116821#3227958>, @dblaikie wrote:
>
>> Not sure what you mean by "it doesn't have any proper compiler tests there", proper in what sense? I expect you could do something like my recent simplified template names roundtrip test ( cross-project-tests/debuginfo-tests/clang_llvm_roundtrip/simplified_template_names.cpp ) which runs clang and checks the result with llvm-dwarfdump. Presumably the same could be done for this functionality - validating that some certain locations that are a signal that instruction referencing was used.
>
> Thanks for the example; by "proper compiler tests" I was meaning that the existing tests involve examining or running the produced binary, rather than stopping compilation half way through and looking at internal representation details. i.e.: `clang test.c -mllvm -stop-after=finalize-isel -o - | FileCheck blah`. That's an easy test to write, but there's no similar prior art as far as I understand it.

Oh, that seems OK to test too/instead - though I'd sort of lean towards cross project tests being more end-to-end "functional" tests, like what's the external behavior we expect to see for this change (I assume that's some improved location quality/coverage/etc). It might be that this. stop-after test might be suitable to live in clang. (tests for things that aren't IR serialized are imperfect - MCoptions (like DWARF type units), etc, - either are untested in clang, or are tested by going a bit further into LLVM than we usually would for a clang test, but I think that's a limitation we live with)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116821