This is an archive of the discontinued LLVM Phabricator instance.

[mips] [IAS] Refactor the function which checks for the availability of AT. NFC.
ClosedPublic

Authored by tomatabacu on Mar 20 2015, 3:36 AM.

Details

Summary

Refactor MipsAsmParser::getATReg to return an internal register number instead of a register index.
Also change all the int's to unsigned, seeing as the current AT register index is stored as an unsigned in MipsAssemblerOptions.

Diff Detail

Event Timeline

tomatabacu updated this revision to Diff 22338.Mar 20 2015, 3:36 AM
tomatabacu retitled this revision from to [mips] [IAS] Refactor the function which checks for the availability of AT. NFC..
tomatabacu updated this object.
tomatabacu edited the test plan for this revision. (Show Details)
tomatabacu added a reviewer: dsanders.
tomatabacu added a subscriber: Unknown Object (MLST).
tomatabacu updated this revision to Diff 22339.Mar 20 2015, 3:54 AM

Remove other WIP from this patch.

dsanders edited edge metadata.Mar 24 2015, 4:06 AM

I agree with changing the int's to unsigned but I think the re-factor is going in the wrong direction at the moment. Instead of partially inlining MipsAsmParser::getATReg() and having a predicate function with side effects (error messages), I'd instead either rename it getATRegIndex() to make it clear it's an index rather than an internal register number, or I'd move the getReg() call inside the function so that it does return an internal register number.

tomatabacu updated this revision to Diff 23410.Apr 8 2015, 3:44 AM
tomatabacu edited edge metadata.

Reverted the isATAvailable change.
Changed getATReg to return an internal register number instead of a register index.

tomatabacu updated this object.Apr 8 2015, 3:45 AM
dsanders accepted this revision.Apr 15 2015, 1:48 AM
dsanders edited edge metadata.

LGTM

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

We should correct this getATRegNum() function too since it really returns an index. That can be a follow-on patch if you prefer.

This revision is now accepted and ready to land.Apr 15 2015, 1:48 AM

Replied to an inline comment.

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

getATRegNum() is renamed to getATRegIndex() in D8480, which depends on this patch.

tomatabacu closed this revision.Apr 15 2015, 3:51 AM