<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><span class="Apple-style-span" style="font-family: monospace; ">Hi David,</span><span class="Apple-style-span" style="font-family: monospace; "><br></span><span class="Apple-style-span" style="font-family: monospace; "><br></span><span class="Apple-style-span" style="font-family: monospace; ">In case you didn't see my reply in llvm-commits. Let me repeat my objections here.</span><div><span class="Apple-style-span" style="font-family: monospace; "><br></span></div><div><span class="Apple-style-span" style="font-family: monospace; ">If I understand correctly, the reason you are pushing for all these extra abstraction features in TableGen language is to one day refactor SSE / AVX. The intention is to support future Intel ISA extensions which may extend AVX to vectors of arbitrary width. Am I understanding the purpose of these patches correctly?</span><span class="Apple-style-span" style="font-family: monospace; "><br></span><span class="Apple-style-span" style="font-family: monospace; "><br></span><span class="Apple-style-span" style="font-family: monospace; ">Unfortunately I strongly disagree with the direction you are taking with this.</span><span class="Apple-style-span" style="font-family: monospace; "><br></span><span class="Apple-style-span" style="font-family: monospace; "><br></span><span class="Apple-style-span" style="font-family: monospace; ">If we are to implement support for future Intel vector extension, we should add these instructions separately. I understand you are concerned about code duplication and would prefer to leverage what's already written. However, our experience has shown the extra levels of abstraction is always high level that makes TableGen debugging harder than it should. Adding more layers, as you are proposing, would just make it unnecessary difficult. I believe it will be counterproductive.</span><span class="Apple-style-span" style="font-family: monospace; "><br></span><span class="Apple-style-span" style="font-family: monospace; "><br></span><span class="Apple-style-span" style="font-family: monospace; ">I have discussed this carefully with Chris and have decided this is not the direction where we want to go with TableGen. We would really appreciate that you would back out these changes.</span><span class="Apple-style-span" style="font-family: monospace; "><br></span><span class="Apple-style-span" style="font-family: monospace; "><br></span><span class="Apple-style-span" style="font-family: monospace; ">Thanks,</span><span class="Apple-style-span" style="font-family: monospace; "><br></span><span class="Apple-style-span" style="font-family: monospace; "><br></span><span class="Apple-style-span" style="font-family: monospace; ">Evan</span><div><font class="Apple-style-span" face="monospace"><br></font><div><div>On Oct 6, 2011, at 12:42 PM, David A. Greene wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div>Jakob Stoklund Olesen <<a href="mailto:jolesen@apple.com">jolesen@apple.com</a>> writes:<br><br><blockquote type="cite">On Oct 6, 2011, at 7:59 AM, David A. Greene wrote:<br></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite"><blockquote type="cite">For example, I want to be able to do this:<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">defm MOVH :<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"> vs1x_fps_binary_vv_node_rmonly<<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"> 0x16, "movh", undef, 0,<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"> // rr<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"> [(undef)],<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"> // rm<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"> [(set DSTREGCLASS:$dst, <br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"> (DSTTYPE (movlhps SRCREGCLASS:$src1,<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"> (DSTTYPE (bitconvert<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"> (v2f64 (scalar_to_vector<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"> (loadf64 addr:$src2))))))))],<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"> // rr Pat<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"> [],<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"> // rm Pat<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"> [[(DSTTYPE (movlhps SRCREGCLASS:$src1, (load addr:$src2))),<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"> (MNEMONIC SRCREGCLASS:$src1, addr:$src2)],<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"> [(INTDSTTYPE (movlhps SRCREGCLASS:$src1, (load addr:$src2))),<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"> (MNEMONIC SRCREGCLASS:$src1, addr:$src2)]]>;<br></blockquote></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">This kind of thing is very hard to read and understand.<br></blockquote><br>What's hard about it? I'm not trying to be agitational here. I'm truly<br>wondering what I can do to make this more understandable.<br><br><blockquote type="cite">ISAs don't change a lot<br></blockquote><br>Well, they do on some common architecutres. :)<br><br><blockquote type="cite">.td files are read lot more than they are written. I actually think<br></blockquote><blockquote type="cite">it is preferable to leave some redundancy in the .td files, it makes<br></blockquote><blockquote type="cite">it much easier to find things.<br></blockquote><br>I had two goals when I designed our AVX implementation. Make it easily<br>extensible to longer vector lengths while keeping it as readable as<br>possible. I am certainly happy to make things more readable and welcome<br>lots of feedback in that area. But the ability to quickly and easily<br>extend the ISA for new vector lengths is critical to us. That drives<br>the generic pattern specification above and the !foreach/!subst stuff<br>you see in MultiPat.td. As I explained in another message, I think a<br>lot of MultiPat.td can be cleaned up with some semi-clever uses of the<br>for proposal.<br><br>I actually find the above easier to read and grasp that the multitude of<br>individual patterns for each different kind of ADD, SUB, etc. because I<br>don't have to go searching to find out how each one is defined. They're<br>all defined exactly the same way.<br><br>But of course, I've been working with this for a long time. I'm glad to<br>be getting some feedback on it now. :)<br><br> -Dave<br>_______________________________________________<br>LLVM Developers mailing list<br><a href="mailto:LLVMdev@cs.uiuc.edu">LLVMdev@cs.uiuc.edu</a> <a href="http://llvm.cs.uiuc.edu">http://llvm.cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a><br></div></blockquote></div><br></div></div></body></html>