From: Andrew Woloszyn Date: Fri, 18 Sep 2015 20:12:03 +0000 (-0400) Subject: Fixed subtle issue that causes tests to fail in VS2013 in some configs. X-Git-Tag: upstream/0.1~374^2 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=2d83ab2f57013e9b0281fb6859a74c8bd3b006d4;p=platform%2Fupstream%2Fglslang.git 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. --- diff --git a/SPIRV/SpvBuilder.cpp b/SPIRV/SpvBuilder.cpp index 3e2768b..9d8288a 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 c5637f0..23b9bc2 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.