Page MenuHomePhabricator

[mips] Add assembler support for .set mips0 directive.
ClosedPublic

Authored by tomatabacu on Aug 18 2014, 8:59 AM.

Details

Summary

This directive is used to reset the assembler options to their initial values.
Assembly programmers use it in conjunction with the ".set mipsX" directives.

This patch depends on the .set push/pop directive (http://reviews.llvm.org/D4821).

Contains work done by Matheus Almeida.

Diff Detail

Event Timeline

tomatabacu updated this revision to Diff 12622.Aug 18 2014, 8:59 AM
tomatabacu retitled this revision from to [mips] Add assembler support for .set mips0 directive..
tomatabacu updated this object.
tomatabacu edited the test plan for this revision. (Show Details)
tomatabacu added a reviewer: dsanders.
tomatabacu updated this revision to Diff 12655.Aug 19 2014, 5:22 AM

Improved test.

dsanders edited edge metadata.Aug 19 2014, 8:51 AM

This patch will need updating after you remove the static variable from D4821.

lib/Target/Mips/AsmParser/MipsAsmParser.cpp
82–84

I don't think these 3 are supposed to be reset by '.set mips0'

tomatabacu updated this revision to Diff 12781.EditedAug 21 2014, 7:57 AM
tomatabacu edited edge metadata.

Updated to match the changes in D4821.
Rebased.
Addressed reviewer's concern.

dsanders accepted this revision.Sep 5 2014, 8:54 AM
dsanders edited edge metadata.

LGTM with a bug fix.

lib/Target/Mips/AsmParser/MipsAsmParser.cpp
2742

Don't you need something like this?

AssemblerOptions.back()->setCurrentFeatures(AssemblerOptions.front()->getFeatures());

or this?

AssemblerOptions.back()->setFeatures(getAvailableFeatures());

The test case I have in mind is:

.set msa
addvi.w $w0, $w0, $w0
.set mips0
addvi.w $w0, $w0, $w0 ; Should be an invalid instruction
.set push
addvi.w $w0, $w0, $w0 ; Should be an invalid instruction
.set dsp
addvi.w $w0, $w0, $w0 ; Should be an invalid instruction
.set pop
addvi.w $w0, $w0, $w0 ; Should be an invalid instruction

I think the '.set pop' will accidentally re-enable MSA because the MipsAssemblerOptions object on the stack still has MSA enabled after the '.set mips0'.

This revision is now accepted and ready to land.Sep 5 2014, 8:54 AM
tomatabacu updated this revision to Diff 13381.Sep 8 2014, 2:23 AM
tomatabacu edited edge metadata.

Fixed the bug.

tomatabacu closed this revision.Sep 9 2014, 6:01 AM