[PATCH] D64672: [X86] Prevent passing vectors of __int128 as in llvm IR

[Craig Topper via Phabricator via cfe-commits]

craig.topper closed this revision.
craig.topper added a comment.

Committed in r371169, but I forgot the Differential Revision line


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64672/new/

https://reviews.llvm.org/D64672



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

[PATCH] D64672: [X86] Prevent passing vectors of __int128 as in llvm IR

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added a comment.

@craig.topper Wasn't this committed at rL371169 
 ?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64672/new/

https://reviews.llvm.org/D64672



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

[PATCH] D64672: [X86] Prevent passing vectors of __int128 as in llvm IR

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon accepted this revision.
RKSimon added a comment.
This revision is now accepted and ready to land.

LGTM - cheers


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64672/new/

https://reviews.llvm.org/D64672



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

[PATCH] D64672: [X86] Prevent passing vectors of __int128 as in llvm IR

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

It looks to me like the patch does things the New Way only for Linux and 
NetBSD, so for PS4 backward compatibility I am okay with it.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64672/new/

https://reviews.llvm.org/D64672



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

[PATCH] D64672: [X86] Prevent passing vectors of __int128 as in llvm IR

[Craig Topper via Phabricator via cfe-commits]

craig.topper updated this revision to Diff 218506.
craig.topper added a comment.

Change test name and adjust to the changes I think @rksimon was asking for. I 
haven't commited the test yet. I'd like to verify that the before version of 
the test is how you would like it pre-committed


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64672/new/

https://reviews.llvm.org/D64672

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/LangOptions.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/x86-vec-i128.c

Index: clang/test/CodeGen/x86-vec-i128.c
===
--- clang/test/CodeGen/x86-vec-i128.c
+++ clang/test/CodeGen/x86-vec-i128.c
@@ -3,24 +3,27 @@
 // RUN: %clang_cc1 -triple x86_64-apple-darwin %s -target-feature +sse2 -S -emit-llvm -o - | FileCheck %s --check-prefixes=CLANG9ABI128,MEM256ALIGN16,MEM512ALIGN16
 // RUN: %clang_cc1 -triple x86_64-scei-ps4 %s -target-feature +sse2 -S -emit-llvm -o - | FileCheck %s --check-prefixes=CLANG9ABI128,MEM256ALIGN32,MEM512ALIGN64
 // RUN: %clang_cc1 -triple x86_64-unknown-freebsd10.0 %s -target-feature +sse2 -S -emit-llvm -o - | FileCheck %s --check-prefixes=CLANG9ABI128,MEM256ALIGN32,MEM512ALIGN64
+// RUN: %clang_cc1 -triple x86_64-linux-gnu %s -target-feature +sse2 -S -emit-llvm -o - -fclang-abi-compat=9 | FileCheck %s --check-prefixes=CLANG9ABI128,MEM256ALIGN32,MEM512ALIGN64
 
 // RUN: %clang_cc1 -triple x86_64-linux-gnu %s -target-feature +avx -S -emit-llvm -o - | FileCheck %s --check-prefixes=CLANG10ABI128,CLANG10ABI256,MEM512ALIGN64
 // RUN: %clang_cc1 -triple x86_64-netbsd %s -target-feature +avx -S -emit-llvm -o - | FileCheck %s --check-prefixes=CLANG10ABI128,CLANG10ABI256,MEM512ALIGN64
 // RUN: %clang_cc1 -triple x86_64-apple-darwin %s -target-feature +avx -S -emit-llvm -o - | FileCheck %s --check-prefixes=CLANG9ABI128,CLANG9ABI256,MEM512ALIGN32
 // RUN: %clang_cc1 -triple x86_64-scei-ps4 %s -target-feature +avx -S -emit-llvm -o - | FileCheck %s --check-prefixes=CLANG9ABI128,CLANG9ABI256,MEM512ALIGN64
 // RUN: %clang_cc1 -triple x86_64-unknown-freebsd10.0 %s -target-feature +avx -S -emit-llvm -o - | FileCheck %s --check-prefixes=CLANG9ABI128,CLANG9ABI256,MEM512ALIGN64
