<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">Hi Tim,<div class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Jun 22, 2016, at 9:16 AM, Tim Northover <<a href="mailto:t.p.northover@gmail.com" class="">t.p.northover@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div class="">Hi Quentin,<br class=""><br class="">On 21 June 2016 at 11:58, Quentin Colombet <<a href="mailto:qcolombet@apple.com" class="">qcolombet@apple.com</a>> wrote:<br class=""><blockquote type="cite" class="">I believe this is work in progress so I did not go into great details to comment on each part of the patch. For instance, for a detailed review, I would have expected the patch to be split into smaller one.<br class=""></blockquote><br class="">Sure, I'll try to look for smaller independently testable bits.<br class=""><br class=""><blockquote type="cite" class="">- I would split the file with the legalization APIs in two:<br class=""> - one for the machine pass<br class=""> - one for the helper<br class=""><br class="">The rationale is to be able to include them independently.<br class=""><br class="">- I would split the LegalizeHelper in two:<br class=""> - One with all the methods you added (narrow, widen, etc), but legalize<br class=""> - One with the legalize API and the set action stuff<br class=""></blockquote><br class="">Yep, sounds compeltely reasonable.<br class=""><br class=""><blockquote type="cite" class="">- Replace the Type/EVT by something lower level (Cf. my email)<br class=""><br class="">Basically, we do not need to know what are the types, we only need to know their size and there number of lanes, i.e., I would go with a different types representation for the MI level.<br class=""></blockquote><br class="">I think we do need to know the the full types (at least MVT<br class="">information) to decide on the correct action. <4 x float> could have<br class="">very different requirements from <4 x i32>. I'm afraid I'm not sure<br class="">what e-mail you're talking about either, do you have a link handy?<br class=""></div></div></blockquote><div><br class=""></div><div>I was talking of the offline discussion in "<span style="color: rgba(0, 0, 0, 0.498039); font-family: 'Helvetica Neue'; font-size: 15px;" class="">GISel - reality check"</span></div><div><br class=""></div><div>That’s just a remark in that email, the basic idea is what I said, I.e., I think we only need the size and number of lanes.</div><div>Let me explain a bit why I think it is sufficient.</div><div><4 x float> and <4 x i32> are <4 x 32-bit> for most operations (load, store, and, shuffle, etc.). The only cases where that make a difference AFAICT are arithmetic operations. However, for those, the type is actually encoded in the opcode, e.g., fadd vs. add (we do not have G_FADD yet but I think we should :)). In other word, when type matters, the opcode should give us the information otherwise we should be able to get away of that business.</div><div><br class=""></div><div>Cheers,</div><div>-Quentin</div><br class=""><blockquote type="cite" class=""><div class=""><div class=""><br class=""><blockquote type="cite" class="">- Add unit test cases for the legalize helper.<br class=""></blockquote><br class="">Good idea. That'll certainly help split the patch into smaller slices.<br class=""><br class="">Cheers.<br class=""><br class="">Tim.<br class=""></div></div></blockquote></div><br class=""></div></body></html>