<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">Oh yeah, thanks. Forgot about that one.<div><br></div><div>I'll commit it now.</div><div><br></div><div>Thanks,</div><div>Pete</div><div><br><div><div>On Nov 2, 2011, at 11:47 AM, Evan Cheng 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; ">Looks fine to me. One stylistic nitpick:<div><br></div><div>}</div><div>else if (OR == ...))</div><div>{</div><div><br></div><div>should be</div><div>} else if (OR == ...)) {</div><div><br></div><div>Evan</div><div><br></div><div><div><div>On Nov 1, 2011, at 5:14 PM, Peter Cooper 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; ">Please take a look at the following patch. I've left in the magic 16 for now, but i'd like to make sure the rest is ok.<div><br></div><div>Thanks,</div><div>Pete</div><div></div></div>
<span><dse.patch></span><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div><br><div><div>On Nov 1, 2011, at 4:39 PM, Peter Cooper 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; ">I agree with you Evan if you consider the magic 16 constant which really does need TargetData, but i could remove that for now. The result would be the following 2 conditions allowing the optimization to take place:<div><br></div><div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 11px/normal Menlo; "><span style="color: #508187">llvm</span>::<span style="color: #32595d">isPowerOf2_64</span>(InstWriteOffset)</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 11px/normal Menlo; "><br></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 11px/normal Menlo; ">which means that the earlier store is going to be trimmed to a power of 2 in which case any instructions over the power of 2 boundary were likely not vector instructions anyway, or if they were then we're replacing all those vector instructions with probably more vector instructions which is good, and</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 11px/normal Menlo; "><br></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 11px/normal Menlo; ">((DepWriteAlign != <span style="color: #2038d6">0</span>) && InstWriteOffset % DepWriteAlign == <span style="color: #2038d6">0)</span></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 11px/normal Menlo; "><br></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 11px/normal Menlo; ">which says that the later store is at an offset at the same alignment as the earlier store. If the alignment was < 16 in this case then we'd probably not generate vector instructions for the earlier stores anyway so trimming the earlier store should be ok.</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 11px/normal Menlo; "><br></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 11px/normal Menlo; ">Pete</div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 11px/normal Menlo; "><br></div><div style="margin-top: 0px; margin-right: 0px; margin-bottom: 0px; margin-left: 0px; font: normal normal normal 11px/normal Menlo; "><br></div><div><div>On Nov 1, 2011, at 4:22 PM, Evan Cheng wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div>This might be something we have to defer until we add "what's native types" to TargetData. I'm worried when / if this does something bad, the performance impact can be very significant.<br><br>Evan<br><br>On Nov 1, 2011, at 3:12 PM, Eli Friedman wrote:<br><br><blockquote type="cite">On Tue, Nov 1, 2011 at 2:47 PM, Eric Christopher <<a href="mailto:echristo@apple.com">echristo@apple.com</a>> wrote:<br></blockquote><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">On Oct 31, 2011, at 5:38 PM, Peter Cooper wrote:<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite">Hi<br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite">Please review this patch to allow DSE to trim stores as opposed to deleting them.<br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite">The logic here is that if the end of the earlier store is dead because of a later store then the length of the earlier store will be trimmed in size to avoid writing dead memory. The only time i won't do this is if the original store was likely to use vector writes which if shortened would end up as multiple scalar writes and so is less efficient.<br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">Not a huge fan of this style:<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">+ (OR = isOverwrite(Loc, DepLoc, *AA,<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">+ DepWriteOffset,<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">+ InstWriteOffset)) != OverwriteUnknown &&<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">in large conditionals. Things that return booleans, or set something for a block, e.g.:<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">if (ConstantInt *CI = dyn_cast<ConstantInt>(Inst)) {<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">}<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><blockquote type="cite">Any help removing the magic vector size (16) constant would be good too :)<br></blockquote></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">Something like this maybe?<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">bool isLegalVector = false;<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">if (VectorType *VecTy = dyn_cast<VectorType>(Store->getType()) {<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"> EVT VT = TLI.getValueType(VecTy);<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"> isLegalVector = TLI.isTypeLegal(VT);<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">}<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">does require target info though and I'm not sure how kosher that is in AA.<br></blockquote></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">There aren't actually any vector types involved here; the issue is<br></blockquote><blockquote type="cite">that, for example, a 32 byte memset is cheaper than a 31-byte memset<br></blockquote><blockquote type="cite">under the default settings on x86-64. I'm not sure what the right<br></blockquote><blockquote type="cite">approach is here.<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">-Eli<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">_______________________________________________<br></blockquote><blockquote type="cite">llvm-commits mailing list<br></blockquote><blockquote type="cite"><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br></blockquote><blockquote type="cite"><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br></blockquote><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br></div></blockquote></div><br></div></div>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br></blockquote></div><br></div></div>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br></blockquote></div><br></div></div></blockquote></div><br></div></body></html>