+// RUN: %clang_cc1 -triple x86_64-linux-gnu %s -target-feature +avx -S -emit-llvm -o - -fclang-abi-compat=9 | FileCheck %s --check-prefixes=CLANG9ABI128,CLANG9ABI256,MEM512ALIGN64
 
 // RUN: %clang_cc1 -triple x86_64-linux-gnu %s -target-feature +avx512f -S -emit-llvm -o - | FileCheck %s --check-prefixes=CLANG10ABI128,CLANG10ABI256,CLANG10ABI512
 // RUN: %clang_cc1 -triple x86_64-netbsd %s -target-feature +avx512f -S -emit-llvm -o - | FileCheck %s --check-prefixes=CLANG10ABI128,CLANG10ABI256,CLANG10ABI512
 // RUN: %clang_cc1 -triple x86_64-apple-darwin %s -target-feature +avx512f -S -emit-llvm -o - | FileCheck %s --check-prefixes=CLANG9ABI128,CLANG9ABI256,CLANG9ABI512
 // RUN: %clang_cc1 -triple x86_64-scei-ps4 %s -target-feature +avx512f -S -emit-llvm -o - | FileCheck %s --check-prefixes=CLANG9ABI128,CLANG9ABI256,CLANG9ABI512
 // RUN: %clang_cc1 -triple x86_64-unknown-freebsd10.0 %s -target-feature +avx512f -S -emit-llvm -o - | FileCheck %s --check-prefixes=CLANG9ABI128,CLANG9ABI256,CLANG9ABI512
+// RUN: %clang_cc1 -triple x86_64-linux-gnu %s -target-feature +avx512f -S -emit-llvm -o - -fclang-abi-compat=9 | FileCheck %s --check-prefixes=CLANG9ABI128,CLANG9ABI256,CLANG9ABI512
 
 typedef unsigned long long v16u64 __attribute__((vector_size(16)));
 typedef unsigned __int128 v16u128 __attribute__((vector_size(16)));
 
 v16u64 test_v16u128(v16u64 a, v16u128 b) {
-// CLANG10ABI128: define <2 x i64> @test_v16u128(<2 x i64> %{{.*}}, <1 x i128> %{{.*}})
+// CLANG10ABI128: define <2 x i64> @test_v16u128(<2 x i64> %{{.*}}, <2 x i64> %{{.*}})
 // CLANG9ABI128: define <2 x i64> @test_v16u128(<2 x i64> %{{.*}}, <1 x i128> %{{.*}})
   return a + (v16u64)b;
 }
@@ -31,7 +34,7 @@
 v32u64 test_v32u128(v32u64 a, v32u128 b) {
 // MEM256ALIGN16: define <4 x i64> @test_v32u128(<4 x i64>* byval(<4 x i64>) align 16 %{{.*}}, <2 x i128>* byval(<2 x i128>) align 16 %{{.*}})
 // MEM256ALIGN32: define <4 x i64> @test_v32u128(<4 x i64>* byval(<4 x i64>) align 32 %{{.*}}, <2 x i128>* byval(<2 x i128>) align 32 %{{.*}})
-// CLANG10ABI256: define <4 x i64> @test_v32u128(<4 x i64> %{{.*}}, <2 x i128> %{{.*}})
+// CLANG10ABI256: define <4 x i64> @test_v32u128(<4 x i64> %{{.*}}, <2 x i128>* byval(<2 x i128>) align 32 %{{.*}})
 // CLANG9ABI256: define <4 x i64> @test_v32u128(<4 x i64> %{{.*}}, <2 x i128> %{{.*}})
   return a + (v32u64)b;
 }
@@ -43,7 +46,7 @@
 // MEM512ALIGN16: define <8 x i64> @test_v64u128(<8 x i64>* byval(<8 x i64>) align 16 %{{.*}}, <4 x i128>* byval(<4 x i128>) align 16 %{{.*}})
 // MEM512ALIGN32: define <8 x i64> @test_v64u128(<8 x i64>* byval(<8 x i64>) align 32 %{{.*}}, <4 x i128>* byval(<4 x i128>) align 32 %{{.*}})
 // MEM512ALIGN64: define <8 x i64> @test_v64u128(<8 x i64>* by

[PATCH] D64672: [X86] Prevent passing vectors of __int128 as in llvm IR

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added a comment.

In D64672#1654983 , @craig.topper 
wrote:

