Revert "shader: keep scalar data native and preserve raw addresses"

This reverts commit cd9fc6ff2d.
2026-10-20 02:47:02 +02:00 · 2026-10-05 17:34:12 +04:00
parent 7a0e1dafd7
commit 266c12cec6
6 changed files with 50 additions and 184 deletions
@@ -664,7 +664,6 @@ static std::vector<ResourceBlock> ResourceControlFlow(const Program& program, co
 		}
 	}
 	std::vector<ResourceBlock> blocks(program.blocks.size());
-	std::vector<std::vector<uint32_t>> predecessors(blocks.size());
 	for (uint32_t i = 0; i < blocks.size(); i++) {
 		auto&                 block      = blocks[i];
 		const auto&           info       = program.block_info[i];
@@ -688,7 +687,6 @@ static std::vector<ResourceBlock> ResourceControlFlow(const Program& program, co
 				return {};
 			}
 			block.successors.push_back(found->second);
-			predecessors[found->second].push_back(i);
 		}
 		for (const auto& inst: *program.blocks[i]) {
 			const auto op     = inst.GetOpcode();
@@ -718,30 +716,7 @@ static std::vector<ResourceBlock> ResourceControlFlow(const Program& program, co
 		block.sources.erase(std::unique(block.sources.begin(), block.sources.end()),
 		                    block.sources.end());
 	}
-	// Host branches only matter when they guard a later resource access. Leave
-	// data-only branches (for example a mutable flag checked before a trap) on the GPU.
-	std::vector<uint8_t> resource_reachable(blocks.size());
-	std::vector<uint32_t> pending;
-	for (uint32_t i = 0; i < blocks.size(); ++i) {
-		if (!blocks[i].sources.empty() || !blocks[i].srt_reads.empty()) {
-			resource_reachable[i] = 1;
-			pending.push_back(i);
-		}
-	}
-	while (!pending.empty()) {
-		const auto index = pending.back();
-		pending.pop_back();
-		for (const auto predecessor: predecessors[index]) {
-			if (resource_reachable[predecessor]) continue;
-			resource_reachable[predecessor] = 1;
-			pending.push_back(predecessor);
-		}
-	}
-	for (auto& block: blocks) {
-		block.condition = std::ranges::any_of(block.successors, [&](uint32_t successor) {
-			return resource_reachable[successor] != 0;
-		}) ? predicate(block.condition) : Value {};
-	}
+	for (auto& block: blocks) block.condition = predicate(block.condition);
 	if (std::ranges::none_of(
 	        blocks, [](const ResourceBlock& block) { return !block.condition.IsEmpty(); })) {
 		return {};
@@ -742,6 +742,15 @@ private:
 				}
 			}
 		}
