[PATCH] D14429: [mips][microMIPS][DSP] Implement PACKRL.PH, PICK.PH, PICK.QB, SHILO, SHILOV and WRDSP instructions

Daniel Sanders via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 6 03:27:12 PST 2015


dsanders requested changes to this revision.
dsanders added a comment.
This revision now requires changes to proceed.

This is almost there but it introduces some dangerous 'let Predicates = ' statements. I'd like you to make an easy change to the existing DSP ASE to avoid it (and bring it into line with our recent work) as described below.


================
Comment at: lib/Target/Mips/MipsDSPInstrInfo.td:1215
@@ -1196,2 +1214,3 @@
 def RDDSP : RDDSP_ENC, RDDSP_DESC;
-def WRDSP : WRDSP_ENC, WRDSP_DESC;
+let Predicates = [HasDSP, NotInMicroMips] in {
+  def WRDSP : WRDSP_ENC, WRDSP_DESC;
----------------
I have to insist that we don't introduce more 'let Predicates = ' statements, they're dangerous since they can easily lose predicates.
Please port the DSP ASE to use the PredicateControl class (it's an easy change*) and use AdditionalPredicates for NotInMicroMips.

*Just add PredicateControl as a base class of DSPInst, set InsnPredicates appropriately in DSPInst, and adapt the existing 'let Predicates =' statements to adjectives or let statements on the sub-lists appropriately.

================
Comment at: lib/Target/Mips/MipsDSPInstrInfo.td:1450
@@ +1449,3 @@
+// Instruction alias.
+let Predicates = [HasDSP, NotInMicroMips] in {
+  def : MipsInstAlias<"wrdsp $rt", (WRDSP GPR32Opnd:$rt, 0x1F), 1>;
----------------
Likewise


http://reviews.llvm.org/D14429





More information about the llvm-commits mailing list