> In D64672#1654687 , @RKSimon wrote:
>
> > Tests look great - please can you pre-commit them and update the patch to 
> > show the diff? Also, maybe call the test file x86-vec-i128.c and add a 
> > comment in the file describing PR42607?
>
>
> How do you want me to show the diff? There a couple options. Three of the 
> check lines have to be removed from the pre-commit since they use a command 
> line option that doesn't exist. Do you want me to change the check lines that 
> say NEWABI to use the old code? Or do you want me to manipulate the FileCheck 
> command lines to use the OLDABI check lines?


How about CLANG9ABI and CLANG10ABI? And the patch just adds the 
-fclang-abi-compat=9 tests?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64672/new/

https://reviews.llvm.org/D64672



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

[PATCH] D64672: [X86] Prevent passing vectors of __int128 as in llvm IR

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

In D64672#1654687 , @RKSimon wrote:

> Tests look great - please can you pre-commit them and update the patch to 
> show the diff? Also, maybe call the test file x86-vec-i128.c and add a 
> comment in the file describing PR42607?


How do you want me to show the diff? There a couple options. Three of the check 
lines have to be removed from the pre-commit since they use a command line 
option that doesn't exist. Do you want me to change the check lines that say 
NEWABI to use the old code? Or do you want me to manipulate the FileCheck 
command lines to use the OLDABI check lines?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64672/new/

https://reviews.llvm.org/D64672



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

[PATCH] D64672: [X86] Prevent passing vectors of __int128 as in llvm IR

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added a comment.

Tests look great - please can you pre-commit them and update the patch to show 
the diff? Also, maybe call the test file x86-vec-i128.c and add a comment in 
the file describing PR42607?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64672/new/

https://reviews.llvm.org/D64672



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

[PATCH] D64672: [X86] Prevent passing vectors of __int128 as in llvm IR

[Craig Topper via Phabricator via cfe-commits]

craig.topper updated this revision to Diff 218284.
craig.topper added a comment.

Add more tests. Clarify which platforms are affected in LangOptions.h


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64672/new/

https://reviews.llvm.org/D64672

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/LangOptions.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/pr42607.c

