Potential issue merging resource coherency states

DxilObjectProperties::updateGLC updates coherency by an XOR when the state does not match. See here:
https://github.com/microsoft/DirectXShaderCompiler/blob/206b77577d15fc5798eb7ad52290388539b7146d/tools/clang/lib/CodeGen/CGHLSLMSFinishCodeGen.cpp#L4037-L4043

When adding the reodercoherent resource attribute and updating this function to DxilObjectProperties::updateCoherence here:
https://github.com/microsoft/DirectXShaderCompiler/pull/7250/files#diff-8a49420abc3b2570e61d7846868d54b5485f52b79f0b9f7bb6e258598a9c4ebaR4047

This follows the existing pattern, but I'm not sure that the existing or new code does exactly the right thing.

First, it doesn't seem correct to ever be removing a coherency flag when correcting a mismatch (by using XOR). I think we would only want to add it when it is missing. This is similar to what we would want to do when merging AnnotateHandle instructions and merging the DxilResourceProperties.

Second, since reorder coherent is subsumed by globally coherent, if we would end up with both flags set, we should have a single canonical representation in the resource properties, where we clear the reorder coherent flag when the globally coherent flag is set (since that was the existing representation for GLC).

Third, I think we need to spell out the scenarios, what we should do for each when the coherency mismatches, and make sure we test each one. This will allow us to confirm whether the first point is correct.

There are some implications if these first two points are correct:

I think we could replace the pair of bools used in getCoherenceMismatch and updateCoherence with a new enum ResourceCoherency defined in DxilConstants.h. We could add basic helpers/accessors using the new enum value to better enforce the canonical coherency representation in flags. The following are some proposed changes.

• Updates to DxilConstants.h:
  • Add enum class ResourceCoherency : unsigned { None = 0, Global = 1, Reorder = 2 };
  • inline operator bool(DXIL::ResourceCoherency RC) { return RC != DXIL::ResourceCoherency::None; }
  • inline DXIL::ResourceCoherency DXIL::MakeResourceCoherency(bool GLC, bool RDC) {...}
  • inline DXIL::ResourceCoherency DXIL::MergeResourceCoherency(DXIL::ResourceCoherency A, DXIL::ResourceCoherency B) {...} - only increase coherency if different
• Updates to DxilResourceProperties:
  • void DxilResourceProperties::setCoherency(ResourceCoherency RC);
  • ResourceCoherency DxilResourceProperties::getCoherency() const;
  • Update tryMergeProps to reflect only increasing coherency (similar to how SamplerCmpOrHasCounter is updated).
• Updates to DxilResource for the internal representation to use the enum, and for accessors to enforce canonical representations in the flags (clear RDC if GLC is set).
• Updates to getCoherenceMismatch and updateCoherence:
  • getCoherenceMismatch may return the ResourceCoherency enum, where:
    • ResourceCoherency::None means no update required,
    • otherwise, we update to the new coherency state.
  • updateCoherence has a ResourceCoherency param instead of two bools.

Start with DxilObjectProperties::updateGLC in tools/clang/lib/CodeGen/CGHLSLMSFinishCodeGen.cpp and review the proposed updateCoherence change, then inspect DxilConstants.h, DxilResourceProperties, and DxilResource. Enumerate the coherency mismatch scenarios and test each one; done means coherency only increases and resource properties use one canonical representation when global coherency is set.