<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; "><br><div><div>On Aug 1, 2012, at 11:18 AM, Chad Rosier <<a href="mailto:mcrosier@apple.com">mcrosier@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">Jush,<div>In the case that the shift amount is zero or greater then the width of the value type, should we just fall back to selection DAG isel to ensure we have consistent behavior (in the event that selection DAG isel changes)? Perhaps someone else could comment on this.</div></div></blockquote><div><br></div><div><br></div>This seems reasonable to me.</div><div><br></div><div>Additional minor thing:</div><div><br></div><div><div>+ if (isa<ConstantInt>(Src2Value)) {</div><div>+ Opc = ARM::MOVsi;</div><div>+ const ConstantInt *CI = cast<ConstantInt>(Src2Value);</div></div><div><br></div><div>This can be simplified to:</div><div>if (const ConstantInt *CI = dyn_cast<ConstantInt>(Src2Value)) {</div><div> Opc = ARM::MOVsi;</div><div><br></div><div><br></div><div>-Jim</div><div><br><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><div><br></div><div>My only other suggestion would be to simplify the test cases. For example,</div><div><br></div><div><div>define i32 @shl() nounwind ssp {</div><div>entry:</div><div> %shl = shl i32 -1, 2</div><div> ret i32 %shl</div><div>}</div><div><br></div><div>define i32 @shl_reg(i32 %src1, i32 %src2) nounwind ssp {</div><div>entry:</div><div> %shl = shl i32 %src1, %src2</div><div> ret i32 %shl</div><div>}</div><div><br></div><div>define i32 @lshr() nounwind ssp {</div><div>entry:</div><div> %lshr = lshr i32 -1, 2</div><div> ret i32 %lshr</div><div>}</div><div><br></div><div>define i32 @lshr_reg(i32 %src1, i32 %src2) nounwind ssp {</div><div>entry:</div><div> %lshr = lshr i32 %src1, %src2</div><div> ret i32 %lshr</div><div>}</div><div><br></div><div>define i32 @ashr() nounwind ssp {</div><div>entry:</div><div> %ashr = ashr i32 -1, 2</div><div> ret i32 %ashr</div><div>}</div><div><br></div><div>define i32 @ashr_reg(i32 %src1, i32 %src2) nounwind ssp {</div><div>entry:</div><div> %ashr = ashr i32 %src1, %src2</div><div> ret i32 %ashr</div><div>}</div></div><div><br></div><div><div>Basically, I ran mem2reg on your test cases… :) </div><div><br></div><div>While I realize this isn't the type of code fast-isel is going to generate, this is much more concise in terms of what you're trying to test (and that's all we care about).</div><div><br></div><div>Please apply after updating the test cases. If anyone disagrees with the handing of the zero/32 shift we can handle that as a post commit; I'm ok with leaving as is..</div><div><br></div><div> Chad</div><div><br></div><div><br><div><div>On Jul 31, 2012, at 11:05 PM, Jush Lu wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><b id="internal-source-marker_0.7258964451029897" style="font-family:'LiHei Pro';font-size:medium;font-weight:normal"><span style="font-size:15px;font-family:Arial;vertical-align:baseline;white-space:pre-wrap">Hi</span><br>
<span style="font-size:15px;font-family:Arial;vertical-align:baseline;white-space:pre-wrap"></span><br><span style="font-size:15px;font-family:Arial;vertical-align:baseline;white-space:pre-wrap">This patch makes ARMFastISel able to handle shl, lshr, and ashr instructions, please help me review it, thanks.</span><br>
<span style="font-size:15px;font-family:Arial;vertical-align:baseline;white-space:pre-wrap"></span><br><span style="font-size:15px;font-family:Arial;vertical-align:baseline;white-space:pre-wrap">Jush</span></b>
<span><fast-isel-shifter.patch></span></blockquote></div><br></div></div></div></div>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits<br></blockquote></div><br></body></html>