From e5455648879bf1177235f80375b7c41faa8fdf23 Mon Sep 17 00:00:00 2001
From: greg-lunarg <greg@lunarg.com>
Date: Fri, 12 Oct 2018 06:46:35 -0600
Subject: [PATCH] Consider atomics that load when analyzing live stores in ADCE
 (#1956) (#1958)

Consider atomics that load when analyzing live stores in ADCE.

Previously it asserted that the base of an OpImageTexelPointer should
be an image. It is actually a pointer to an image, so IsValidBasePointer
should suffice.
---
 source/opcode.cpp                             | 9 ++++++---
 source/opcode.h                               | 4 ++++
 source/opt/aggressive_dead_code_elim_pass.cpp | 2 +-
 source/opt/instruction.cpp                    | 1 +
 source/opt/instruction.h                      | 8 ++++++++
 source/opt/mem_pass.cpp                       | 2 +-
 6 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/source/opcode.cpp b/source/opcode.cpp
index 7c5fbb15..69c9561d 100644
--- a/source/opcode.cpp
+++ b/source/opcode.cpp
@@ -391,10 +391,9 @@ bool spvOpcodeIsBranch(SpvOp opcode) {
   }
 }
 
-bool spvOpcodeIsAtomicOp(const SpvOp opcode) {
+bool spvOpcodeIsAtomicWithLoad(const SpvOp opcode) {
   switch (opcode) {
     case SpvOpAtomicLoad:
-    case SpvOpAtomicStore:
     case SpvOpAtomicExchange:
     case SpvOpAtomicCompareExchange:
     case SpvOpAtomicCompareExchangeWeak:
@@ -410,13 +409,17 @@ bool spvOpcodeIsAtomicOp(const SpvOp opcode) {
     case SpvOpAtomicOr:
     case SpvOpAtomicXor:
     case SpvOpAtomicFlagTestAndSet:
-    case SpvOpAtomicFlagClear:
       return true;
     default:
       return false;
   }
 }
 
+bool spvOpcodeIsAtomicOp(const SpvOp opcode) {
+  return (spvOpcodeIsAtomicWithLoad(opcode) || opcode == SpvOpAtomicStore ||
+          opcode == SpvOpAtomicFlagClear);
+}
+
 bool spvOpcodeIsReturn(SpvOp opcode) {
   switch (opcode) {
     case SpvOpReturn:
diff --git a/source/opcode.h b/source/opcode.h
index 5643a64c..2bbd4177 100644
--- a/source/opcode.h
+++ b/source/opcode.h
@@ -100,6 +100,10 @@ bool spvOpcodeIsDecoration(const SpvOp opcode);
 // function only considers core instructions.
 bool spvOpcodeIsLoad(const SpvOp opcode);
 
+// Returns true if the opcode is an atomic operation that uses the original
+// value.
+bool spvOpcodeIsAtomicWithLoad(const SpvOp opcode);
+
 // Returns true if the opcode is an atomic operation.
 bool spvOpcodeIsAtomicOp(const SpvOp opcode);
 
diff --git a/source/opt/aggressive_dead_code_elim_pass.cpp b/source/opt/aggressive_dead_code_elim_pass.cpp
index 0d98c708..9b8cfe6e 100644
--- a/source/opt/aggressive_dead_code_elim_pass.cpp
+++ b/source/opt/aggressive_dead_code_elim_pass.cpp
@@ -439,7 +439,7 @@ bool AggressiveDCEPass::AggressiveDCE(Function* func) {
       AddBreaksAndContinuesToWorklist(mergeInst);
     }
     // If local load, add all variable's stores if variable not already live
-    if (liveInst->opcode() == SpvOpLoad) {
+    if (liveInst->opcode() == SpvOpLoad || liveInst->IsAtomicWithLoad()) {
       uint32_t varId;
       (void)GetPtr(liveInst, &varId);
       if (varId != 0) {
diff --git a/source/opt/instruction.cpp b/source/opt/instruction.cpp
index 3bbc1d46..3a0594ef 100644
--- a/source/opt/instruction.cpp
+++ b/source/opt/instruction.cpp
@@ -187,6 +187,7 @@ Instruction* Instruction::GetBaseAddress() const {
     case SpvOpStore:
     case SpvOpAccessChain:
     case SpvOpInBoundsAccessChain:
+    case SpvOpImageTexelPointer:
     case SpvOpCopyObject:
       // A load or store through a pointer.
       assert(base_inst->IsValidBasePointer() &&
diff --git a/source/opt/instruction.h b/source/opt/instruction.h
index f323d4a1..6a9d9546 100644
--- a/source/opt/instruction.h
+++ b/source/opt/instruction.h
@@ -349,6 +349,10 @@ class Instruction : public utils::IntrusiveNodeBase<Instruction> {
   // uniform buffer.
   bool IsVulkanUniformBuffer() const;
 
+  // Returns true if the instruction is an atom operation that uses original
+  // value.
+  inline bool IsAtomicWithLoad() const;
+
   // Returns true if the instruction is an atom operation.
   inline bool IsAtomicOp() const;
 
@@ -728,6 +732,10 @@ bool Instruction::IsDecoration() const {
 
 bool Instruction::IsLoad() const { return spvOpcodeIsLoad(opcode()); }
 
+bool Instruction::IsAtomicWithLoad() const {
+  return spvOpcodeIsAtomicWithLoad(opcode());
+}
+
 bool Instruction::IsAtomicOp() const { return spvOpcodeIsAtomicOp(opcode()); }
 
 bool Instruction::IsConstant() const {
diff --git a/source/opt/mem_pass.cpp b/source/opt/mem_pass.cpp
index c65e0493..cc097676 100644
--- a/source/opt/mem_pass.cpp
+++ b/source/opt/mem_pass.cpp
@@ -119,7 +119,7 @@ Instruction* MemPass::GetPtr(uint32_t ptrId, uint32_t* varId) {
 
 Instruction* MemPass::GetPtr(Instruction* ip, uint32_t* varId) {
   assert(ip->opcode() == SpvOpStore || ip->opcode() == SpvOpLoad ||
-         ip->opcode() == SpvOpImageTexelPointer);
+         ip->opcode() == SpvOpImageTexelPointer || ip->IsAtomicWithLoad());
 
   // All of these opcode place the pointer in position 0.
   const uint32_t ptrId = ip->GetSingleWordInOperand(0);