This is an archive of the discontinued LLVM Phabricator instance.

Differential D5380

builtins: remove definition of __ARM_ARCH
ClosedPublic

Authored by compnerd on Sep 17 2014, 8:11 AM.

Details

Reviewers
rengolin
t.p.northover
jroelofs
joerg
Summary

__ARM_ARCH is part of the ACLE specification. At least clang and GCC have
supported this part of the ACLE for some time now. Let the compiler provide the
proper definition for the macro rather than try to guess it.

Diff Detail

Event Timeline

compnerd updated this revision to Diff 13784.Sep 17 2014, 8:11 AM
compnerd retitled this revision from to builtins: remove definition of __ARM_ARCH.
compnerd updated this object.
compnerd edited the test plan for this revision. (Show Details)
compnerd added reviewers: t.p.northover, jroelofs, rengolin, joerg.
compnerd set the repository for this revision to rL LLVM.
compnerd added a subscriber: Unknown Object (MLST).
Herald added a subscriber: aemerson. · View Herald TranscriptSep 17 2014, 8:11 AM
joerg edited edge metadata.Sep 17 2014, 9:52 AM

GCC 4.5 was a secondary target for us (NetBSD) at the time and I think that is one of the versions missing it.

jroelofs edited edge metadata.Sep 17 2014, 5:12 PM

Looks reasonable to me, but I'd defer to Tim & Renato.

rengolin edited edge metadata.Sep 17 2014, 5:30 PM

Joerg, is ACLE correct for BSD now?

joerg added a comment.Sep 18 2014, 2:35 AM

Double checked, GCC 4.5 is certainly missing the defines. For NetBSD this is no longer relevant, but I can't speak for FreeBSD and OpenBSD which are more likely to run into problems with GCC 4.2 on this.

rengolin accepted this revision.Sep 18 2014, 8:22 AM
rengolin edited edge metadata.

Thanks Joerg, I think it should be fine. LGTM.

This revision is now accepted and ready to land.Sep 18 2014, 8:22 AM
emaste added a subscriber: emaste.Sep 18 2014, 10:15 AM

On FreeBSD we use Clang as the system compiler on amd64/amd64, arm/arm, arm/armv6, i386/i386, pc98/i386, powerpc/powerpc and powerpc/powerpc64.

emaste added a comment.Sep 18 2014, 11:10 AM

Oops -- I copied the wrong list, it should be the platforms on which we set WITH_CLANG_IS_CC by default. That list does still include arm/arm and arm/armv6.

compnerd closed this revision.Sep 18 2014, 6:45 PM

Committed as SVN r218095.

Revision Contents

PathSize
lib/
builtins/
27 lines

Diff 13784

lib/builtins/assembly.h

Context not available.
#endif #endif
#if defined(__arm__) #if defined(__arm__)
#ifndef __ARM_ARCH
#if defined(__ARM_ARCH_7__) || defined(__ARM_ARCH_7A__) || \
defined(__ARM_ARCH_7R__) || defined(__ARM_ARCH_7M__) || \
defined(__ARM_ARCH_7EM__)
#define __ARM_ARCH 7
#endif
#endif
#ifndef __ARM_ARCH
#if defined(__ARM_ARCH_6__) || defined(__ARM_ARCH_6J__) || \
defined(__ARM_ARCH_6K__) || defined(__ARM_ARCH_6Z__) || \
defined(__ARM_ARCH_6ZK__) || defined(__ARM_ARCH_6ZM__)
#define __ARM_ARCH 6
#endif
#endif
#ifndef __ARM_ARCH
#if defined(__ARM_ARCH_5__) || defined(__ARM_ARCH_5T__) || \
defined(__ARM_ARCH_5TE__) || defined(__ARM_ARCH_5TEJ__)
#define __ARM_ARCH 5
#endif
#endif
#ifndef __ARM_ARCH
#define __ARM_ARCH 4
#endif
#if defined(__ARM_ARCH_4T__) || __ARM_ARCH >= 5 #if defined(__ARM_ARCH_4T__) || __ARM_ARCH >= 5
#define ARM_HAS_BX #define ARM_HAS_BX
#endif #endif
Context not available.