[cfe-commits] [llvm-commits] [PATCH] Rename llvm::Triple::ANDROIDEABI to llvm::Triple::Android

[Evgeniy Stepanov]

Do we need to support arm-linux-android at all? AFAIK, it's always
arm-linux-androideabi.

On Mon, Sep 3, 2012 at 12:48 PM, Logan Chien <tzuhsiang.chien at gmail.com> wrote:
> I did hesitate while adding "android" to isEABI as well.
>
> The problem I am facing is that I wish isEABI("arm-linux-android") == true,
> isEABI("i686-unknown-linux-android") == false.  After some consideration,
> I thought it is strange to check isEABI() on non-ARM architecture, thus I
> chose to add "android".
>
> BTW, I'm afraid that adding a new different environment type won't help.
> Because we want "-android" considered as EABI on ARM, but not on X86.
>
> If we should exclude non-ARM architecture, how about changing the code into:
>
> bool isEABI() const {
>   const llvm::Triple& TT = getContext().getTargetInfo().getTriple();
>   StringRef Env = TT.getEnvironmentName();
>
>   return (Env == "gnueabi" || Env == "eabi" || Env == "androideabi" ||
>           ((TT.getArch() == llvm::Triple::arm ||
>             TT.getArch() == llvm::Triple::thumb)) && Env == "android"));
> }
>
> Or do you have better suggestion?  Thanks for your review.
>
> Sincerely,
> Logan
>
>
>
> On Mon, Sep 3, 2012 at 4:16 PM, Evgeniy Stepanov <eugeni.stepanov at gmail.com>
> wrote:
>>
>> The following chunk looks weird to me.
>>
>>    bool isEABI() const {
>>      StringRef Env =
>>        getContext().getTargetInfo().getTriple().getEnvironmentName();
>> -    return (Env == "gnueabi" || Env == "eabi" || Env == "androideabi");
>> +    return (Env == "gnueabi" || Env == "eabi" ||
>> +            Env == "android" || Env == "androideabi");
>>    }
>>
>> Now we have isEABI() == true for i686-linux-android.
>> I'd rather we kept more information about the original triple by
>> having both android and androideabi environments, and a helper
>> function isAndroid() somewhere.
>>
>> On Sun, Sep 2, 2012 at 1:48 PM, Logan Chien <tzuhsiang.chien at gmail.com>
>> wrote:
>> > Thanks.  Commited as r163087, r163088.
>> > -Logan
>> >
>> >
>> >
>> > On Fri, Aug 31, 2012 at 5:25 PM, Anton Korobeynikov
>> > <anton at korobeynikov.info> wrote:
>> >>
>> >> > This is because that we are using "StartsWith" to translate
>> >> > environment
>> >> > name
>> >> > (eg. andriodeabi, android, gnueabi, ...etc) into
>> >> > Triple::EnvironmentType.
>> >> > Thus,
>> >> > I believe it won't case backward compatibility issue.  :-)
>> >> Ok. LGTM then :)
>> >>
>> >> --
>> >> With best regards, Anton Korobeynikov
>> >> Faculty of Mathematics and Mechanics, Saint Petersburg State University
>> >
>> >
>> >
>> > _______________________________________________
>> > cfe-commits mailing list
>> > cfe-commits at cs.uiuc.edu
>> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>> >
>
>