Hi,<br><br>  Yes.  Currently, only arm-linux-androideabi is used.  However<br>I have 2 concerns:<br><br>1. I'm afraid that it will be confusing to the user that clang supports<br>arm-linux-androideabi, i686-linux-android, mipsel-linux-android, but<br>
NOT arm-linux-android.<br><br>2. From the LLVM/Clang developer point of view, adding one additional<br>case will become error-prone.  (see the switch cases in the attached patch)<br><br>I have created the new patch set to split Android into Android and AndroidEABI<br>
as you have suggested.  However, I'm not sure it is a good idea or not,<br>please let me know if you have different thought or if I did anything wrong.<br>Your comments are appreciated.  Thanks.<br><br>Sincerely,<br>Logan<br>
<br>p.s. For your convenience,<br><a href="https://github.com/loganchien/llvm-logan/compare/master...logan-androideabi">https://github.com/loganchien/llvm-logan/compare/master...logan-androideabi</a><br><a href="https://github.com/loganchien/clang-logan/compare/master...logan-androideabi">https://github.com/loganchien/clang-logan/compare/master...logan-androideabi</a><br>
<br><div class="gmail_quote">On Mon, Sep 3, 2012 at 5:48 PM, Evgeniy Stepanov <span dir="ltr"><<a href="mailto:eugeni.stepanov@gmail.com" target="_blank">eugeni.stepanov@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Do we need to support arm-linux-android at all? AFAIK, it's always<br>
arm-linux-androideabi.<br>
<div class="HOEnZb"><div class="h5"><br>
On Mon, Sep 3, 2012 at 12:48 PM, Logan Chien <<a href="mailto:tzuhsiang.chien@gmail.com">tzuhsiang.chien@gmail.com</a>> wrote:<br>
> I did hesitate while adding "android" to isEABI as well.<br>
><br>
> The problem I am facing is that I wish isEABI("arm-linux-android") == true,<br>
> isEABI("i686-unknown-linux-android") == false.  After some consideration,<br>
> I thought it is strange to check isEABI() on non-ARM architecture, thus I<br>
> chose to add "android".<br>
><br>
> BTW, I'm afraid that adding a new different environment type won't help.<br>
> Because we want "-android" considered as EABI on ARM, but not on X86.<br>
><br>
> If we should exclude non-ARM architecture, how about changing the code into:<br>
><br>
> bool isEABI() const {<br>
>   const llvm::Triple& TT = getContext().getTargetInfo().getTriple();<br>
>   StringRef Env = TT.getEnvironmentName();<br>
><br>
>   return (Env == "gnueabi" || Env == "eabi" || Env == "androideabi" ||<br>
>           ((TT.getArch() == llvm::Triple::arm ||<br>
>             TT.getArch() == llvm::Triple::thumb)) && Env == "android"));<br>
> }<br>
><br>
> Or do you have better suggestion?  Thanks for your review.<br>
><br>
> Sincerely,<br>
> Logan<br>
><br>
><br>
><br>
> On Mon, Sep 3, 2012 at 4:16 PM, Evgeniy Stepanov <<a href="mailto:eugeni.stepanov@gmail.com">eugeni.stepanov@gmail.com</a>><br>
> wrote:<br>
>><br>
>> The following chunk looks weird to me.<br>
>><br>
>>    bool isEABI() const {<br>
>>      StringRef Env =<br>
>>        getContext().getTargetInfo().getTriple().getEnvironmentName();<br>
>> -    return (Env == "gnueabi" || Env == "eabi" || Env == "androideabi");<br>
>> +    return (Env == "gnueabi" || Env == "eabi" ||<br>
>> +            Env == "android" || Env == "androideabi");<br>
>>    }<br>
>><br>
>> Now we have isEABI() == true for i686-linux-android.<br>
>> I'd rather we kept more information about the original triple by<br>
>> having both android and androideabi environments, and a helper<br>
>> function isAndroid() somewhere.<br>
>><br>
>> On Sun, Sep 2, 2012 at 1:48 PM, Logan Chien <<a href="mailto:tzuhsiang.chien@gmail.com">tzuhsiang.chien@gmail.com</a>><br>
>> wrote:<br>
>> > Thanks.  Commited as r163087, r163088.<br>
>> > -Logan<br>
>> ><br>
>> ><br>
>> ><br>
>> > On Fri, Aug 31, 2012 at 5:25 PM, Anton Korobeynikov<br>
>> > <<a href="mailto:anton@korobeynikov.info">anton@korobeynikov.info</a>> wrote:<br>
>> >><br>
>> >> > This is because that we are using "StartsWith" to translate<br>
>> >> > environment<br>
>> >> > name<br>
>> >> > (eg. andriodeabi, android, gnueabi, ...etc) into<br>
>> >> > Triple::EnvironmentType.<br>
>> >> > Thus,<br>
>> >> > I believe it won't case backward compatibility issue.  :-)<br>
>> >> Ok. LGTM then :)<br>
>> >><br>
>> >> --<br>
>> >> With best regards, Anton Korobeynikov<br>
>> >> Faculty of Mathematics and Mechanics, Saint Petersburg State University<br>
>> ><br>
>> ><br>
>> ><br>
>> > _______________________________________________<br>
>> > cfe-commits mailing list<br>
>> > <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
>> > <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
>> ><br>
><br>
><br>
</div></div></blockquote></div><br>