Index: clang/test/CodeGen/pr42607.c
===
--- /dev/null
+++ clang/test/CodeGen/pr42607.c
@@ -0,0 +1,52 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu %s -target-feature +sse2 -S -emit-llvm -o - | FileCheck %s --check-prefixes=NEWABI128,MEM256ALIGN32,MEM512ALIGN64
+// RUN: %clang_cc1 -triple x86_64-netbsd %s -target-feature +sse2 -S -emit-llvm -o - | FileCheck %s --check-prefixes=NEWABI128,MEM256ALIGN32,MEM512ALIGN64
+// RUN: %clang_cc1 -triple x86_64-apple-darwin %s -target-feature +sse2 -S -emit-llvm -o - | FileCheck %s --check-prefixes=OLDABI128,MEM256ALIGN16,MEM512ALIGN16
+// RUN: %clang_cc1 -triple x86_64-scei-ps4 %s -target-feature +sse2 -S -emit-llvm -o - | FileCheck %s --check-prefixes=OLDABI128,MEM256ALIGN32,MEM512ALIGN64
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd10.0 %s -target-feature +sse2 -S -emit-llvm -o - | FileCheck %s --check-prefixes=OLDABI128,MEM256ALIGN32,MEM512ALIGN64
+// RUN: %clang_cc1 -triple x86_64-linux-gnu %s -target-feature +sse2 -S -emit-llvm -o - -fclang-abi-compat=9 | FileCheck %s --check-prefixes=OLDABI128,MEM256ALIGN32,MEM512ALIGN64
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu %s -target-feature +avx -S -emit-llvm -o - | FileCheck %s --check-prefixes=NEWABI128,NEWABI256,MEM512ALIGN64
+// RUN: %clang_cc1 -triple x86_64-netbsd %s -target-feature +avx -S -emit-llvm -o - | FileCheck %s --check-prefixes=NEWABI128,NEWABI256,MEM512ALIGN64
+// RUN: %clang_cc1 -triple x86_64-apple-darwin %s -target-feature +avx -S -emit-llvm -o - | FileCheck %s --check-prefixes=OLDABI128,OLDABI256,MEM512ALIGN32
+// RUN: %clang_cc1 -triple x86_64-scei-ps4 %s -target-feature +avx -S -emit-llvm -o - | FileCheck %s --check-prefixes=OLDABI128,OLDABI256,MEM512ALIGN64
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd10.0 %s -target-feature +avx -S -emit-llvm -o - | FileCheck %s --check-prefixes=OLDABI128,OLDABI256,MEM512ALIGN64
+// RUN: %clang_cc1 -triple x86_64-linux-gnu %s -target-feature +avx -S -emit-llvm -o - -fclang-abi-compat=9 | FileCheck %s --check-prefixes=OLDABI128,OLDABI256,MEM512ALIGN64
+
+// RUN: %clang_cc1 -triple x86_64-linux-gnu %s -target-feature +avx512f -S -emit-llvm -o - | FileCheck %s --check-prefixes=NEWABI128,NEWABI256,NEWABI512
+// RUN: %clang_cc1 -triple x86_64-netbsd %s -target-feature +avx512f -S -emit-llvm -o - | FileCheck %s --check-prefixes=NEWABI128,NEWABI256,NEWABI512
+// RUN: %clang_cc1 -triple x86_64-apple-darwin %s -target-feature +avx512f -S -emit-llvm -o - | FileCheck %s --check-prefixes=OLDABI128,OLDABI256,OLDABI512
+// RUN: %clang_cc1 -triple x86_64-scei-ps4 %s -target-feature +avx512f -S -emit-llvm -o - | FileCheck %s --check-prefixes=OLDABI128,OLDABI256,OLDABI512
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd10.0 %s -target-feature +avx512f -S -emit-llvm -o - | FileCheck %s --check-prefixes=OLDABI128,OLDABI256,OLDABI512
+// RUN: %clang_cc1 -triple x86_64-linux-gnu %s -target-feature +avx512f -S -emit-llvm -o - -fclang-abi-compat=9 | FileCheck %s --check-prefixes=OLDABI128,OLDABI256,OLDABI512
+
+typedef unsigned long long v16u64 __attribute__((vector_size(16)));
+typedef unsigned __int128 v16u128 __attribute__((vector_size(16)));
+
+v16u64 test_v16u128(v16u64 a, v16u128 b) {
+// NEWABI128: define <2 x i64> @test_v16u128(<2 x i64> %{{.*}}, <2 x i64> %{{.*}})
+// OLDABI128: define <2 x i64> @test_v16u128(<2 x i64> %{{.*}}, <1 x i128> %{{.*}})
+  return a + (v16u64)b;
+}
+
+typedef unsigned long long v32u64 __attribute__((vector_size(32)));
+typedef unsigned __int128 v32u128 __attribute__((vector_size(32)));
+
+v32u64 test_v32u128(v32u64 a, v32u128 b) {
+// MEM256ALIGN16: define <4 x i64> @test_v32u128(<4 x i64>* byval(<4 x i64>) align 16 %{{.*}}, <2 x i128>* byval(<2 x i128>) align 16 %{{.*}})
+// MEM256ALIGN32: define <4 x i64> @test_v32u128(<4 x i64>* byval(<4 x i64>) align 32 %{{.*}}, <2 x i128>* byval(<2 x i128>) align 32 %{{.*}})
+// NEWABI256: define <4 x i64> @test_v32u128(<4 x i64> %{{.*}}, <2 x i128>* byval(<2 x i128>) align 32 %{{.*}})
+// OLDABI256: define <4 x i64> @test_v32u128(<4 x i64> %{{.*}}, <2 x i128> %{{.*}})
+  return a + (v32u64)b;
+}
+
+typedef unsigned long long v64u64 __attribute__((vector_size(64)));
+typedef unsigned __int128 v64u128 __attribute__((vector_size(64)));
+
+v64u64 test_v64u128(v64u64 a, v64u128 b) {
+// MEM512ALIGN16: define <8 x i64> @test_v64u128(<8 x i64>* byval(<8 x i64>) align 16 %{{.*}}, <4 x i128>* byval(<4 x i128>) align 16 %{{.*}})
+// MEM512ALIGN32: define <8 x i64> @tes

[PATCH] D64672: [X86] Prevent passing vectors of __int128 as in llvm IR

[Craig Topper via Phabricator via cfe-commits]

craig.topper added a comment.

In D64672#1653847 , @RKSimon wrote:

> Please can you extend the test coverage to cover more vector sizes and cpu 
> target features, not just avx512, and also add x86_64-scei-ps4 triple tests. 
> Maybe 32-bit tests as well?


Other than 256-bit what other size do you want?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64672/new/

https://reviews.llvm.org/D64672



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

[PATCH] D64672: [X86] Prevent passing vectors of __int128 as in llvm IR

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added a comment.

Please can you extend the test coverage to cover more vector sizes and cpu 
target features, not just avx512, and also add x86_64-scei-ps4 triple tests. 
Maybe 32-bit tests as well?




Comment at: clang/include/clang/Basic/LangOptions.h:143
+/// (SVN r351319). This causes vectors of __int128 to be passed in memory
+/// instead of passing in multiple scalar registers.
+Ver9,

Add that this is just for Linux and NetBSD


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64672/new/

https://reviews.llvm.org/D64672



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

[PATCH] D64672: [X86] Prevent passing vectors of __int128 as in llvm IR

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

In D64672#1623784 , @xbolva00 wrote:

> Maybe this ABI issue should be fixed for 9.0 ? Not sure how important, but
>  @hans maybe should track this one.


IIUC, this is not a new issue, so I'd prefer not trying to rush it in for llvm 
9, and rather get it right for llvm 10 instead.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64672/new/

https://reviews.llvm.org/D64672



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

[PATCH] D64672: [X86] Prevent passing vectors of __int128 as in llvm IR

[Craig Topper via Phabricator via cfe-commits]

craig.topper updated this revision to Diff 214540.
craig.topper added a comment.

More test check lines. Test the compatibility flag.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64672/new/

https://reviews.llvm.org/D64672

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/LangOptions.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/pr42607.c

Index: clang/test/CodeGen/pr42607.c
===
--- /dev/null
+++ clang/test/CodeGen/pr42607.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu %s -target-feature +avx512f -S -emit-llvm -o - | FileCheck %s --check-prefix=NEWABI
+// RUN: %clang_cc1 -triple x86_64-apple-darwin %s -target-feature +avx512f -S -emit-llvm -o - | FileCheck %s --check-prefix=OLDABI
+// RUN: %clang_cc1 -triple x86_64-unknown-freebsd10.0 %s -target-feature +avx512f -S -emit-llvm -o - | FileCheck %s --check-prefix=OLDABI
+// RUN: %clang_cc1 -triple x86_64-linux-gnu %s -target-feature +avx512f -S -emit-llvm -o - -fclang-abi-compat=9 | FileCheck %s --check-prefix=OLDABI
+
+typedef unsigned long long v16u64 __attribute__((vector_size(16)));
+typedef unsigned __int128 v16u128 __attribute__((vector_size(16)));
+
+v16u64 bar(v16u64 a, v16u128 b) {
+// NEWABI: define <2 x i64> @bar(<2 x i64> %{{.*}}, <2 x i64> %{{.*}})
+// OLDABI: define <2 x i64> @bar(<2 x i64> %{{.*}}, <1 x i128> %{{.*}})
+  return a + (v16u64)b;
+}
+
+typedef unsigned long long v64u64 __attribute__((vector_size(64)));
+typedef unsigned __int128 v64u128 __attribute__((vector_size(64)));
+
+v64u64 foo(v64u64 a, v64u128 b) {
+// NEWABI: define <8 x i64> @foo(<8 x i64> %{{.*}}, <4 x i128>* byval(<4 x i128>) align 64 %{{.*}})
+// OLDABI: define <8 x i64> @foo(<8 x i64> %{{.*}}, <4 x i128> %{{.*}})
+  return a + (v64u64)b;
+}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3129,6 +3129,8 @@
 Opts.setClangABICompat(LangOptions::ClangABI::Ver6);
   else if (Major <= 7)
 Opts.setClangABICompat(LangOptions::ClangABI::Ver7);
+  else if (Major <= 9)
+Opts.setClangABICompat(LangOptions::ClangABI::Ver9);
 } else if (Ver != "latest") {
   Diags.Report(diag::err_drv_invalid_value)
   << A->getAsString(Args) << A->getValue();
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -2177,6 +2177,17 @@
 return true;
   }
 
