[PATCH] D24752: [Modules] Add missing dependencies to clang builtins modulemap

2016-10-06 Thread Bruno Cardoso Lopes via cfe-commits 
bruno added inline comments.


> eladcohen wrote in module.modulemap:133
> emmintrin.h is already included explicitly in wmmintrin.h & __wmmintrin_aes.h.
> 
> If a user includes / 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  (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 '; 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.

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

https://reviews.llvm.org/D24752



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24752: [Modules] Add missing dependencies to clang builtins modulemap

2016-09-28 Thread Elad Cohen via cfe-commits 
eladcohen added inline comments.


Comment at: lib/Headers/module.modulemap:133
@@ -131,2 +132,3 @@
 explicit module aes {
+  export sse2
   header "__wmmintrin_aes.h"

bruno wrote:
> 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 
>  in the source file?
emmintrin.h is already included explicitly in wmmintrin.h & __wmmintrin_aes.h.

If a user includes / 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  (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 '; 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.



https://reviews.llvm.org/D24752



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24752: [Modules] Add missing dependencies to clang builtins modulemap

2016-09-27 Thread Bruno Cardoso Lopes via cfe-commits 
bruno added a subscriber: bruno.


Comment at: lib/Headers/module.modulemap:133
@@ -131,2 +132,3 @@
 explicit module aes {
+  export sse2
   header "__wmmintrin_aes.h"

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  in 
the source file?


https://reviews.llvm.org/D24752



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D24752: [Modules] Add missing dependencies to clang builtins modulemap

2016-09-27 Thread Elad Cohen via cfe-commits 
eladcohen added a comment.

ping


https://reviews.llvm.org/D24752



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24752: [Modules] Add missing dependencies to clang builtins modulemap

2016-09-19 Thread Elad Cohen via cfe-commits 
eladcohen created this revision.
eladcohen added a reviewer: rsmith.
eladcohen added a subscriber: cfe-commits.

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.

https://reviews.llvm.org/D24752

Files:
  lib/Headers/module.modulemap
  test/Modules/compiler_builtins_x86_submodules.c

Index: test/Modules/compiler_builtins_x86_submodules.c
===
--- test/Modules/compiler_builtins_x86_submodules.c
+++ test/Modules/compiler_builtins_x86_submodules.c
@@ -0,0 +1,15 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -triple i686-unknown-unknown -target-feature +3dnowa 
-fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s 
-verify -ffreestanding
+
+// expected-no-diagnostics
+
+#include
+
+__m64 x; // Verify that types which are used by mm3dnow.h
+ // but declared in the mmx submodule get imported
+
+#include
+
+__m128i y; // Verify that types which are used by wmmintrin.h
+   // but declared in the sse2 submodule get imported
+
Index: lib/Headers/module.modulemap
===
--- lib/Headers/module.modulemap
+++ lib/Headers/module.modulemap
@@ -119,6 +119,7 @@
 }
 
 explicit module mm3dnow {
+  export mmx
   header "mm3dnow.h"
 }
 
@@ -129,6 +130,7 @@
 }
 
 explicit module aes {
+  export sse2
   header "__wmmintrin_aes.h"
 }
 


Index: test/Modules/compiler_builtins_x86_submodules.c
===
--- test/Modules/compiler_builtins_x86_submodules.c
+++ test/Modules/compiler_builtins_x86_submodules.c
@@ -0,0 +1,15 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -triple i686-unknown-unknown -target-feature +3dnowa -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %s -verify -ffreestanding
+
+// expected-no-diagnostics
+
+#include
+
+__m64 x; // Verify that types which are used by mm3dnow.h
+ // but declared in the mmx submodule get imported
+
+#include
+
+__m128i y; // Verify that types which are used by wmmintrin.h
+   // but declared in the sse2 submodule get imported
+
Index: lib/Headers/module.modulemap
===
--- lib/Headers/module.modulemap
+++ lib/Headers/module.modulemap
@@ -119,6 +119,7 @@
 }
 
 explicit module mm3dnow {
+  export mmx
   header "mm3dnow.h"
 }
 
@@ -129,6 +130,7 @@
 }
 
 explicit module aes {
+  export sse2
   header "__wmmintrin_aes.h"
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits