Skip to content
This repository was archived by the owner on Sep 15, 2025. It is now read-only.

Commit 72cce05

Browse files
perlfuperlfu
committed
Revert "Do not generate dummy exports for PS on GFX10"
There is a subtle interaction with fragment shaders which write to color buffers with no attachment. LLPC removes these outputs, but it appears some shaders are expecting an output to happen and relying on the results of a discard through this. This was not picked up by Vulkan CTS testing, persumably as the pipelines it uses are well-defined, but this behaviour does occur in game titles. The old dummy export behaviour was masking this problem by always exporting when discards occur. Revert to this behaviour while we investigate further. This reverts commit 533147b.
1 parent 10cea37 commit 72cce05

5 files changed

Lines changed: 18 additions & 11 deletions

File tree

‎lgc/elfLinker/GlueShader.cpp‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,7 @@ class ColorExportShader : public GlueShader {
156156
// The encoded or hashed (in some way) single string version of the above.
157157
std::string m_shaderString;
158158
PipelineState *m_pipelineState; // The pipeline state. Used to set meta data information.
159+
bool m_killEnabled; // True if this fragment shader has kill enabled.
159160
};
160161
} // anonymous namespace
161162

@@ -361,6 +362,11 @@ ColorExportShader::ColorExportShader(PipelineState *pipelineState, ArrayRef<Colo
361362
static_cast<ExportFormat>(pipelineState->computeExportFormat(exp.ty, exp.location));
362363
}
363364
m_pipelineState = pipelineState;
365+
366+
PalMetadata *metadata = pipelineState->getPalMetadata();
367+
DB_SHADER_CONTROL shaderControl = {};
368+
shaderControl.u32All = metadata->getRegister(mmDB_SHADER_CONTROL);
369+
m_killEnabled = shaderControl.bits.KILL_ENABLE;
364370
}
365371

366372
// =====================================================================================================================
@@ -372,6 +378,7 @@ StringRef ColorExportShader::getString() {
372378
constexpr uint32_t estimatedTypeSize = 10;
373379
uint32_t sizeEstimate = (sizeof(ColorExportInfo) + estimatedTypeSize) * m_exports.size();
374380
sizeEstimate += sizeof(m_exportFormat);
381+
sizeEstimate += sizeof(m_killEnabled);
375382
m_shaderString.reserve(sizeEstimate);
376383

377384
for (ColorExportInfo colorExportInfo : m_exports) {
@@ -384,6 +391,7 @@ StringRef ColorExportShader::getString() {
384391
m_shaderString += getTypeName(colorExportInfo.ty);
385392
}
386393
m_shaderString += StringRef(reinterpret_cast<const char *>(m_exportFormat), sizeof(m_exportFormat)).str();
394+
m_shaderString += StringRef(reinterpret_cast<const char *>(&m_killEnabled), sizeof(m_killEnabled));
387395
}
388396
return m_shaderString;
389397
}
@@ -410,7 +418,7 @@ Module *ColorExportShader::generate() {
410418
values[m_exports[idx].hwColorTarget] = colorExportFunc->getArg(idx);
411419
}
412420

413-
bool dummyExport = m_lgcContext->getTargetInfo().getGfxIpVersion().major < 10;
421+
bool dummyExport = m_lgcContext->getTargetInfo().getGfxIpVersion().major < 10 || m_killEnabled;
414422
fragColorExport->generateExportInstructions(m_exports, values, m_exportFormat, dummyExport, builder);
415423
return colorExportFunc->getParent();
416424
}
@@ -453,5 +461,5 @@ void ColorExportShader::updatePalMetadata(PalMetadata &palMetadata) {
453461
break;
454462
}
455463
}
456-
palMetadata.updateSpiShaderColFormat(m_exports, hasDepthExpFmtZero);
464+
palMetadata.updateSpiShaderColFormat(m_exports, hasDepthExpFmtZero, m_killEnabled);
457465
}