+  // GCC classifies vectors of __int128 as memory.
+  bool passInt128VectorsInMem() const {
+// Clang <= 9.0 did not do this.
+if (getContext().getLangOpts().getClangABICompat() <=
+LangOptions::ClangABI::Ver9)
+  return false;
+
+const llvm::Triple &T = getTarget().getTriple();
+return T.isOSLinux() || T.isOSNetBSD();
+  }
+
   X86AVXABILevel AVXLevel;
   // Some ABIs (e.g. X32 ABI and Native Client OS) use 32 bit pointers on
   // 64-bit hardware.
@@ -2657,6 +2668,14 @@
 Hi = Lo;
 } else if (Size == 128 ||
(isNamedArg && Size <= getNativeVectorSizeForAVXABI(AVXLevel))) {
+  QualType ElementType = VT->getElementType();
+
+  // gcc passes 256 and 512 bit  vectors in memory. :(
+  if (passInt128VectorsInMem() && Size != 128 &&
+  (ElementType->isSpecificBuiltinType(BuiltinType::Int128) ||
+   ElementType->isSpecificBuiltinType(BuiltinType::UInt128)))
+return;
+
   // Arguments of 256-bits are split into four eightbyte chunks. The
   // least significant one belongs to class SSE and all the others to class
   // SSEUP. The original Lo and Hi design considers that types can't be
@@ -2899,6 +2918,11 @@
 unsigned LargestVector = getNativeVectorSizeForAVXABI(AVXLevel);
 if (Size <= 64 || Size > LargestVector)
   return true;
+QualType EltTy = VecTy->getElementType();
+if (passInt128VectorsInMem() &&
+(EltTy->isSpecificBuiltinType(BuiltinType::Int128) ||
+ EltTy->isSpecificBuiltinType(BuiltinType::UInt128)))
+  return true;
   }
 
   return false;
