[PATCH] D12279: [mips][microMIPS] Implement ADDIUR1SP, ADDIUR2, ADDIUS5 and ADDIUSP instructions

Daniel Sanders via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 27 04:11:44 PDT 2015


dsanders added a comment.

We seem to be talking about different things.

To summarize:
The problem with http://reviews.llvm.org/D12279 is that there are no tests in the patch. You've told me that the tests were committed in an earlier patch (http://reviews.llvm.org/rL245554). However, those tests pass without needing the implementations in this patch. This begs the question 'Why do we need these implementations?' and your comment that the microMIPS(R3) and microMIPSR6 encodings are identical hint that the answer is "we don't, it's unnecessary duplication". When questioned about this, your answer was that another patch changed the DecoderNamespace and referenced http://reviews.llvm.org/D11181 which doesn't change any DecoderNamespace's.

My position is that, I don't want duplicate instructions without a very good reason. If the microMIPS(R3) instructions are correct, then we should use them to handle microMIPSR6 as well. Don't forget that the architecture predicates are cumulative, so all microMIPS(R3) instructions are enabled for microMIPSR6 too unless you explicitly prevent this like the NotMips32r6 predicate and the ISA_MIPS32R2_NOT_32R6_64R6 adjective do for the standard encodings.


http://reviews.llvm.org/D12279





More information about the llvm-commits mailing list