[llvm] [SPIRV] Emitting DebugSource, DebugCompileUnit (PR #97558)

[via llvm-commits]

bwlodarcz wrote:

> To explain my point of view, consider the recent fail of github actions step "Test SPIR-V / Lit Tests" applied to this PR. The error with OpLabel generation was caught by spirv-val rather than your CHECK's. The test didn't have CHECK's relevant to this error before spirv-val discovered it, and it didn't get any new CHECK's even after your recent commit [ad701a2](https://github.com/llvm/llvm-project/commit/ad701a28544bf83550dde24e2e667443d0865db5) that fixed spirv-val's complaint.

Ok let's go through this.
The solution for the problem was to actually emit new instructions on the end of the BB. Why? The comment explains it:
```
    // To correct placement of a OpLabel instruction during SPIRVAsmPrinter
    // emission all new instructions needs to be placed after OpFunction
    MachineIRBuilder MIRBuilder(MBB, MBB.end());
``` 
The error was a consequence that in SPIRVAsmPrinter.cpp `emitInstruction`:
```
  // Output OpLabel after OpFunction and OpFunctionParameter in the first MBB.
  const MachineInstr *NextMI = MI->getNextNode();
  if (!MAI->hasMBBRegister(*MI->getParent()) && isFuncOrHeaderInstr(MI, TII) &&
      (!NextMI || !isFuncOrHeaderInstr(NextMI, TII))) {
    assert(MI->getParent()->getNumber() == MF->front().getNumber() &&
           "OpFunction is not in the front MBB of MF");
    emitOpLabel(*MI->getParent());
  }
```
doesn't catch the scenario when you are emitting instructions on the head of the first MBB.
Why? Because if have `(!NextMI || !isFuncOrHeaderInstr(NextMI, TII))` so it's true if next instruction isn't `FuncOrHeaderInstr` and I emitted two `OpExtInst` before `OpFunction` which triggered the clause.
This kind of hidden assumptions aren't something which you reasonable want to catch in introducing first quasi-PoC implementation of DI nor are something to even have in the codebase in the first place.
And sorry to pointing this out but e.g. if you would trigger the same condition e.g. in #96091 or #95147 you also aren't testing the existence of `OpLabel` after `OpFunction` in newly created tests and would miss this. 
Also note that these also doesn't have such extensive multifile, multi conditional testing which you require from my PR and are in semantic (more important) part of the codebase. 

Please understand that the codebase isn't set in stone. It's not a skyscraper that after laying fundamentals and building a few floors you are fixed to the initial design. Code have wonderful property of being modifyable and you can change it in the future to whatever shape you want (and you should because requirements/algorithms change). 
The code and assumptions will change with maturity of the implementations and the extensive repetitive (to other) tests are a liability in prototype phase.
The PR was posted early because I'm following the philosophy of "given enough eyeballs, all bugs are shallow". I expected to put some additional work on it and I'm happy when you find out the mistakes which I missed. Especially due to your maturity in the project in comparison to mine.

The current test is for the `OpString`, `DebugSource` and `DebugCompilationUnit` emission and maximizes code coverage for these three ops and only for them.
DI tests (especially on the initial, prototype phase) shouldn't check anything besides DI because:
a) you don't want to change all you test cases everywhere when e.g. something will change in the order of emission. 
b) you want to understand what's happening inside the PR just by looking on the test 

In my opinion the test at current stage is enough. Testing empty doesn't have a sense (because there is no DI emitted at all) and functions manipulations aren't relevant for this PR - copy paste of this test, removal of one function doesn't change or test anything in the logic and emission of duplicates is tested by the two function version which we have now.
Metadata contract (for now) is also tested for correctness.

https://github.com/llvm/llvm-project/pull/97558