@@ -2973,14 +2997,28 @@
 Ty = QualType(InnerTy, 0);
 
   llvm::Type *IRType = CGT.ConvertType(Ty);
-  if (isa(IRType) ||
-  IRType->getTypeID() == llvm::Type::FP128TyID)
+  if (isa(IRType)) {
+// Don't pass vXi128 vectors in their native type, the backend can't
+// legalize them.
+if (passInt128VectorsInMem() &&
+IRType->getVectorElementType()->isIntegerTy(128)) {
+  // Use a vXi64 vector.
+  uint64_t Size = getContext().getTypeSize(Ty);
+  return llvm::VectorType::get(llvm::Type::getInt64Ty(getVMContext()),
+   Size / 64);
+}
+
+return IRType;
+  }
+

[PATCH] D64672: [X86] Prevent passing vectors of __int128 as in llvm IR

[Craig Topper via Phabricator via cfe-commits]

craig.topper updated this revision to Diff 214539.
craig.topper edited the summary of this revision.
craig.topper added a comment.
Herald added a project: clang.

Add test cases. Change the type to vXi64 instead of vXf64. Add abi 
compatibility flag. Restrict to linux and freebsd.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64672/new/

https://reviews.llvm.org/D64672

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/LangOptions.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/pr42607.c

Index: clang/test/CodeGen/pr42607.c
===
--- /dev/null
+++ clang/test/CodeGen/pr42607.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu %s -target-feature +avx512f -S -emit-llvm -o - | FileCheck %s
+
+typedef unsigned long long v16u64 __attribute__((vector_size(16)));
+typedef unsigned __int128 v16u128 __attribute__((vector_size(16)));
+
+v16u64 bar(v16u64 a, v16u128 b) {
+// CHECK: define <2 x i64> @bar(<2 x i64> %{{.*}}, <2 x i64> %{{.*}})
+  return a + (v16u64)b;
+}
+
+typedef unsigned long long v64u64 __attribute__((vector_size(64)));
+typedef unsigned __int128 v64u128 __attribute__((vector_size(64)));
+
+v64u64 foo(v64u64 a, v64u128 b) {
+// CHECK: define <8 x i64> @foo(<8 x i64> %{{.*}}, <4 x i128>* byval(<4 x i128>) align 64 %{{.*}})
+  return a + (v64u64)b;
+}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3129,6 +3129,8 @@
 Opts.setClangABICompat(LangOptions::ClangABI::Ver6);
   else if (Major <= 7)
 Opts.setClangABICompat(LangOptions::ClangABI::Ver7);
