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.
This commit is contained in:
Andrew Woloszyn 2015-09-18 16:12:03 -04:00
parent b9cd3996c7
commit 2d83ab2f57
2 changed files with 16 additions and 6 deletions

View File

@ -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

View File

@ -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.