[PATCH] D38537: [mips] Implement .set dspr2 directive

[Petar Jovanovic via Phabricator via llvm-commits]

petarj added inline comments.


================
Comment at: test/MC/Mips/set-nodsp.s:1
-# RUN: not llvm-mc %s -mcpu=mips32 -mattr=+dsp -triple mips-unknown-linux 2>%t1
+# RUN: not llvm-mc %s -mcpu=mips32 -mattr=+dsp -mattr=+dspr2 -triple mips-unknown-linux 2>%t1
 # RUN: FileCheck %s < %t1
----------------
sdardis wrote:
> petarj wrote:
> > -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?
> > -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.
> 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.


https://reviews.llvm.org/D38537