<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div><blockquote type="cite"><div lang="EN-US" link="blue" vlink="purple" style="letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div class="WordSection1" style="page: WordSection1;"><br><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">I’m not sure that I follow. The proposed transformation does the same, it creates only one cast. You can see this in the provided test.<o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"></div></div></div></blockquote><div><br></div><div>In the current transformation the old vector cast goes away because its only user is removed. In the new transformation that you are proposing you are adding a new cast instruction, but the old vector cast still stays in place. So, after your transformation we will have one more cast operation.</div><br><blockquote type="cite"><div lang="EN-US" link="blue" vlink="purple" style="letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div class="WordSection1" style="page: WordSection1;"><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><o:p> </o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;">>> I did not read the code but from a quick peek I noticed that the docs are not capitalized and punctuated properly and the variable names are funny, and you left some code commented out and there are extra braces, etc. <o:p></o:p></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 10pt; font-family: Verdana, sans-serif; color: purple;"> </span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 10pt; font-family: Verdana, sans-serif; color: purple;">I made some corrections.<o:p></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: 'Times New Roman', serif;"><span style="font-size: 10pt; font-family: Verdana, sans-serif; color: purple;"> </span></div></div></div></blockquote></div><div><div><br></div><div>+Instruction *InstCombiner::scalarizePHI(ExtractElementInst &EI, PHINode *PN) {</div><div>+ // Verify that it has only 2 uses. If so, it's known at this point that one</div><div>+ // operand is PHI and the other is an extractelement node.</div><div>+ if (PN->hasNUses(2)) {</div><div>+ // Find the PHI user that is not the extractelement node.</div><div><br></div><div>Please use early-exits. There is no need to indent all of the code in the function if we can simply return null. </div><div><br></div><div>+ Value::use_iterator iu = PN->use_begin();</div><div>+ Instruction *PHIUser = dyn_cast<Instruction>(*iu);</div><div>+ if (PHIUser == cast<Instruction>(&EI)) {</div><div>+ PHIUser = cast<Instruction>(*(++iu));</div><div>+ }</div><div><br></div><div>No need for braces around the PHIUser … Also, can you add comment that explains this code ?</div><div><br></div><div><br></div><div>+ // Verify that this PHI user has one use, which is the PHI itself,</div><div>+ // and that it is a binary operation which is cheap to scalarize.</div><div>+ if (PHIUser->hasOneUse() && PHIUser->use_back() == PN &&</div><div>+ (isa<BinaryOperator>(PHIUser)) &&</div><div>+ CheapToScalarize(PHIUser, true)) {</div><div><br></div><div>Early exit. </div><div><br></div><div><br></div><div>Except for these comments it looks good to me. </div><div><br></div></div></body></html>