<p dir="ltr">On 21 Jul 2015 8:25 pm, "Ben Langmuir" <<a href="mailto:blangmuir@apple.com">blangmuir@apple.com</a>> wrote:<br>
><br>
> Hi all,<br>
><br>
> While reviewing <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D10423&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=CnzuN65ENJ1H9py9XLiRvC_UQz6u3oG6GUNn7_wosSM&m=R68l_w6zwRouloOO0a6XNZM3EmqInkal1l7JEzR0B78&s=z1v08wPz8H4Uxp708EhN7uB4XS3t-RmjnoaN85kIFpw&e=">http://reviews.llvm.org/D10423</a>, I ran into a problem with our _Builtin_Intrinsics module map file.  It contains a submodule for arm intrinsics:<br>
><br>
>   explicit module arm {<br>
>     requires arm<br>
><br>
> 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.<br>
><br>
> I propose that we rename the current “arm” feature to “arm32” and take over the name “arm” to mean “arm32 || aarch64”.<br>
><br>
> Pros<br>
> * Mirrors the x86/x86_32/x86_64 feature names and functionality<br>
> * Anecdotally, this better matches user expectation of what “requires arm” means<br>
><br>
> Cons<br>
> * This is a backwards-incompatible change if any modules rely on “requires arm” failing on aarch64 targets.<br>
><br>
> 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?</p>
<p dir="ltr">I agree that this is the right solution, assuming it doesn't break anyone.</p>
<p dir="ltr">> The other possibilities I considered, but felt were worse solutions:<br>
> (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.<br>
> (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<br>
> (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).<br>
><br>
><br>
> Ben<br>
><br>
</p>