Commit 55644211 by Olli Etuaho Committed by Commit Bot

Fix constant folding right shift corner cases

Right-shifting the minimum signed integer needs to be handled as a special case, since it can't go through the usual path that clears the sign bit. Code for right-shifting by zero also had a typo that resulted in setting the wrong value to the result. BUG=chromium:662706 TEST=angle_unittests Change-Id: Ief24d738064906a72212242e0917ce30e45d6b25 Reviewed-on: https://chromium-review.googlesource.com/408158Reviewed-by: 's avatarCorentin Wallez <cwallez@chromium.org> Reviewed-by: 's avatarJamie Madill <jmadill@chromium.org> Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
parent e5c53e3e
...@@ -423,27 +423,42 @@ TConstantUnion TConstantUnion::rshift(const TConstantUnion &lhs, ...@@ -423,27 +423,42 @@ TConstantUnion TConstantUnion::rshift(const TConstantUnion &lhs,
// ESSL 3.00.6 section 5.9: "If E1 is a signed integer, the right-shift will extend // ESSL 3.00.6 section 5.9: "If E1 is a signed integer, the right-shift will extend
// the sign bit." In C++ shifting negative integers is undefined, so we implement // the sign bit." In C++ shifting negative integers is undefined, so we implement
// extending the sign bit manually. // extending the sign bit manually.
bool extendSignBit = false;
int lhsSafe = lhs.iConst; int lhsSafe = lhs.iConst;
if (lhsSafe < 0) if (lhsSafe == std::numeric_limits<int>::min())
{ {
extendSignBit = true; // The min integer needs special treatment because only bit it has set is the
// Clear the sign bit so that bitshift right is defined in C++. // sign bit, which we clear later to implement safe right shift of negative
lhsSafe &= 0x7fffffff; // numbers.
ASSERT(lhsSafe > 0); lhsSafe = -0x40000000;
--shiftOffset;
} }
returnValue.setIConst(lhsSafe >> shiftOffset); if (shiftOffset > 0)
{
// Manually fill in the extended sign bit if necessary. bool extendSignBit = false;
if (extendSignBit) if (lhsSafe < 0)
{
extendSignBit = true;
// Clear the sign bit so that bitshift right is defined in C++.
lhsSafe &= 0x7fffffff;
ASSERT(lhsSafe > 0);
}
returnValue.setIConst(lhsSafe >> shiftOffset);
// Manually fill in the extended sign bit if necessary.
if (extendSignBit)
{
int extendedSignBit = static_cast<int>(0xffffffffu << (31 - shiftOffset));
returnValue.setIConst(returnValue.getIConst() | extendedSignBit);
}
}
else
{ {
int extendedSignBit = static_cast<int>(0xffffffffu << (31 - shiftOffset)); returnValue.setIConst(lhsSafe);
returnValue.setIConst(returnValue.getIConst() | extendedSignBit);
} }
} }
else else
{ {
returnValue.setIConst(rhs.iConst); returnValue.setIConst(lhs.iConst);
} }
break; break;
} }
......
...@@ -1026,3 +1026,39 @@ TEST_F(ConstantFoldingTest, FoldMinimumSignedIntegerNegation) ...@@ -1026,3 +1026,39 @@ TEST_F(ConstantFoldingTest, FoldMinimumSignedIntegerNegation)
// Negating the minimum signed integer overflows the positive range, so it wraps back to itself. // Negating the minimum signed integer overflows the positive range, so it wraps back to itself.
ASSERT_TRUE(constantFoundInAST(-0x7fffffff - 1)); ASSERT_TRUE(constantFoundInAST(-0x7fffffff - 1));
} }
// Test that folding of shifting the minimum representable integer works.
TEST_F(ConstantFoldingTest, FoldMinimumSignedIntegerRightShift)
{
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"
" int j = (0x80000000 >> 7);\n"
" my_FragColor = vec4(i, j, i, j);\n"
"}\n";
compile(shaderString);
ASSERT_TRUE(constantFoundInAST(-0x40000000));
ASSERT_TRUE(constantFoundInAST(-0x01000000));
}
// Test that folding of shifting by 0 works.
TEST_F(ConstantFoldingTest, FoldShiftByZero)
{
const std::string &shaderString =
"#version 300 es\n"
"precision mediump float;\n"
"out vec4 my_FragColor;\n"
"void main()\n"
"{\n"
" int i = (3 >> 0);\n"
" int j = (73 << 0);\n"
" my_FragColor = vec4(i, j, i, j);\n"
"}\n";
compile(shaderString);
ASSERT_TRUE(constantFoundInAST(3));
ASSERT_TRUE(constantFoundInAST(73));
}
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