[llvm-commits] [llvm] r140178 - in /llvm/trunk/lib/Target/Mips: Mips.td MipsSubtarget.h

[Akira Hatanaka]

I think we should first make it clear what these functions mean.

This is what I intended:
isMips32(): true for any architecture supporting Mips32r1
(Mips32r1/r2, Mips64r1/r2)
isMips32r2(): true for any architecture supporting Mips32r2 (Mips32r2, Mips64r2)
isMips64(): true for any architecture supporting Mips64r1 (Mips64r1/r2)
isMips64r2(): true for any architecture supporting Mips64r2 (Mips64r2)

Is this what you want?
isMips32(): true for Mips32r1/r2
isMips32r2(): true for Mips32r2
isMips64(): true for Mips64r1/r2
isMips64r2(): true for Mips64r2

If that is the case, we will need to change isMips32() too (need to
return false for Mips64r1/r2) and just rename isMips32r2 to
isMipsRev2. Also, I think we will have to replace "isMips32()" with
"isMips32() && isMips64()" and "isMips32r2()" with "isMips32r2() &&
isMips64r2()" in several places (do "grep isMips32()" or "grep
isMips32r2").

On Tue, Sep 20, 2011 at 1:57 PM, Bruno Cardoso Lopes
<bruno.cardoso at gmail.com> wrote:
> Hi Akira,
>
>>   bool isMips32() const { return MipsArchVersion >= Mips32; }
>> -  bool isMips32r2() const { return MipsArchVersion == Mips32r2; }
>> +  bool isMips32r2() const { return MipsArchVersion == Mips32r2 ||
>> +                                   MipsArchVersion == Mips64r2; }
>
> This seems a bit confusing, probably not the way to go. I think
> isMips32r2() should answer only for Mips32r2, if you want to check
> both, have a predicate like "isMipsRev2". Why you want isMips32r2() to
> answer for both?
>
> --
> Bruno Cardoso Lopes
> http://www.brunocardoso.cc
>