+  else if (Major <= 9)
+Opts.setClangABICompat(LangOptions::ClangABI::Ver9);
 } else if (Ver != "latest") {
   Diags.Report(diag::err_drv_invalid_value)
   << A->getAsString(Args) << A->getValue();
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -2177,6 +2177,17 @@
 return true;
   }
 
+  // GCC classifies vectors of __int128 as memory.
+  bool passInt128VectorsInMem() const {
+// Clang <= 9.0 did not do this.
+if (getContext().getLangOpts().getClangABICompat() <=
+LangOptions::ClangABI::Ver9)
+  return false;
+
+const llvm::Triple &T = getTarget().getTriple();
+return T.isOSLinux() || T.isOSNetBSD();
+  }
+
   X86AVXABILevel AVXLevel;
   // Some ABIs (e.g. X32 ABI and Native Client OS) use 32 bit pointers on
   // 64-bit hardware.
@@ -2657,6 +2668,14 @@
 Hi = Lo;
 } else if (Size == 128 ||
(isNamedArg && Size <= getNativeVectorSizeForAVXABI(AVXLevel))) {
+  QualType ElementType = VT->getElementType();
+
+  // gcc passes 256 and 512 bit  vectors in memory. :(
+  if (passInt128VectorsInMem() && Size != 128 &&
+  (ElementType->isSpecificBuiltinType(BuiltinType::Int128) ||
+   ElementType->isSpecificBuiltinType(BuiltinType::UInt128)))
+return;
+
   // Arguments of 256-bits are split into four eightbyte chunks. The
   // least significant one belongs to class SSE and all the others to class
   // SSEUP. The original Lo and Hi design considers that types can't be
@@ -2899,6 +2918,11 @@
 unsigned LargestVector = getNativeVectorSizeForAVXABI(AVXLevel);
 if (Size <= 64 || Size > LargestVector)
   return true;
+QualType EltTy = VecTy->getElementType();
+if (passInt128VectorsInMem() &&
+(EltTy->isSpecificBuiltinType(BuiltinType::Int128) ||
+ EltTy->isSpecificBuiltinType(BuiltinType::UInt128)))
+  return true;
   }
 
   return false;
@@ -2973,14 +2997,28 @@
 Ty = QualType(InnerTy, 0);
 
   llvm::Type *IRType = CGT.ConvertType(Ty);
-  if (isa(IRType) ||
-  IRType->getTypeID() == llvm::Type::FP128TyID)
+  if (isa(IRType)) {
+// Don't pass vXi128 vectors in their native type, the backend can't
+// legalize them.
+if (passInt128VectorsInMem() &&
+IRType->getVectorElementType()->isIntegerTy(128)) {
+  // Use a vXi64 vector.
+  uint64_t Size = getContext().getTypeSize(Ty);
+  return llvm::VectorType::get(llvm::Type::getInt64Ty(getVMContext()),
+   Size / 64);
+}
+
+return IRType;
+  }
+
+  if (IRType->getTypeID() == llvm::Type::FP128TyID)
 return IRType;
 
   // We couldn't find the preferred IR vector type for 'Ty'.
   uint64_t Size = getContext().getTypeSize(Ty);
   assert((Size == 128 || Size == 256 || Size == 512) && "Invalid type found!");
 