+		for (auto* block: m_program.blocks) {
+			for (auto& inst: *block) {
+				uint32_t index = 0;
+				if (inst.GetOpcode() == ValueOpcode::LoadAddressU32 &&
+				    ScalarReadMemory(inst, index) != nullptr && inst.Arg(1).Resolve().IsImmediate() &&
+				    ValidateRuntimeValue(m_program, Value(&inst)))
+					CollectScalarRead(Value(&inst), inst.Flags<MemoryFlags>().pc);
+			}
+		}
 		for (auto* read: m_scalar_reads) {
 			const auto flags = read->Flags<MemoryFlags>();
 			auto& memory = m_program.memory_info[flags.index];
@@ -23,6 +23,9 @@ namespace {
 constexpr uint64_t AddressMask = 0x0000ffffffffffffull;

 bool AddSignedAddress(uint64_t base, int64_t offset, uint64_t& result) {
+	if (base > AddressMask) {
+		return false;
+	}
 	if (offset < 0) {
 		const auto magnitude = uint64_t {0} - static_cast<uint64_t>(offset);
 		if (magnitude > base) {
@@ -32,7 +35,7 @@ bool AddSignedAddress(uint64_t base, int64_t offset, uint64_t& result) {
 		return true;
 	}
 	const auto magnitude = static_cast<uint64_t>(offset);
-	if (magnitude > UINT64_MAX - base) {
+	if (magnitude > AddressMask - base) {
 		return false;
 	}
 	result = base + magnitude;
@@ -477,7 +480,7 @@ bool SrtWalker::EvaluateRawRead(const Inst& inst, uint64_t& result) {
 	if (!Arg(*handle, 0, low) || !Arg(*handle, 1, high) || !Arg(inst, 1, offset)) {
 		return false;
 	}
-	const auto base      = (high << 32u) | static_cast<uint32_t>(low);
+	const auto base      = ((high << 32u) | static_cast<uint32_t>(low)) & AddressMask;
 	const auto immediate = static_cast<int64_t>(static_cast<int32_t>(mem.offset));
 	uint64_t   address   = 0;
 	if (inst.GetOpcode() == ValueOpcode::ReadConstBuffer || vector) {
@@ -507,7 +510,7 @@ bool SrtWalker::EvaluateRawRead(const Inst& inst, uint64_t& result) {
 		if (byte_offset > size || size - byte_offset < sizeof(uint32_t)) {
 			return false;
 		}
-		address = (base & AddressMask & ~uint64_t {3}) + byte_offset;
+		address = (base & ~uint64_t {3}) + byte_offset;
 	} else {
 		const auto relative = (immediate & ~int64_t {3}) +
 		                      static_cast<int64_t>(static_cast<uint32_t>(offset) & ~3u);
@@ -32,8 +32,7 @@ AddValueBlock(Libs::Graphics::ShaderRecompiler::IR::Program &program) {
  return *result;
 }

-Libs::Graphics::ShaderRecompiler::IR::ResourcePlan SrtPlan(uint64_t address,
-                                                       bool buffer = false) {
+Libs::Graphics::ShaderRecompiler::IR::ResourcePlan SrtPlan(uint64_t address) {
  using namespace Libs::Graphics::ShaderRecompiler::IR;
  Program program;
  program.stage = Libs::Graphics::ShaderType::Compute;
@@ -42,22 +41,16 @@ Libs::Graphics::ShaderRecompiler::IR::ResourcePlan SrtPlan(uint64_t address,
  auto &value_block = AddValueBlock(program);

  MemoryInfo memory;
-  memory.kind = buffer ? ResourceKind::ScalarBuffer : ResourceKind::ScalarAddress;
+  memory.kind = ResourceKind::ScalarAddress;
  memory.planning_only = true;
  program.memory_info.push_back(memory);
  const auto low = Value(static_cast<uint32_t>(address));
  const auto high = Value(static_cast<uint32_t>(address >> 32u));
-  auto &handle = buffer
-                     ? value_block.AppendNewInst(ValueOpcode::GetBufferResource,
-                                                {low, high, Value(4u), Value(0u)})
-                     : value_block.AppendNewInst(ValueOpcode::GetAddressResource,
-                                                {low, high});
-  auto &raw = buffer
-                  ? value_block.AppendNewInst(ValueOpcode::ReadConstBuffer,
-                                             {Value(&handle), Value(0u)})
-                  : value_block.AppendNewInst(
-                        ValueOpcode::LoadAddressU32,
-                        {Value(&handle), Value(0u), Value(0u), Value(true)});
+  auto &handle =
+      value_block.AppendNewInst(ValueOpcode::GetAddressResource, {low, high});
+  auto &raw = value_block.AppendNewInst(
+      ValueOpcode::LoadAddressU32,
+      {Value(&handle), Value(0u), Value(0u), Value(true)});
  raw.SetFlags(MemoryFlags{.index = 0, .pc = 0x40});
  program.srt_reads.push_back({Value(&raw), 0});

@@ -175,32 +168,6 @@ void TestMappedSrtUsesDirectReaderByDefault() {
        "cache rematerialization did not use the direct reader by default");
 }

-void TestRawScalarAddressPreservesHighBits() {
-  using namespace Libs::Graphics::ShaderRecompiler::IR;
-  constexpr uint64_t address = 0xabcd000156780003ull;
-  for (const bool buffer : {false, true}) {
-    auto plan = SrtPlan(address, buffer);
-    uint64_t observed = 0;
-    const SrtRuntime runtime{
-        .read_memory = [](void *userdata, uint64_t requested,
-                          std::span<uint32_t> words) {
-          if (words.size() != 1) return false;
-          *static_cast<uint64_t *>(userdata) = requested;
-          words[0] = 0x12345678;
-          return true;
-        },
-        .userdata = &observed};
-    ResourceSnapshot snapshot;
-    ResourceSpecialization specialization;
-    Check(MaterializeResources(plan, runtime, snapshot, specialization),
-          "scalar address materialization failed");
-    Check(observed == (buffer ? 0x0000000156780000ull : address & ~uint64_t{3}),
-          "raw pointer high bits and buffer descriptor flags were conflated");
-    Check(snapshot.flattened_srt == std::vector<uint32_t>{0x12345678},
-          "scalar address read did not reach its flattened slot");
-  }
-}
-
 void TestIntegerRuntimeValueFollowsSrtReads() {
  using namespace Libs::Graphics::ShaderRecompiler::IR;
  auto plan = SrtPlan(0x10000);
@@ -599,7 +566,6 @@ void DbgExit(int) { std::abort(); }

 int main() {
  TestMappedSrtUsesDirectReaderByDefault();
-  TestRawScalarAddressPreservesHighBits();
  TestIntegerRuntimeValueFollowsSrtReads();
  TestUniformVectorDescriptorRead();
  TestExactReciprocalDescriptorArithmetic();
@@ -3096,79 +3096,6 @@ ResourcePlan ConditionalBufferPlan(ConditionalBufferUse use) {
  return ExtractResourcePlan(fixture.program);
 }

-void TestNativeScalarAtomicDataDoesNotEnterResourcePlan() {
-  namespace CFG = Libs::Graphics::ShaderRecompiler::CFG;
-  Fixture fixture;
-  auto *entry = fixture.block;
-  for (uint32_t i = 1; i < 5; ++i) fixture.AddBlock();
-  fixture.program.block_info[0].terminator = {
-      .kind = CFG::TerminatorKind::ConditionalBranch, .true_block = 1, .false_block = 4};
-  for (uint32_t i = 1; i < 4; ++i) {
-    fixture.program.block_info[i].terminator = {
-        .kind = CFG::TerminatorKind::Branch, .true_block = i + 1};
-  }
-  fixture.program.block_info[4].terminator.kind = CFG::TerminatorKind::Return;
-
-  const auto table = fixture.Address(fixture.UserData(0), fixture.UserData(1));
-  MemoryInfo scalar;
-  scalar.kind = ResourceKind::ScalarAddress;
-  scalar.component_count = 2;
-  scalar.offset = 136;
-  const auto low = fixture.Emit(ValueOpcode::LoadAddressU32,
-      {table, Value(0u), Value(0u), Value(true)}, fixture.AddMemory(scalar, 0x560));
-  scalar.offset = 140;
-  scalar.component_index = 1;
-  const auto high = fixture.Emit(ValueOpcode::LoadAddressU32,
-      {table, Value(0u), Value(0u), Value(true)}, fixture.AddMemory(scalar, 0x560));
-  const auto address = fixture.Address(low, high);
-  scalar.offset = 24;
-  scalar.component_count = 1;
-  scalar.component_index = 0;
-  const auto data_flags = fixture.AddMemory(scalar, 0x584);
-  const auto data = fixture.Emit(ValueOpcode::LoadAddressU32,
-      {address, Value(0u), Value(0u), Value(true)}, data_flags);
-  const auto descriptor = fixture.Buffer({low,
-      fixture.Emit(ValueOpcode::BitwiseOr32, {high, Value(256u << 16u)}),
-      Value(1u), Value(90628u)}, 0x58c);
-  MemoryInfo vector;
-  vector.kind = ResourceKind::Buffer;
-  vector.offset = 24;
-  vector.data_dwords = 2;
-  fixture.Emit(ValueOpcode::BufferAtomicOr64,
-      {descriptor, Value(0u), Value(0u), Value(0u), Value(uint64_t{1}), Value(true)},
-      fixture.AddMemory(vector, 0x58c));
-  const auto flag = fixture.Emit(ValueOpcode::BitwiseAnd32, {data, Value(16u)});
-  fixture.program.block_info[0].condition =
-      fixture.Emit(ValueOpcode::INotEqual32, {flag, Value(0u)});
-  fixture.PlanAndTrack();
-  Check(data.Instruction()->Parent() == entry &&
-            !fixture.program.memory_info[data_flags.index].planning_only &&
-            fixture.program.info.buffers.size() == 1 && fixture.program.info.buffers[0].atomic,
-        "mutable scalar data was replaced by a host snapshot before its native atomic write");
-  auto plan = ExtractResourcePlan(fixture.program);
-  Check(plan.srt_reads.size() == 2 && plan.control_flow.empty() &&
-            !plan.capture_specialization_reads,
-        "resource-free successor chain retained the shader's scalar data predicate");
-  struct Reads { uint32_t descriptors = 0; uint32_t data = 0; } reads;
-  const auto Read = +[](void *userdata, uint64_t address, std::span<uint32_t> words) {
-    auto &reads = *static_cast<Reads *>(userdata);
-    if (address == 0x2018u) { ++reads.data; return false; }
-    if (words.size() != 1 || (address != 0x1088u && address != 0x108cu)) return false;
-    ++reads.descriptors;
-    words[0] = address == 0x1088u ? 0x2000u : 0u;
-    return true;
-  };
-  const std::array<uint32_t, 2> user_data{0x1000u, 0u};
-  const SrtRuntime runtime{.user_data = user_data, .read_memory = Read, .userdata = &reads,
-                           .read_specialization_memory = Read};
-  ResourceSnapshot snapshot;
-  ResourceSpecialization specialization;
-  Check(MaterializeResources(plan, runtime, snapshot, specialization) &&
-            reads.descriptors == 2 && reads.data == 0 && snapshot.specialization_reads.empty() &&
-            snapshot.buffers[0].dwords[0] == 0x2000u,
-        "resource materialization read mutable scalar data or lost its descriptor pointer");
-}
-
 void TestConditionalBufferMaterialization() {
  auto plan = ConditionalBufferPlan(ConditionalBufferUse::Optional);
  // GTA III leaves packet words in s[12:15] when its scalar control word is zero.
@@ -3728,7 +3655,6 @@ int main() {
    Run("DMA address materialization", TestDmaAddressMaterialization);
    Run("dynamic FLAT address", TestDynamicFlatAddressesUseDma);
    Run("buffer swizzle specialization", TestBufferSwizzleSpecialization);
-    Run("native scalar atomic data", TestNativeScalarAtomicDataDoesNotEnterResourcePlan);
    Run("conditional buffer materialization", TestConditionalBufferMaterialization);
    Run("guarded scalar descriptor reads", TestGuardedScalarDescriptorReads);
    Run("conservative buffer reachability", TestConservativeBufferReachability);
@@ -27021,18 +27021,15 @@ TestCase ScalarMemoryLoadVariants() {
  std::vector<u32> expected = initial;
  expected.insert(expected.end(), initial.begin(), initial.end());

-  TestCase test{"ScalarMemoryLoadVariants",
-                code,
-                initial,
-                expected,
-                {O::S_MOV_B32, O::S_LOAD_DWORD, O::S_LOAD_DWORDX2, O::S_LOAD_DWORDX4,
-                 O::S_LOAD_DWORDX8, O::S_LOAD_DWORDX16, O::S_BUFFER_LOAD_DWORD,
-                 O::S_BUFFER_LOAD_DWORDX2, O::S_BUFFER_LOAD_DWORDX4,
-                 O::S_BUFFER_LOAD_DWORDX8, O::S_BUFFER_LOAD_DWORDX16, O::V_MOV_B32,
-                 O::BUFFER_STORE_DWORD, O::S_ENDPGM}};
-  test.bda_mappings = {{0, 0}};
-  test.required_spirv = {"get_bda_pointer"};
-  return test;
+  return {"ScalarMemoryLoadVariants",
+          code,
+          initial,
+          expected,
+          {O::S_MOV_B32, O::S_LOAD_DWORD, O::S_LOAD_DWORDX2, O::S_LOAD_DWORDX4,
+           O::S_LOAD_DWORDX8, O::S_LOAD_DWORDX16, O::S_BUFFER_LOAD_DWORD,
+           O::S_BUFFER_LOAD_DWORDX2, O::S_BUFFER_LOAD_DWORDX4,
+           O::S_BUFFER_LOAD_DWORDX8, O::S_BUFFER_LOAD_DWORDX16, O::V_MOV_B32,
+           O::BUFFER_STORE_DWORD, O::S_ENDPGM}};
 }

 TestCase ScalarBufferOffsetAlignmentAndCarry() {
@@ -27076,15 +27073,11 @@ TestCase ScalarLoadSignedImmediateOffsetAddsSoffset() {
  AppendStoreSgpr(&code, 1, 0);
  AppendEnd(&code);

-  TestCase test{"ScalarLoadSignedImmediateOffsetAddsSoffset",
-                code,
-                {0x11111111u, 0x22222222u},
-                {0x22222222u, 0x22222222u},
-                {O::S_MOV_B32, O::S_LOAD_DWORD, O::BUFFER_STORE_DWORD, O::S_ENDPGM}};
-  test.bda_mappings = {{0, 0}};
-  test.required_spirv = {"get_bda_pointer"};
-  test.forbidden_spirv = {"flattened_srt"};
-  return test;
+  return {"ScalarLoadSignedImmediateOffsetAddsSoffset",
+          code,
+          {0x11111111u, 0x22222222u},
+          {0x22222222u, 0x22222222u},
+          {O::S_MOV_B32, O::S_LOAD_DWORD, O::BUFFER_STORE_DWORD, O::S_ENDPGM}};
 }

 TestCase BufferLoadStore() {
@@ -29458,27 +29451,23 @@ TestCase BranchVccnzUsesCarryProducedWaveMask() {
  return test;
 }

-TestCase ScalarLoadAlignsComponents() {
+TestCase ScalarLoadAlignsComponentsAndMasksAddress() {
  using O = ShaderOpcode;

  std::vector<u32> code;
  AppendSMovLiteral(&code, 0, 1u);
  AppendSMovLiteral(&code, 2, 3u);
-  AppendSMovLiteral(&code, 3, 0u);
+  AppendSMovLiteral(&code, 3, 0xffff0000u);
  code.push_back(EncodeSmem0(0x00, 1, 1));
  code.push_back(EncodeSmem1(3, 0));
  AppendStoreSgpr(&code, 1, 0);
  AppendEnd(&code);

-  TestCase test{"ScalarLoadAlignsComponents",
-                code,
-                {0x11111111u, 0x22222222u},
-                {0x11111111u, 0x22222222u},
-                {O::S_MOV_B32, O::S_LOAD_DWORD, O::BUFFER_STORE_DWORD, O::S_ENDPGM}};
-  test.bda_mappings = {{0, 0}};
-  test.required_spirv = {"get_bda_pointer"};
-  test.forbidden_spirv = {"flattened_srt"};
-  return test;
+  return {"ScalarLoadAlignsComponentsAndMasksAddress",
+          code,
+          {0x11111111u, 0x22222222u},
+          {0x11111111u, 0x22222222u},
+          {O::S_MOV_B32, O::S_LOAD_DWORD, O::BUFFER_STORE_DWORD, O::S_ENDPGM}};
 }

 TestCase ScalarLoadAlignsDynamicBase() {
@@ -34881,7 +34870,7 @@ std::vector<TestCase> MakeCases() {
  AddCase(ScalarMemoryLoadVariants);
  AddCase(ScalarBufferOffsetAlignmentAndCarry);
  AddCase(ScalarLoadSignedImmediateOffsetAddsSoffset);
-  AddCase(ScalarLoadAlignsComponents);
+  AddCase(ScalarLoadAlignsComponentsAndMasksAddress);
  AddCase(ScalarLoadAlignsDynamicBase);
  cases.push_back(ScalarBufferFromLoopReadlane(32));
  cases.push_back(ScalarBufferFromLoopReadlane(64));
@@ -37792,19 +37781,17 @@ void CheckResourcePlanHandoff() {
    ValidateSpirv(name, compiled.spirv);
    const auto &bindings = compiled.program.bindings;
    Require(name, "GPU data requirements",
-            bindings.user_data_registers == (numeric_read ? std::vector<u32>{0, 1}
-                                                          : std::vector<u32>{}) &&
-                bindings.memory_offset_count == 1 &&
-                FindBinding(bindings, DescriptorBindingKind::FlattenedSrt) == nullptr &&
-                (FindBinding(bindings, DescriptorBindingKind::BdaPagetable) != nullptr) == numeric_read,
-            "descriptor-only shader retained SRT uploads or native scalar data lost its address");
+            bindings.user_data_registers.empty() && bindings.memory_offset_count == 1 &&
+                (FindBinding(bindings, DescriptorBindingKind::FlattenedSrt) != nullptr) ==
+                    numeric_read,
+            "descriptor-only shader retained SRT uploads or live scalar data was removed");

    memory[0] = 0x2000u;
    memory[4] = 13u;
    Require(name, "independent host plan",
            MaterializeResources(plan, runtime, snapshot, specialization) &&
                snapshot.buffers[0].dwords[0] == 0x2000u &&
-                snapshot.flattened_srt.size() == 4,
+                (!numeric_read || snapshot.flattened_srt.back() == 13u),
            "compiled shader cleanup invalidated or froze the host resource plan");
  }
  std::printf("[host]    %-32s ok\n", name);