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

[Logan Chien]

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
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120903/153c98e3/attachment.html>