<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 Oct 7, 2013, at 4:04 PM, Quentin Colombet <<a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><meta http-equiv="Content-Type" content="text/html charset=windows-1252"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">Hi Owen,<div><br></div><div>Thanks for the review.</div><div>See my comments inlined with yours.</div><div><br><div><div>On Oct 7, 2013, at 3:47 PM, Owen Anderson <<a href="mailto:resistor@mac.com">resistor@mac.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><meta http-equiv="Content-Type" content="text/html charset=windows-1252"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><br><div><div>On Oct 7, 2013, at 1:24 PM, Quentin Colombet <<a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><meta http-equiv="Content-Type" content="text/html charset=windows-1252"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">Hi Owen, hi Chandler,<div><br></div><div>Owen, you may be a good match to review this patch, which is SDSel related and not instcombine related (as the subject says… it changes :)).</div></div></blockquote><div><br></div><div>I’m not sure I like the cost model this represents. It seems very x86-centric. For example, some RISCs (PowerPC and AArch64, offhand) can do single-instruction paired-register bitfield extracts, which means that original input sequence was better for them (1 load + 1 extract, instead of 2 loads).</div></div></div></blockquote><div>This shouldn’t happen unless the target specified that it supports paired load (the new target hook that this patch adds), which is false by default.</div><div>In that case, you end up with 1 paired load instead of 1 load + 1 extract.</div><div><br></div><div>Does it make sense?</div></div></div></div></blockquote><div><br></div><div>The criteria for when a target should/should not implement the has-paired-load target hook. How is it different from supporting vector loads?</div><div><br></div><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div><div><blockquote type="cite"><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">Even on x86, you’re probably best off trying to turn this into a <2 x i32> vector load + lane copy rather than splitting the load.</div></blockquote><div>My plan here was to turn this into two split loads, then having another dag combine for merging those loads into a vector load. I though maybe target supporting paired load already do this kind of target specific combine.</div></div></div></div></blockquote><br></div><div>You certainly can go that way, but it sounds challenging to implement. My point was that it seems like you could achieve most of the same benefit with a DAG combine that turned this example into a <2 x float> vector load, with much less concern about the cost model, etc.</div><div><br></div><div>--Owen</div><br></body></html>