‎lgc/include/lgc/state/PalMetadata.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ class PalMetadata {
164164
void finalizePipeline(bool isWholePipeline);
165165

166166
// Updates the PS register information that depends on the exports.
167-
void updateSpiShaderColFormat(llvm::ArrayRef<ColorExportInfo> exps, bool hasDepthExpFmtZero);
167+
void updateSpiShaderColFormat(llvm::ArrayRef<ColorExportInfo> exps, bool hasDepthExpFmtZero, bool killEnabled);
168168

169169
// Sets the finalized 128-bit cache hash. The version identifies the version of LLPC used to generate the hash.
170170
void setFinalized128BitCacheHash(const lgc::Hash128 &finalizedCacheHash, const llvm::VersionTuple &version);

‎lgc/patch/FragColorExport.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -488,12 +488,13 @@ bool LowerFragColorExport::runOnModule(Module &module) {
488488
exportFormat[exp.hwColorTarget] =
489489
static_cast<ExportFormat>(m_pipelineState->computeExportFormat(exp.ty, exp.location));
490490
}
491-
bool dummyExport = m_pipelineState->getTargetInfo().getGfxIpVersion().major < 10;
491+
bool dummyExport =
492+
(m_pipelineState->getTargetInfo().getGfxIpVersion().major < 10 || m_resUsage->builtInUsage.fs.discard);
492493
fragColorExport.generateExportInstructions(m_info, m_exportValues, exportFormat, dummyExport, builder);
493494

494495
const auto &builtInUsage = m_resUsage->builtInUsage.fs;
495496
bool hasDepthExpFmtZero = !(builtInUsage.sampleMask || builtInUsage.fragStencilRef || builtInUsage.fragDepth);
496-
m_pipelineState->getPalMetadata()->updateSpiShaderColFormat(m_info, hasDepthExpFmtZero);
497+
m_pipelineState->getPalMetadata()->updateSpiShaderColFormat(m_info, hasDepthExpFmtZero, builtInUsage.discard);
497498
return !m_info.empty() || dummyExport;
498499
}
499500

‎lgc/state/PalMetadata.cpp‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -842,9 +842,7 @@ llvm::Type *PalMetadata::getLlvmType(StringRef tyName) const {
842842
// =====================================================================================================================
843843
// Updates the SPI_SHADER_COL_FORMAT entry.
844844
//
845-
// @param exps : Array describing all color exports.
846-
// @param hasDepthExpFmtZero : Flag indicating absence of depth exports.
847-
void PalMetadata::updateSpiShaderColFormat(ArrayRef<ColorExportInfo> exps, bool hasDepthExpFmtZero) {
845+
void PalMetadata::updateSpiShaderColFormat(ArrayRef<ColorExportInfo> exps, bool hasDepthExpFmtZero, bool killEnabled) {
848846
unsigned spiShaderColFormat = 0;
849847
for (auto &exp : exps) {
850848
if (exp.hwColorTarget == MaxColorTargets)
@@ -854,7 +852,7 @@ void PalMetadata::updateSpiShaderColFormat(ArrayRef<ColorExportInfo> exps, bool
854852
}
855853

856854
if (spiShaderColFormat == 0 && hasDepthExpFmtZero) {
857-
if (m_pipelineState->getTargetInfo().getGfxIpVersion().major < 10) {
855+
if (m_pipelineState->getTargetInfo().getGfxIpVersion().major < 10 || killEnabled) {
858856
// NOTE: Hardware requires that fragment shader always exports "something" (color or depth) to the SX.
859857
// If both SPI_SHADER_Z_FORMAT and SPI_SHADER_COL_FORMAT are zero, we need to override
860858
// SPI_SHADER_COL_FORMAT to export one channel to MRT0. This dummy export format will be masked

‎llpc/test/shaderdb/relocatable_shaders/PipelineVsFs_GlueCache.pipe‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
; CREATE: ID for glue shader0: 00000000000000007632663332570000000200000003000000040000000F00000000000000030000000400000000000000000000000000000000000000000000000E0000000700000000000000
1919
; CREATE: Cache miss for glue shader 0
2020
; CREATE: Updating the cache for glue shader 0
21-
; CREATE: ID for glue shader1: 00000000000000000076346633320900000000000000000000000000000000000000000000000000000000000000
21+
; CREATE: ID for glue shader1: 0000000000000000007634663332090000000000000000000000000000000000000000000000000000000000000000
2222
; CREATE: Cache miss for glue shader 1
2323
; CREATE: Updating the cache for glue shader 1
2424
; CREATE: ===== AMDLLPC SUCCESS =====
@@ -48,7 +48,7 @@
4848
; LOAD: ID for glue shader0: 00000000000000007632663332570000000200000003000000040000000F00000000000000030000000400000000000000000000000000000000000000000000000E0000000700000000000000
4949
; LOAD: Cache hit for glue shader 0
5050
; LOAD-NOT: Updating the cache for glue shader
51-
; LOAD: ID for glue shader1: 00000000000000000076346633320900000000000000000000000000000000000000000000000000000000000000
51+
; LOAD: ID for glue shader1: 0000000000000000007634663332090000000000000000000000000000000000000000000000000000000000000000
5252
; LOAD: Cache hit for glue shader 1
5353
; LOAD-NOT: Updating the cache for glue shader
5454
; LOAD: ===== AMDLLPC SUCCESS =====

0 commit comments

Comments
 (0)