[DebugInfo] Fix instruction enumeration

[frasercrmck]

These two opcodes were mistakenly swapped when they were originally added, at least according to the DebugInfo, OpenCL.DebugInfo.100, and NonSemantic.Shader.DebugInfo.100 extended instruction sets.

This might break existing third-party SPIR-V translators if they are accommodating this bug, mistakenly or otherwise.

[frasercrmck]

I'm new to this repo so I don't know how best to test this. This isn't a bug that is easily observed within the closed-loop llvm-spirv cycle, but could be seen with external tools using the correct instruction enumeration. I only spotted one test using spirv-dis so I thought I'd check here first.

[MrSidims]

Thanks for noticing and the patch! It can be tested via adding .spvasm test, calling spirv-as to it and then reverse translating it. Though I personally won't insist on it.

One thing that worries me, that llvm-spirv based SPIR-V consumers, that might be slightly lagging from the trunk will become broken after this fix. Probably it should be fixed on the downstream. Anyway, can we give some graceful period before merging the fix?

[frasercrmck]

Thanks for noticing and the patch! It can be tested via adding .spvasm test, calling spirv-as to it and then reverse translating it. Though I personally won't insist on it.

Ah yes, good idea!. I'll try my hand at that while I wait for reviews etc.

One thing that worries me, that llvm-spirv based SPIR-V consumers, that might be slightly lagging from the trunk will become broken after this fix. Probably it should be fixed on the downstream.

Yes, I'm a bit worried about that too. I've no idea what's generally done in these situations!

I don't think it's possible for downstream consumers to reliably distinguish between these two opcodes, unless the pack has > 1 optional operand, but that could also be seen as an invalid SPIR-V binary.

I was also wondering if this sort of thing is back-ported to the older LLVM branches?

Anyway, can we give some graceful period before merging the fix?

That might make sense. What kind of period do you have in mind? I (Codeplay) have an interest in getting this fixed in Intel's DPC++ quite shortly, so I might need to look into patching it "downstream" there first.

[frasercrmck]

I've added a LIT test.

[frasercrmck]

I've updated the test to remove the invalid DebugCompilationUnit source language (I copied from another test)

The test still isn't spirv-val clean, but spirv-val doesn't seem to accept DebugInfoNone in many places.

[asudarsa]

Closed and reopened to rerun tests.
@MrSidims and @frasercrmck
Is this ready to be merged?
May be we can add a '[DONOTMERGE]' label if it's not ready?

Thanks