+
   // Return a LLVM IR vector type based on the size of 'Ty'.
   return llvm::VectorType::get(llvm::Type::getDoubleTy(getVMContext()),
Size 

[PATCH] D64672: [X86] Prevent passing vectors of __int128 as in llvm IR

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added subscribers: hans, xbolva00.
xbolva00 added a comment.

Maybe this ABI issue should be fixed for 9.0 ? Not sure how important, but
@hans maybe should track this one.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64672/new/

https://reviews.llvm.org/D64672



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

[PATCH] D64672: [X86] Prevent passing vectors of __int128 as in llvm IR

[Warren Ristow via Phabricator via cfe-commits]

wristow added a comment.

> Do we need to keep the old behavior on platforms where clang is the de facto 
> compiler?

I know we (PlayStation) will want to keep the old behavior.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64672/new/

https://reviews.llvm.org/D64672



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

[PATCH] D64672: [X86] Prevent passing vectors of __int128 as in llvm IR

[Craig Topper via Phabricator via cfe-commits]

craig.topper created this revision.
craig.topper added reviewers: eli.friedman, RKSimon, spatel, rnk.

As far as I can tell, gcc passes 256/512 bit vectors __int128 in memory. And 
passes a vector of 1 _int128 in an xmm register. The backend considers  as an illegal type and will scalarize any arguments with that type. So we 
need to coerce the argument types in the frontend to match to avoid the illegal 
type.

Are there other element types to consider? Do we need to keep the old behavior 
on platforms where clang is the de facto compiler?

This issue was identified in PR42607. Though even with the types changed, we 
still seem to be doing some unnecessary stack realignment.

I'll add test cases later today or over the weekend.


https://reviews.llvm.org/D64672

Files:
  clang/lib/CodeGen/TargetInfo.cpp


Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -2657,6 +2657,14 @@
 Hi = Lo;
 } else if (Size == 128 ||
(isNamedArg && Size <= getNativeVectorSizeForAVXABI(AVXLevel))) 
{
+  QualType ElementType = VT->getElementType();
+
+  // gcc passes 256 and 512 bit  vectors in memory. :(
+  if (Size != 128 &&
+  (ElementType->isSpecificBuiltinType(BuiltinType::Int128) ||
+   ElementType->isSpecificBuiltinType(BuiltinType::UInt128)))
+return;
+
   // Arguments of 256-bits are split into four eightbyte chunks. The
   // least significant one belongs to class SSE and all the others to class
   // SSEUP. The original Lo and Hi design considers that types can't be
@@ -2899,6 +2907,10 @@
 unsigned LargestVector = getNativeVectorSizeForAVXABI(AVXLevel);
 if (Size <= 64 || Size > LargestVector)
   return true;
+QualType EltTy = VecTy->getElementType();
+if (EltTy->isSpecificBuiltinType(BuiltinType::Int128) ||
+EltTy->isSpecificBuiltinType(BuiltinType::UInt128))
+  return true;
   }
 
   return false;
@@ -2972,8 +2984,11 @@
   if (const Type *InnerTy = isSingleElementStruct(Ty, getContext()))
 Ty = QualType(InnerTy, 0);
 
+  // Don't pass vXi128 vectors in their native type, the backend can't
+  // legalize them.
   llvm::Type *IRType = CGT.ConvertType(Ty);
-  if (isa(IRType) ||
+  if ((isa(IRType) &&
+   !IRType->getVectorElementType()->isIntegerTy(128)) ||
   IRType->getTypeID() == llvm::Type::FP128TyID)
 return IRType;
 


Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -2657,6 +2657,14 @@
 Hi = Lo;
 } else if (Size == 128 ||
(isNamedArg && Size <= getNativeVectorSizeForAVXABI(AVXLevel))) {
+  QualType ElementType = VT->getElementType();
+
+  // gcc passes 256 and 512 bit  vectors in memory. :(
+  if (Size != 128 &&
+  (ElementType->isSpecificBuiltinType(BuiltinType::Int128) ||
+   ElementType->isSpecificBuiltinType(BuiltinType::UInt128)))
+return;
+
   // Arguments of 256-bits are split into four eightbyte chunks. The
   // least significant one belongs to class SSE and all the others to class
   // SSEUP. The original Lo and Hi design considers that types can't be
@@ -2899,6 +2907,10 @@
 unsigned LargestVector = getNativeVectorSizeForAVXABI(AVXLevel);
 if (Size <= 64 || Size > LargestVector)
   return true;
+QualType EltTy = VecTy->getElementType();
+if (EltTy->isSpecificBuiltinType(BuiltinType::Int128) ||
+EltTy->isSpecificBuiltinType(BuiltinType::UInt128))
+  return true;
   }
 
   return false;
@@ -2972,8 +2984,11 @@
   if (const Type *InnerTy = isSingleElementStruct(Ty, getContext()))
 Ty = QualType(InnerTy, 0);
 
+  // Don't pass vXi128 vectors in their native type, the backend can't
+  // legalize them.
   llvm::Type *IRType = CGT.ConvertType(Ty);
-  if (isa(IRType) ||
+  if ((isa(IRType) &&
+   !IRType->getVectorElementType()->isIntegerTy(128)) ||
   IRType->getTypeID() == llvm::Type::FP128TyID)
 return IRType;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits