Commit d4453573 by Olli Etuaho Committed by Commit Bot

Fix integer division constant folding corner cases

ESSL has undefined behavior when the integer modulus operator is used on negative operands. Generate a warning and fold the result to zero in this case. In case the minimum representable signed integer is divided by -1, the result is defined to be either the maximum or minimum representable value. We choose to fold the calculation to the maximum representable value in this case. BUG=angleproject:1537 TEST=angle_unittests Change-Id: I57fac6b54a3553b7a0f0e36cc6ba0ed59a88eea9 Reviewed-on: https://chromium-review.googlesource.com/390251Reviewed-by: 's avatarCorentin Wallez <cwallez@chromium.org> Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
parent ece7c5a8
...@@ -1204,14 +1204,42 @@ TConstantUnion *TIntermConstantUnion::foldBinary(TOperator op, ...@@ -1204,14 +1204,42 @@ TConstantUnion *TIntermConstantUnion::foldBinary(TOperator op,
} }
else else
{ {
int lhs = leftArray[i].getIConst();
int divisor = rightArray[i].getIConst();
if (op == EOpDiv) if (op == EOpDiv)
{ {
resultArray[i].setIConst(leftArray[i].getIConst() / rightArray[i].getIConst()); // Check for the special case where the minimum representable number is
// divided by -1. If left alone this leads to integer overflow in C++.
// ESSL 3.00.6 section 4.1.3 Integers:
// "However, for the case where the minimum representable value is
// divided by -1, it is allowed to return either the minimum
// representable value or the maximum representable value."
if (lhs == -0x7fffffff - 1 && divisor == -1)
{
resultArray[i].setIConst(0x7fffffff);
}
else
{
resultArray[i].setIConst(lhs / divisor);
}
} }
else else
{ {
ASSERT(op == EOpIMod); ASSERT(op == EOpIMod);
resultArray[i].setIConst(leftArray[i].getIConst() % rightArray[i].getIConst()); if (lhs < 0 || divisor < 0)
{
// ESSL 3.00.6 section 5.9: Results of modulus are undefined when
// either one of the operands is negative.
diagnostics->warning(getLine(),
"Negative modulus operator operand "
"encountered during constant folding",
"%", "");
resultArray[i].setIConst(0);
}
else
{
resultArray[i].setIConst(lhs % divisor);
}
} }
} }
break; break;
......
...@@ -824,3 +824,22 @@ TEST_F(ConstantFoldingTest, FoldBitShiftRightDifferentSignedness) ...@@ -824,3 +824,22 @@ TEST_F(ConstantFoldingTest, FoldBitShiftRightDifferentSignedness)
compile(shaderString); compile(shaderString);
ASSERT_TRUE(constantFoundInAST(0x3u)); ASSERT_TRUE(constantFoundInAST(0x3u));
} }
// Test that dividing the minimum signed integer by -1 works.
// ESSL 3.00.6 section 4.1.3 Integers:
// "However, for the case where the minimum representable value is divided by -1, it is allowed to
// return either the minimum representable value or the maximum representable value."
TEST_F(ConstantFoldingTest, FoldDivideMinimumIntegerByMinusOne)
{
const std::string &shaderString =
"#version 300 es\n"
"precision mediump float;\n"
"out vec4 my_FragColor;\n"
"void main()\n"
"{\n"
" int i = 0x80000000 / (-1);\n"
" my_FragColor = vec4(i);\n"
"}\n";
compile(shaderString);
ASSERT_TRUE(constantFoundInAST(0x7fffffff) || constantFoundInAST(-0x7fffffff - 1));
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment