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

[Sean Silva]

On Wed, Jul 22, 2015 at 10:29 AM, Richard Smith <metafoo at gmail.com> wrote:

> On 21 Jul 2015 8:25 pm, "Ben Langmuir" <blangmuir at apple.com> wrote:
> >
> > Hi all,
> >
> > While reviewing 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.

-- 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/20150722/2ca9014f/attachment.html>