From 2d83ab2f57013e9b0281fb6859a74c8bd3b006d4 Mon Sep 17 00:00:00 2001
From: Andrew Woloszyn <awoloszyn@google.com>
Date: Fri, 18 Sep 2015 16:12:03 -0400
Subject: [PATCH] Fixed subtle issue that causes tests to fail in VS2013 in
some configs.
Depending on specific optimization settings VS2013 will sometimes
execute the operands to
new Instruction(builder.getUniqueId(), builder.makeBoolType(), OpPhi)
left-to-right, and sometimes right-to-left. Since makeBoolType can
also call getUniqueId(), the IDs to the OpPhi can sometimes be swapped.
This guarantees an explicit ordering of the Ids so that tests work
reliably.
---
SPIRV/SpvBuilder.cpp | 17 +++++++++++++----
SPIRV/SpvBuilder.h | 5 +++--
2 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/SPIRV/SpvBuilder.cpp b/SPIRV/SpvBuilder.cpp
index 3e2768b66..9d8288a2c 100755
--- a/SPIRV/SpvBuilder.cpp
+++ b/SPIRV/SpvBuilder.cpp
@@ -2304,9 +2304,18 @@ Builder::Loop::Loop(Builder& builder, bool testFirstArg)
merge(new Block(builder.getUniqueId(), *function)),
body(new Block(builder.getUniqueId(), *function)),
testFirst(testFirstArg),
- isFirstIteration(testFirst
- ? nullptr
- : new Instruction(builder.getUniqueId(), builder.makeBoolType(), OpPhi))
- {}
+ isFirstIteration(nullptr)
+{
+ if (!testFirst)
+ {
+// You may be tempted to rewrite this as
+// new Instruction(builder.getUniqueId(), builder.makeBoolType(), OpPhi);
+// This will cause subtle test failures because builder.getUniqueId(),
+// and builder.makeBoolType() can then get run in a compiler-specific
+// order making tests fail for certain configurations.
+ Id instructionId = builder.getUniqueId();
+ isFirstIteration = new Instruction(instructionId, builder.makeBoolType(), OpPhi);
+ }
+}
}; // end spv namespace
diff --git a/SPIRV/SpvBuilder.h b/SPIRV/SpvBuilder.h
index c5637f042..23b9bc2c0 100755
--- a/SPIRV/SpvBuilder.h
+++ b/SPIRV/SpvBuilder.h
@@ -564,8 +564,9 @@ protected:
const bool testFirst;
// When the test executes after the body, this is defined as the phi
// instruction that tells us whether we are on the first iteration of
- // the loop. Otherwise this is null.
- Instruction* const isFirstIteration;
+ // the loop. Otherwise this is null. This is non-const because
+ // it has to be initialized outside of the initializer-list.
+ Instruction* isFirstIteration;
};
// Our loop stack.
--
GitLab