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

[Sean Silva]

On Thu, Jul 23, 2015 at 2:39 PM, Ben Langmuir <blangmuir at apple.com> wrote:

>
> 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> 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.
>
>
> 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.
>
>
Ok, then since the builtin module will be in sync with its corresponding
clang, we should not have any compatibility concern here.

-- Sean Silva


>
> -- 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/694a590f/attachment.html>