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 <[email protected]> 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 <[email protected]> > 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 <[email protected]> >> wrote: >> > Thanks. Commited as r163087, r163088. >> > -Logan >> > >> > >> > >> > On Fri, Aug 31, 2012 at 5:25 PM, Anton Korobeynikov >> > <[email protected]> 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 >> > [email protected] >> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> > > > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
