rjmccall added inline comments.
================
Comment at: lib/CodeGen/CGExprScalar.cpp:2716
@@ +2715,3 @@
+ if (Ops.LHS->getType() != RHS->getType()) {
+ bool isSigned =
dyn_cast<BinaryOperator>(Ops.E)->getRHS()->getType().getTypePtr()->isSignedIntegerType();
+ RHS = Builder.CreateIntCast(RHS, Ops.LHS->getType(), isSigned, "sh_prom");
----------------
ichesnokov wrote:
> The OpenCL 2.0 standard does not say anything about right operand sign of
> shift operators.
>
> C standard says the behavior is undefined:
> http://www.coding-guidelines.com/cbook/cbook1_0b.pdf, 1175
> "If the value of the right operand is negative or is greater than or equal to
> the width of the promoted left
> operand, the behavior is undefined."
>
> In C++11 standard said the same:
> http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n4296.pdf, 5.8
> "The behavior is undefined if the right operand
> is negative, or greater than or equal to the length in bits of the promoted
> left operand"
>
> I.e., we can close this bug as NAB and do not apply patch.
> However, I believe that passing right operand, preserving it's sign, gives
> more flexibility to developers. It makes opportunity to developers use signed
> right operator, not only unsigned (if development is working for specific
> platform where developer knows what happens when right operand is signed).
>
I agree that it would be abstractly better to sign-extend the RHS if it's a
signed type. Please do this for both << and >>, and please make sure your test
covers <<= and >>= as well.
Also, please use cast<> for unconditional casts instead of dyn_cast.
Repository:
rL LLVM
http://reviews.llvm.org/D16460
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
