[PATCH] D18548: [Mips] add assembler support for .set arch=octeon

[Daniel Sanders via llvm-commits]

dsanders added a comment.

LGTM with the test change


================
Comment at: lib/Target/Mips/AsmParser/MipsAsmParser.cpp:5299
@@ -5298,2 +5298,3 @@
           .Case("cnmips", "cnmips")
+          .Case("octeon", "cnmips")
           .Case("r4000", "mips3") // This is an implementation of Mips3.
----------------
petarj wrote:
> mpf wrote:
> > petarj wrote:
> > > Since the goal of this change is only to support ".set arch=octeon" in inline assembly, I am fine with it.
> > > Having said this, shouldn't we delete the line above, since I do not think that ".set arch=cnmips" is a valid option used anywhere?
> > In case it was not clear I was not objecting to this patch. I'm merely concerned that the support for octeon may end up inconsistent if nobody finishes the other parts of LLVM that would be affected. The most important part would be to sort out the integrated assembler to set E_MIPS_MACH_OCTEON if someone uses -march=octeon or .module=octeon otherwise an octeon elf will masquerade as a simple mips64r2.
> I agree, that's another/related issue with the integrated assembler that should be resolved.
> Having said this, shouldn't we delete the line above, since I do not think that ".set arch=cnmips" is a valid option used anywhere?

I agree with removing the cnmips->cnmips mapping. I don't mind whether it's in this patch or separate.

> The most important part would be to sort out the integrated assembler to set E_MIPS_MACH_OCTEON if someone uses -march=octeon or .module=octeon otherwise an octeon elf will masquerade as a simple mips64r2.

D18899 will fix this.

================
Comment at: test/MC/Mips/set-arch.s:40
@@ -39,2 +39,3 @@
     .set arch=cnmips
+    .set arch=octeon
     .set arch=r4000
----------------
Could you add an Octeon specific instruction after this line and add a check for it below? It doesn't matter which one since we're just testing that the feature bits were updated correctly


Repository:
  rL LLVM

http://reviews.llvm.org/D18548