<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">Hi Visa,<div><br></div><div>All the occurrences in widening should use <span class="Apple-style-span" style="color: rgb(84, 0, 0); ">isTypeLegal should be updated to use </span><span class="Apple-style-span" style="color: rgb(84, 0, 0); ">isTypeSynthesizable but I can do that update if you prefer after this patch is in.</span></div><div><span class="Apple-style-span" style="color: rgb(84, 0, 0); "><br></span></div><div><span class="Apple-style-span" style="color: rgb(84, 0, 0); ">Looking at the changes, it overall looks good. The code now separates the code that breaks the vector into synthesizable types with where it recombines it. I think we can simplify the code during the recombine step</span></div><div><span class="Apple-style-span" style="color: rgb(84, 0, 0); "><div> while (true) {</div><div> bool wrongTypeInConcatOps = false;</div><div> for (Idx = ConcatEnd - 1; Idx >= 0; Idx--) {</div><div> if (ConcatOps[Idx].getValueType() != MaxVT) {</div><div> wrongTypeInConcatOps = true;</div><div> break;</div><div> }</div><div> }</div><div> if(!wrongTypeInConcatOps)</div><div> break;</div><div><font class="Apple-style-span" color="#000000"><font class="Apple-style-span" color="#540000"> ...</font></font></div><div><font class="Apple-style-span" color="#000000"><font class="Apple-style-span" color="#540000"> }</font></font></div><div><font class="Apple-style-span" color="#000000"><font class="Apple-style-span" color="#540000"><br></font></font></div><div><font class="Apple-style-span" color="#000000"><font class="Apple-style-span" color="#540000">When we split the vector into </font></font>synthesizable types, we go form largest to smallest. If the last piece is the list of operations we want to concatenate is MaxVT, I believe every other piece must be MaxVT so we don't need the loop. I think we can get rid of the for loop and we can move the condition into the while. </div><div><br></div><div>-- Mon Ping</div><div><br></div></span></div><div><br></div><div><div><div><br></div><div><br><div><div>On Jun 9, 2010, at 9:31 AM, Mon Ping Wang wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div>I'll take a look as well.<br><br> -- Mon Ping<br><br>On Jun 8, 2010, at 11:32 PM, Duncan Sands wrote:<br><br><blockquote type="cite">Hi Dale,<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite">Thanks for the advice. I changed all three occurances of isTypeLegal to<br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite">isTypeSynthesizable in DAGTypeLegalizer::WidenVecRes_Binary. There are<br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite">still six other isTypeLegal call sites in LegalizeVectorTypes.cpp that<br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite">probably should be updated, but I didn't touch those for now. The<br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite">updated patch is attached.<br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">It looks OK to me but I am not expert in this code. Duncan, you wrote<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">this didn't you? Can you look?<br></blockquote></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">Mon Ping wrote the vector widening stuff. I will try to take a look too.<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">Ciao,<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">Duncan.<br></blockquote><br></div></blockquote></div><br></div></div></div></body></html>