<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">Hi Eli,<br>
<br>
Thank you so much for your good reviewing and comment. I think
It's not good way to insert "ImplicitCastExpr of LvalueToRValue"
on checkConditionalConvertScalarsToVectors()" and I have no other
idea to fix it. I will commit the patch with a test case like
attached patch.<br>
<br>
Thanks for your good guide again,<br>
JinGu Kang<br>
<br>
2013-09-03 오전 2:28, Eli Friedman 쓴 글:<br>
</div>
<blockquote
cite="mid:CAJdarcEuOYJcLhGrWTgAh136P_T=zA0jtDrwB7qB6FmqeTXvaA@mail.gmail.com"
type="cite">
<div dir="ltr">On Fri, Aug 30, 2013 at 4:19 AM, JinGu Kang <span
dir="ltr"><<a moz-do-not-send="true"
href="mailto:jingu@codeplay.com" target="_blank">jingu@codeplay.com</a>></span>
wrote:<br>
<div class="gmail_extra">
<div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<div text="#000000" bgcolor="#FFFFFF"> Hi Eli,<br>
<br>
I usually appreciate your comment. <br>
<div class="im"> <br>
>Shouldn't the UsualArithmeticConversions call come
after the checkConditionalConvertScalarsToVectors
check? Not completely sure here (the rules aren't
really clear, and clang's implementation of the check
in question isn't consistent with any interpretation
of the OpenCL standard).<br>
<br>
</div>
I also agree your opinion but it causes assertion
"assert(0 && "can't implicitly cast lvalue to
rvalue with this cast kind");" with following OpenCL
example.<br>
<br>
Source Code: <a moz-do-not-send="true"
href="http://test.cl" target="_blank">test.cl</a><br>
typedef char char16
__attribute__((ext_vector_type(16)));<br>
<br>
__kernel void test(void) {<br>
char valA;<br>
char valB;<br>
char16 valC;
<div class="im"><br>
char16 destVal = valC ? valA : valB;<br>
}<br>
<br>
</div>
This assertion comes from "ImpCastExprToType()" function
in "checkConditionalConvertScalarsToVectors()" function
because LHS and RHS are LValue at that moment without
"UsualArithmeticConversions()". In order to move
"UsualArithmeticConversions()" after
"checkConditionalConvertScalarsToVectors ()",
"checkConditionalConvertScalarsToVectors ()" needs
"DefaultLvalueConversion()" to make 'LValueToRValue'
ImplicitCastExpr with LHS and RHS. I think that
"DefaultLvalueConversion()" is redundant because
"UsualArithmeticConversions()" also executes
"DefaultLvalueConversion()". How do you feel about
this?</div>
</blockquote>
<div><br>
</div>
<div>If you feel like you don't know how to fix
<div bgcolor="#FFFFFF">checkConditionalConvertScalarsToVectors,
feel free to leave it as-is.<br>
</div>
</div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<div text="#000000" bgcolor="#FFFFFF">
<div class="im"><br>
<br>
>If you're going to go the route of updating
CondTy, LHSTy, and RHSTy whenever the expressions
change, please make sure they stay consistently
up-to-date throughout the function.<br>
<br>
</div>
I am so sorry that I can not understand your comment
completely due to my idiot brain. I simply modified
patch to use "getType()" function. If there are
something wrong, please guide me to correct direction.
:)<br>
<br>
</div>
</blockquote>
<div><br>
</div>
<div>I was referring specifically to places which update <span
class="">LHS and RHS without updating RHSTy and LHSTy.
Taking another look, the only offender is actually</span><span
class=""> <span class=""></span><span class="">checkConditionalConvertScalarsToVectors</span><span
class="">, so feel free to ignore that.<br>
<br>
<br>
</span></span></div>
<div><span class=""><span class="">One more review comment I
forgot to note the first time around: the patch is
missing a testcase.<br>
<br>
</span></span></div>
<div><span class=""><span class="">-Eli<br>
</span></span></div>
</div>
</div>
</div>
</blockquote>
<br>
</body>
</html>