This is an archive of the discontinued LLVM Phabricator instance.

[mips] Add macros _MIPS_ISA and __mips_isa_rev (same expansion as defined by GCC).
ClosedPublic

Authored by matheusalmeida on May 14 2014, 3:29 AM.

Diff Detail

Event Timeline

matheusalmeida retitled this revision from to [mips] Add macros _MIPS_ISA and __mips_isa_rev (same expansion as defined by GCC)..
matheusalmeida updated this object.
matheusalmeida edited the test plan for this revision. (Show Details)
atanasyan added inline comments.May 14 2014, 11:44 AM
lib/Basic/Targets.cpp
5231

I suggest to return StringRef. Usually we use this interface to manipulate with string so it reduce a number of type conversions later.

5458

Do we really need a case insensitive string comparison here? As far as i understand MIPS32R2 is not a valid CPU's name. For example, gcc rejects it.

matheusalmeida added inline comments.Jun 5 2014, 5:58 AM
lib/Basic/Targets.cpp
5231

I don't think we'll benefit much of returning a StringRef here. Are you happy if I keep returning a const string& and drop the StringRef from the actual string compare later on ? The rationale is that I only need to compare a string against other and I can easily do it with std:string.

5458

I'm sorry it took so long to get back to this but I have been very busy. The rationale behind forcing a lower case compare is because Clang doesn't verify the CPU string specified by the user. For example, I can specify clang --target=MIPS-unknown-unknown -mcpu=MIPS32r2 -E and clang won't complain about i and the macros won't be defined for such case.

atanasyan accepted this revision.Jun 5 2014, 6:17 AM
atanasyan edited edge metadata.

LGTM

This revision is now accepted and ready to land.Jun 5 2014, 6:17 AM
matheusalmeida edited edge metadata.

Stopped creating StringRefs before comparing CPUStr.
Also stopped forcing lower case string comparisons. Another patch will canonicalize the cpu name so that
we can rely the name will be lower case.

LGTM Feel free to commit this revision.

Sure. I uploaded the new patch so that I could use phabricator to commit it and it would automatically close the revision.

Thanks,
Matheus

matheusalmeida closed this revision.Jun 5 2014, 8:07 AM