[cfe-dev] [RFC] Make "requires arm" also include AArch64 in module map files

[Ben Langmuir]

> On Jul 22, 2015, at 12:56 PM, Sean Silva <chisophugis at gmail.com> wrote:
> 
> 
> 
> On Wed, Jul 22, 2015 at 10:29 AM, Richard Smith <metafoo at gmail.com <mailto:metafoo at gmail.com>> wrote:
> On 21 Jul 2015 8:25 pm, "Ben Langmuir" <blangmuir at apple.com <mailto:blangmuir at apple.com>> wrote:
> >
> > Hi all,
> >
> > While reviewing http://reviews.llvm.org/D10423 <http://reviews.llvm.org/D10423>, I ran into a problem with our _Builtin_Intrinsics module map file.  It contains a submodule for arm intrinsics:
> >
> >   explicit module arm {
> >     requires arm
> >
> > This doesn’t work as intended, because today only 32bit arm targets publish the “arm” target feature, but this module is supposed to be used on AArch64 as well (e.g. for the arm_neon.h header).  The target features that we can use in a module “requires” declaration are controlled by TargetInfo::hasFeature().  Only module map parsing currently checks for the arm feature.
> >
> > I propose that we rename the current “arm” feature to “arm32” and take over the name “arm” to mean “arm32 || aarch64”.
> >
> > Pros
> > * Mirrors the x86/x86_32/x86_64 feature names and functionality
> > * Anecdotally, this better matches user expectation of what “requires arm” means
> >
> > Cons
> > * This is a backwards-incompatible change if any modules rely on “requires arm” failing on aarch64 targets.
> >
> > Would this break existing modules for anyone who cannot just add a “requires !aarch64” to their module map? Or does anyone feel strongly that this is the wrong solution?
> 
> I agree that this is the right solution, assuming it doesn't break anyone.
> 
> Agreed. Realistically I don't expect anybody besides Apple to be shipping modules in a situation where ambiguity with aarch64 could be an issue, so the breakage (if any) should hopefully be limited to apple systems Ben can test.

The Darwin SDKs don’t use the “arm” requirement anywhere.  The only case I’ve come across at all is the one in the clang builtin modules.

> 
> -- Sean Silva 
> > The other possibilities I considered, but felt were worse solutions:
> > (a) Keep the meaning of “arm” and invent a new feature to mean "arm || aarch64".  Mostly I felt this left the existing name unnecessarily confusing.
> > (b) accepting the state of the compiler and duplicating the “arm” submodule. In theory, isBetterKnownHeader will allow this to “just work” for #includes, but users would have to spell their @imports differently for different targets, or
> > (c) adding some general feature to spell disjunctions in module requires decls (this didn’t seem worth it since I can’t think of any other users for such a feature).
> >
> >
> > Ben
> >
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20150723/eb948ffc/attachment.html>