This is an archive of the discontinued LLVM Phabricator instance.

[Modules] Add missing dependencies to clang builtins modulemap
Needs ReviewPublic

Authored by eladcohen on Sep 19 2016, 10:37 PM.

Details

Reviewers
rsmith
jtsoftware
Summary

X86 intrinsic header files mm3dnow.h and wmmintrin.h include and depend on mmintrin.h and emmintrin.h respectively.
This patch adds these missing dependencies to the corresponding submodules in the clang builtins modulemap.

Diff Detail

Event Timeline

eladcohen updated this revision to Diff 71899.Sep 19 2016, 10:37 PM
eladcohen retitled this revision from to [Modules] Add missing dependencies to clang builtins modulemap.
eladcohen updated this object.
eladcohen added a reviewer: rsmith.
eladcohen added a subscriber: cfe-commits.
bruno added a subscriber: bruno.Sep 27 2016, 9:57 AM
bruno added inline comments.
lib/Headers/module.modulemap
133

The mmx case above makes sense to me, but I find conceptually odd that we need to re-export sse2 in aes module, why not explicitly #include <emmintrin.h> in the source file?

eladcohen added inline comments.Sep 28 2016, 12:03 AM
lib/Headers/module.modulemap
133

emmintrin.h is already included explicitly in wmmintrin.h & __wmmintrin_aes.h.

If a user includes <x86intrin.h>/<immintrin.h> there is no problem, since the intel submodule has an 'export *' directive and both aes & sse2 will be imported.

However, if the user only includes <wmmintrin.h> (like in the 2nd half of the test I added), and uses modules, then any use of the '___m128i' type (which is used in the aes intrinsics and declared in sse2) will result with the error:
"File tst.c Line 11: missing '#include <emmintrin.h>'; declaration of '___m128i' must be imported from module '_Builtin_intrinsics.intel.sse2' before it is required"

IIUC the possible fixes for this are adding 'export *' or 'export sse2' to the aes submodule.

bruno added inline comments.Oct 6 2016, 1:21 PM
lib/Headers/module.modulemap
133

I see, if you need the type to use the exported functions it makes sense to re-export it. It might be worth adding a comment // note: for ___m128i