From 824625760b7c95400205b3a0958cc2690a6aedcf Mon Sep 17 00:00:00 2001
From: Alan Baker <alanbaker@google.com>
Date: Thu, 1 Mar 2018 15:24:41 -0500
Subject: [PATCH] Fixes #1361. Mark all non-constant global values as varying
 in CCP

* Also mark function parameters as varying
* Conservatively mark assignment instructions as varying if any input is
varying after attempting to fold
* Added a test to catch this case
---
 source/opt/ccp_pass.cpp | 19 +++++++++++++++++--
 test/opt/ccp_test.cpp   | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/source/opt/ccp_pass.cpp b/source/opt/ccp_pass.cpp
index 498a668..e9044ad 100644
--- a/source/opt/ccp_pass.cpp
+++ b/source/opt/ccp_pass.cpp
@@ -143,6 +143,15 @@ SSAPropagator::PropStatus CCPPass::VisitAssignment(ir::Instruction* instr) {
     return SSAPropagator::kInteresting;
   }
 
+  // Conservatively mark this instruction as varying if any input id is varying.
+  if (!instr->WhileEachInId([this](uint32_t* op_id) {
+        auto iter = values_.find(*op_id);
+        if (iter != values_.end() && IsVaryingValue(iter->second)) return false;
+        return true;
+      })) {
+    return MarkInstructionVarying(instr);
+  }
+
   // If not, see if there is a least one unknown operand to the instruction.  If
   // so, we might be able to fold it later.
   if (!instr->WhileEachInId([this](uint32_t* op_id) {
@@ -266,6 +275,11 @@ bool CCPPass::ReplaceValues() {
 }
 
 bool CCPPass::PropagateConstants(ir::Function* fp) {
+  // Mark function parameters as varying.
+  fp->ForEachParam([this](const ir::Instruction* inst) {
+    values_[inst->result_id()] = kVaryingSSAId;
+  });
+
   const auto visit_fn = [this](ir::Instruction* instr,
                                ir::BasicBlock** dest_bb) {
     return VisitInstruction(instr, dest_bb);
@@ -290,10 +304,11 @@ void CCPPass::Initialize(ir::IRContext* c) {
   // module.  The values of each OpConstant declaration is the identity
   // assignment (i.e., each constant is its own value).
   for (const auto& inst : get_module()->types_values()) {
-    // Skip specialization constants. Treat undef as varying.
+    // Record compile time constant ids. Treat all other global values as
+    // varying.
     if (inst.IsConstant()) {
       values_[inst.result_id()] = inst.result_id();
-    } else if (inst.opcode() == SpvOpUndef) {
+    } else {
       values_[inst.result_id()] = kVaryingSSAId;
     }
   }
diff --git a/test/opt/ccp_test.cpp b/test/opt/ccp_test.cpp
index 3bc9fb6..47ae9cc 100644
--- a/test/opt/ccp_test.cpp
+++ b/test/opt/ccp_test.cpp
@@ -855,6 +855,41 @@ OpFunctionEnd
 
   SinglePassRunAndMatch<opt::CCPPass>(text, true);
 }
+
+// Test for #1361.
+TEST_F(CCPTest, CompositeConstructOfGlobalValue) {
+  const std::string text = R"(
+; CHECK: [[phi:%\w+]] = OpPhi
+; CHECK-NEXT: OpCompositeExtract {{%\w+}} [[phi]] 0
+OpCapability Shader
+OpMemoryModel Logical GLSL450
+OpEntryPoint Fragment %func "func" %in
+%void = OpTypeVoid
+%int = OpTypeInt 32 1
+%bool = OpTypeBool
+%functy = OpTypeFunction %void
+%ptr_int_Input = OpTypePointer Input %int
+%in = OpVariable %ptr_int_Input Input
+%struct = OpTypeStruct %ptr_int_Input %ptr_int_Input
+%struct_null = OpConstantNull %struct
+%func = OpFunction %void None %functy
+%1 = OpLabel
+OpBranch %2
+%2 = OpLabel
+%phi = OpPhi %struct %struct_null %1 %5 %4
+%extract = OpCompositeExtract %ptr_int_Input %phi 0
+OpLoopMerge %3 %4 None
+OpBranch %4
+%4 = OpLabel
+%5 = OpCompositeConstruct %struct %in %in
+OpBranch %2
+%3 = OpLabel
+OpReturn
+OpFunctionEnd
+)";
+
+  SinglePassRunAndMatch<opt::CCPPass>(text, true);
+}
 #endif
 
 }  // namespace
-- 
2.7.4