[PATCH] D16454: [mips][microMIPS] Add CodeGen support for ADD, ADDIU*, ADDU* and DADD* instructions

[Daniel Sanders via llvm-commits]

dsanders added inline comments.

================
Comment at: lib/Target/Mips/Mips64InstrInfo.td:542-545
@@ -534,8 +541,6 @@
               (DSUBu GPR64:$lhs, GPR64:$rhs)>;
-let AdditionalPredicates = [NotDSP] in {
-  def : MipsPat<(addc GPR64:$lhs, GPR64:$rhs),
-                (DADDu GPR64:$lhs, GPR64:$rhs)>;
-  def : MipsPat<(addc GPR64:$lhs, immSExt16:$imm),
-                (DADDiu GPR64:$lhs, imm:$imm)>;
-}
+def : MipsPat<(addc GPR64:$lhs, GPR64:$rhs),
+              (DADDu GPR64:$lhs, GPR64:$rhs)>, ISA_NOT_MICROMIPS_DSP;
+def : MipsPat<(addc GPR64:$lhs, immSExt16:$imm),
+              (DADDiu GPR64:$lhs, imm:$imm)>, ISA_NOT_MICROMIPS_DSP;
 
----------------
zbuljan wrote:
> dsanders wrote:
> > These used to be disabled for any DSP, not just microMIPS DSP. Is there a reason for the change?
> I wanted do disable these for microMIPS also so I introduced new ISA_NOT_MICROMIPS_DSP class with list of predicates: NotInMicroMips and NotDSP.
> Maybe it would be better to name class as ISA_NOT_MICROMIPS_NOT_DSP?
Ah ok, it's just the name that's a bit misleading.

I think the change you want is 
  let AdditionalPredicates = [NotInMicroMips,NotDSP] in {
This is because predicates belong to a specific group of predicates (e.g. EncodingPredicates, InsnPredicates, etc.) and both of these belong to AdditionalPredicates at the moment. That said, it would be easy to change NotDSP into an InsnPredicate since there's only three uses of it, then you could have
  let AdditionalPredicates = [NotInMicroMips] in {
    def : MipsPat<(addc GPR64:$lhs, GPR64:$rhs),
                (DADDu GPR64:$lhs, GPR64:$rhs)>, ASE_NOT_DSP;
which is more consistent with other code.

At some point we need to move NotInMicroMips into the EncodingPredicates group.


http://reviews.llvm.org/D16454