<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div>Please add a message string to your asserts. Except for that, LGTM! </div><div><br></div><br><div><div>On Jul 26, 2013, at 10:48 AM, Tom Stellard <<a href="mailto:tom@stellard.net">tom@stellard.net</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">Hi,<br><br>The attached patch ensures that the SelectionDAG always uses MVT::i32<br>for the index operands of EXTRACT_VECTOR_ELT, INSERT_VECTOR_ELT,<br>INSERT_SUBVECTOR, and EXTRACT_SUBVECTOR. MVT::i32 was chosen because<br>the LLVM IR specification uses i32 for all its vector instructions that<br>take an index arguement.<br><br>The previous code was using TargetLowering::getPointerTy() for vector<br>indices. I think the reason for this was to make it easier to handle<br>non-constant vector indices, which required that the vector be<br>stored in memory.<br><br>Using TargetLowering::getPointerTy() can be a problem for targets with<br>pointer sizes that differ across address spaces, since the address space<br>used to store the vector may have a different pointer size than the<br>'zero' address space which is what TargetLowering::getPointerTy() uses.<br><br>There are two motivations for this patch, the first is to clean<br>up some bad code that is generated when using dynamic indexing<br>of vectors in the R600 target. The second is to reduce the use of<br>TargetLowering::getPointerTy() in the SelectionDAG code which will help<br>anyone who is trying to improve support for targets with pointer sizes<br>that differ across address spaces.<br><br>Please Review.<br><br>Thanks,<br>Tom<br><br><br><span><SelectionDAG-Use-MVT-i32-for-vector-index.patch></span></div></blockquote></div><br></body></html>