[mips] Implement .set dspr2 directive

[mstojanovic]

Summary

Implement .set dspr2 directive with appropriate feature bits. This directive is a counterpart of -mattr=dspr2 command line option with the exception that it does not influence elf header flags.

[sdardis]

LGTM.

[petarj]

test/MC/Mips/set-nodsp.s

1 ↗

(On Diff #117656)

-mattr=+dsp seems redundant in presence of -mattr=+dspr2

Furthermore, DSP extension requires Rev2 at least, so the test should probable have -mcpu=mips32r2

GCC has a warning like:

"Warning: the `dsp' extension requires MIPS32 revision 2 or greater'"

It seems that LLVM does not do that. Maybe it should?

[sdardis]

test/MC/Mips/set-nodsp.s

1 ↗

(On Diff #117656)

-mattr=+dsp seems redundant in presence of -mattr=+dspr2

Well spotted, though the test would have to be altered to test for the differences between dsp / dspr2.

GCC has a warning like:

"Warning: the `dsp' extension requires MIPS32 revision 2 or greater'"

It seems that LLVM does not do that. Maybe it should?

I believe we should. I think we should be stricter but binutils already accepts code that combination of options so we should at least be as compatible / informative.

Some quick testing shows that gas 2.24 only warns on the first instance of a particular error in the test cases I've tried for mixing directives to change the ISA revision.

I'm leaning towards giving warnings where an ASE is enabled that requires a higher ISA revision by an assembler directive at every occurrence, along with a warning for incoherent command-line options, though that may be a bit verbose. Thoughts?

Adding warnings for incompatible ASE/ISA revisions should be submitted as a separate patch.

[petarj]

test/MC/Mips/set-nodsp.s

1 ↗

(On Diff #117656)

I'm leaning towards giving warnings where an ASE is enabled that requires a higher ISA revision by an assembler directive at every occurrence, along with a warning for incoherent command-line options, though that may be a bit verbose. Thoughts?

I do not have a strong opinion about it, I would probably vote for mimicking what gas does.

[mstojanovic]

Removed -mattr=dsp from set-nodsp.s test.

llvm/trunk/lib/Target/Mips/AsmParser/MipsAsmParser.cpp

bool MipsAsmParser::parseSetFeature(uint64_t Feature) {
 
   switch (Feature) {
   default:
     llvm_unreachable("Unimplemented feature");
   case Mips::FeatureDSP:
     setFeatureBits(Mips::FeatureDSP, "dsp");
     getTargetStreamer().emitDirectiveSetDsp();
     break;
+  case Mips::FeatureDSPR2:
+    setFeatureBits(Mips::FeatureDSPR2, "dspr2");
+    getTargetStreamer().emitDirectiveSetDspr2();
+    break;
   case Mips::FeatureMicroMips:
     setFeatureBits(Mips::FeatureMicroMips, "micromips");
     getTargetStreamer().emitDirectiveSetMicroMips();
     break;
   case Mips::FeatureMips1:
     selectArch("mips1");
     getTargetStreamer().emitDirectiveSetMips1();
     break;
bool MipsAsmParser::parseDirectiveSet() {
   } else if (Tok.getString() == "mips64r3") {
     return parseSetFeature(Mips::FeatureMips64r3);
   } else if (Tok.getString() == "mips64r5") {
     return parseSetFeature(Mips::FeatureMips64r5);
   } else if (Tok.getString() == "mips64r6") {
     return parseSetFeature(Mips::FeatureMips64r6);
   } else if (Tok.getString() == "dsp") {
     return parseSetFeature(Mips::FeatureDSP);
+  } else if (Tok.getString() == "dspr2") {
+    return parseSetFeature(Mips::FeatureDSPR2);
   } else if (Tok.getString() == "nodsp") {
     return parseSetNoDspDirective();
   } else if (Tok.getString() == "msa") {
     return parseSetMsaDirective();
   } else if (Tok.getString() == "nomsa") {
     return parseSetNoMsaDirective();
   } else if (Tok.getString() == "mt") {
     return parseSetMtDirective();

llvm/trunk/lib/Target/Mips/MCTargetDesc/MipsTargetStreamer.cpp

 void MipsTargetStreamer::emitDirectiveSetPush() { forbidModuleDirective(); }
 void MipsTargetStreamer::emitDirectiveSetSoftFloat() {
   forbidModuleDirective();
 }
 void MipsTargetStreamer::emitDirectiveSetHardFloat() {
   forbidModuleDirective();
 }
 void MipsTargetStreamer::emitDirectiveSetDsp() { forbidModuleDirective(); }
+void MipsTargetStreamer::emitDirectiveSetDspr2() { forbidModuleDirective(); }
 void MipsTargetStreamer::emitDirectiveSetNoDsp() { forbidModuleDirective(); }
 void MipsTargetStreamer::emitDirectiveCpLoad(unsigned RegNo) {}
 bool MipsTargetStreamer::emitDirectiveCpRestore(
     int Offset, function_ref<unsigned()> GetATReg, SMLoc IDLoc,
     const MCSubtargetInfo *STI) {
   forbidModuleDirective();
   return true;
 }
void MipsTargetAsmStreamer::emitDirectiveSetMips64R6() {
   MipsTargetStreamer::emitDirectiveSetMips64R6();
 }
 
 void MipsTargetAsmStreamer::emitDirectiveSetDsp() {
   OS << "\t.set\tdsp\n";
   MipsTargetStreamer::emitDirectiveSetDsp();
 }
 
+void MipsTargetAsmStreamer::emitDirectiveSetDspr2() {
+  OS << "\t.set\tdspr2\n";
+  MipsTargetStreamer::emitDirectiveSetDspr2();
+}
+
 void MipsTargetAsmStreamer::emitDirectiveSetNoDsp() {
   OS << "\t.set\tnodsp\n";
   MipsTargetStreamer::emitDirectiveSetNoDsp();
 }
 
 void MipsTargetAsmStreamer::emitDirectiveSetPop() {
   OS << "\t.set\tpop\n";
   MipsTargetStreamer::emitDirectiveSetPop();

llvm/trunk/lib/Target/Mips/MipsTargetStreamer.h

public:
   virtual void emitDirectiveSetMips32R5();
   virtual void emitDirectiveSetMips32R6();
   virtual void emitDirectiveSetMips64();
   virtual void emitDirectiveSetMips64R2();
   virtual void emitDirectiveSetMips64R3();
   virtual void emitDirectiveSetMips64R5();
   virtual void emitDirectiveSetMips64R6();
   virtual void emitDirectiveSetDsp();
+  virtual void emitDirectiveSetDspr2();
   virtual void emitDirectiveSetNoDsp();
   virtual void emitDirectiveSetPop();
   virtual void emitDirectiveSetPush();
   virtual void emitDirectiveSetSoftFloat();
   virtual void emitDirectiveSetHardFloat();
 
   // PIC support
   virtual void emitDirectiveCpLoad(unsigned RegNo);
public:
   void emitDirectiveSetMips32R5() override;
   void emitDirectiveSetMips32R6() override;
   void emitDirectiveSetMips64() override;
   void emitDirectiveSetMips64R2() override;
   void emitDirectiveSetMips64R3() override;
   void emitDirectiveSetMips64R5() override;
   void emitDirectiveSetMips64R6() override;
   void emitDirectiveSetDsp() override;
+  void emitDirectiveSetDspr2() override;
   void emitDirectiveSetNoDsp() override;
   void emitDirectiveSetPop() override;
   void emitDirectiveSetPush() override;
   void emitDirectiveSetSoftFloat() override;
   void emitDirectiveSetHardFloat() override;
 
   // PIC support
   void emitDirectiveCpLoad(unsigned RegNo) override;

llvm/trunk/test/MC/Mips/mips_directives.s

# CHECK: drotr $9, $6, 30              # encoding: [0x00,0x26,0x4f,0xba]
     drotr   $9, $6, 30
 
 # CHECK:   .set dsp
 # CHECK:   lbux    $7, $10($11)         # encoding: [0x7d,0x6a,0x39,0x8a]
 # CHECK:   lhx     $5, $6($7)           # encoding: [0x7c,0xe6,0x29,0x0a]
    .set dsp
    lbux    $7, $10($11)
    lhx     $5, $6($7)
+
+# CHECK:   .set dspr2
+# CHECK:   append   $7, $10, 2          # encoding: [0x7d,0x47,0x10,0x31]
+# CHECK:   balign   $5, $6, 3           # encoding: [0x7c,0xc5,0x1c,0x31]
+   .set dspr2
+   append   $7, $10, 2
+   balign   $5, $6, 3
 No newline at end of file

llvm/trunk/test/MC/Mips/module-directive-bad.s

# CHECK: :[[@LINE-1]]:13: error: .module directive must appear before any code
     .llvm_internal_mips_reallow_module_directive
     .module fp=32
 # CHECK-NOT: :[[@LINE-1]]:13: error: .module directive must appear before any code
 
     .set dsp
     .module fp=64
 # CHECK: :[[@LINE-1]]:13: error: .module directive must appear before any code
 
+    .set dspr2
+    .module fp=64
+# CHECK: :[[@LINE-1]]:13: error: .module directive must appear before any code
+
     .llvm_internal_mips_reallow_module_directive
     .module fp=32
 # CHECK-NOT: :[[@LINE-1]]:13: error: .module directive must appear before any code
 
     .set nodsp
     .module fp=64
 # CHECK: :[[@LINE-1]]:13: error: .module directive must appear before any code
 

llvm/trunk/test/MC/Mips/set-nodsp.s

-# RUN: not llvm-mc %s -mcpu=mips32 -mattr=+dsp -triple mips-unknown-linux 2>%t1
+# RUN: not llvm-mc %s -mcpu=mips32 -mattr=+dspr2 -triple mips-unknown-linux 2>%t1
 # RUN: FileCheck %s < %t1
 
   lbux    $7, $10($11)
+  append  $4, $10, 2
 
   .set nodsp
   lbux    $6, $10($11)
   # CHECK: error: instruction requires a CPU feature not currently enabled
 
   .set dsp
   lbux    $5, $10($11)
   # CHECK-NOT: error: instruction requires a CPU feature not currently enabled
+
+  .set nodsp
+  append  $3, $10, 2
+  # CHECK: error: instruction requires a CPU feature not currently enabled
+
+  .set dspr2
+  append  $2, $10, 2
+  # CHECK-NOT: error: instruction requires a CPU feature not currently enabled