[PATCH] D91043: [DebugInfo] Fix convert-loclist.ll

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D91043#2414969 <https://reviews.llvm.org/D91043#2414969>, @luismarques wrote:

> In D91043#2406312 <https://reviews.llvm.org/D91043#2406312>, @probinson wrote:
>
>> I think the first question is rhetorical, but it ensures a DWARF target.  The second question is the key:  What makes this test special, for @luismarques environment?  If you build with just RISC-V as the target, this entire directory should become unsupported.  If you have both X86 and RISC-V, then these tests do run, and apparently (mostly) run fine.  Why didn't this one?
>
> I think the answer is perhaps that you need *all* of:
>
> 1. a non-x86 machine;
> 2. a build that includes the (non-native) x86 target;
> 3. a test that is x86-specific, but doesn't specify the x86 target triple nor `REQUIRES` x86.
>
> Perhaps this test and my build setup was a rare occurrence that matched all 3 requirements?

(3) is part of it, but not quite. All these test "REQUIRES" x86 (the DebugInfo/X86 directory has this as a general requirement) - but I did figure out how some tests don't use an -mtriple (so they don't conflict with the llc_dwarf usage): They use a target triple in the IR itself (which I guess doesn't produce an error - not sure if the IR's triple is ignored, or the command line - but one of them seems to be).

So that explains some benign llc_dwarf usage. Though not all of it - I found 7 tests without -mtriple or a target triple in the IR:

  llvm/test/DebugInfo/X86/convert-debugloc.ll                                                                                                                                                                                                                                                                                                                             
  llvm/test/DebugInfo/X86/convert-inlined.ll                                                                                                                                                                                                                                                                                                                              
  llvm/test/DebugInfo/X86/convert-linked.ll                                                                                                                                                                                                                                                                                                                               
  llvm/test/DebugInfo/X86/convert-loclist.ll                                                                                                                                                                                                                                                                                                                              
  llvm/test/DebugInfo/X86/dbg-byval-parameter.ll            
  llvm/test/DebugInfo/X86/debug-macro.ll
  llvm/test/DebugInfo/X86/dwarf-callsite-related-attrs.ll

Perhaps the other 6 here are actually not X86 specific & are portable/should be moved to test/DebugInfo (non-X86)... hmm. Seems some are probably portable, others maybe one RUN line in the file is portable. So I've just wholesale switched them all to explicit triples to avoid excess churn - though maybe some of them could've been moved to the generic test bucket in test/DebugInfo directly.

Thanks for your patience, sorry I drew this out a bit further - but glad to better understand what was going on. (mostly it was the IR triples I think that surprised me)

I've committed the cleanup of these 6-7 tests using llc_dwarf without a triple in the metadata (that should close out this review). I'll commit a follow-up commit removing the llc_dwarf from the remaining tests here that rely on the IR target triple, since they don't need the llc_dwarf, I don't think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91043