[PATCH] D61717: Fix arm_neon.h to be clean under -fno-lax-vector-conversions.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

Many thanks! I've passed on the message, hopefully we can do something about 
this.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61717



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

[PATCH] D61717: Fix arm_neon.h to be clean under -fno-lax-vector-conversions.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D61717#1668980 , @SjoerdMeijer 
wrote:

> Do you perhaps have a test case or error that I can look at? Perhaps I or 
> someone else here can help out a bit here.


You can reproduce the problem with, for example:

  echo '#include ' | clang -target arm64-linux-gnu -arch +neon 
-fsyntax-only -x c - -fno-lax-vector-conversions

I've cleaned up all our other intrinsics headers to be clean under at least 
`-flax-vector-conversions=integer`. I'll be switching the default for all 
targets other than ARM NEON to that imminently. (It'll take a bit more work to 
be able to switch to `-fno-lax-vector-conversions` by default, but getting 
there should be our goal.)

I've filed PR43341 for this issue.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61717



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

[PATCH] D61717: Fix arm_neon.h to be clean under -fno-lax-vector-conversions.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

Yeah, sorry about that

Do you perhaps have a test case or error that I can look at? Perhaps I or 
someone else here can help out a bit here.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61717



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

[PATCH] D61717: Fix arm_neon.h to be clean under -fno-lax-vector-conversions.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith abandoned this revision.
rsmith added a comment.

This patch doesn't work, and the ARM NEON intrinsic header emitter is too 
opaque and mysterious for me to be able to fix this, and it's not even clear 
that the generator knows what the actual parameter types of the compiler 
builtins are. For now I think the best I can do is to leave ARM targets with 
lax vector conversions enabled, and disable it for all the other targets. :-(

(It's also completely ridiculous that we're generating a 63K LoC (2.1MB) header 
file for this; our approach to intrinsics headers is unreasonable in the 
extreme. But I digress.)


Repository:
  rC Clang

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

https://reviews.llvm.org/D61717



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

[PATCH] D61717: Fix arm_neon.h to be clean under -fno-lax-vector-conversions.

[Sjoerd Meijer via Phabricator via cfe-commits]

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

Looks okay to me.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61717



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

[PATCH] D61717: Fix arm_neon.h to be clean under -fno-lax-vector-conversions.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith updated this revision to Diff 198762.
rsmith added a comment.

Remove unneeded test change.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61717

Files:
  test/Headers/arm-neon-header.c
  utils/TableGen/NeonEmitter.cpp


Index: utils/TableGen/NeonEmitter.cpp
===
--- utils/TableGen/NeonEmitter.cpp
+++ utils/TableGen/NeonEmitter.cpp
@@ -1430,7 +1430,7 @@
 }
 
 // Check if an explicit cast is needed.
-if (CastToType.isVector()) {
+if (CastToType.isVector() && (LocalCK == ClassB || CastToType.isChar())) {
   CastToType.makeInteger(8, true);
   Arg = "(" + CastToType.str() + ")" + Arg;
 }
Index: test/Headers/arm-neon-header.c
===
--- test/Headers/arm-neon-header.c
+++ test/Headers/arm-neon-header.c
@@ -2,6 +2,9 @@
 // RUN: %clang_cc1 -triple thumbv7-apple-darwin10 -target-cpu cortex-a8 
-fsyntax-only -fno-lax-vector-conversions -ffreestanding %s
 // RUN: %clang_cc1 -x c++ -triple thumbv7-apple-darwin10 -target-cpu cortex-a8 
-fsyntax-only -Wvector-conversions -ffreestanding %s
 
+// RUN: %clang_cc1 -triple arm64-linux-gnu -fallow-half-arguments-and-returns 
-target-feature +neon -target-feature +fullfp16 -ffreestanding -fsyntax-only %s 
-fno-lax-vector-conversions
+// RUN: %clang_cc1 -triple aarch64-linux-gnu 
-fallow-half-arguments-and-returns -target-feature +fullfp16 -target-feature 
+neon -ffreestanding -fsyntax-only %s -fno-lax-vector-conversions
+
 // RUN: %clang -fsyntax-only   -ffreestanding 
--target=aarch64-none-eabi -march=armv8.2-a+fp16 -std=c89 -xc %s
 // RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding 
--target=aarch64-none-eabi -march=armv8.2-a+fp16 -std=c99 -xc %s
 // RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding 
--target=aarch64-none-eabi -march=armv8.2-a+fp16 -std=c11 -xc %s


Index: utils/TableGen/NeonEmitter.cpp
===
--- utils/TableGen/NeonEmitter.cpp
+++ utils/TableGen/NeonEmitter.cpp
@@ -1430,7 +1430,7 @@
 }
 
 // Check if an explicit cast is needed.
-if (CastToType.isVector()) {
+if (CastToType.isVector() && (LocalCK == ClassB || CastToType.isChar())) {
   CastToType.makeInteger(8, true);
   Arg = "(" + CastToType.str() + ")" + Arg;
 }
Index: test/Headers/arm-neon-header.c
===
--- test/Headers/arm-neon-header.c
+++ test/Headers/arm-neon-header.c
@@ -2,6 +2,9 @@
 // RUN: %clang_cc1 -triple thumbv7-apple-darwin10 -target-cpu cortex-a8 -fsyntax-only -fno-lax-vector-conversions -ffreestanding %s
 // RUN: %clang_cc1 -x c++ -triple thumbv7-apple-darwin10 -target-cpu cortex-a8 -fsyntax-only -Wvector-conversions -ffreestanding %s
 
+// RUN: %clang_cc1 -triple arm64-linux-gnu -fallow-half-arguments-and-returns -target-feature +neon -target-feature +fullfp16 -ffreestanding -fsyntax-only %s -fno-lax-vector-conversions
+// RUN: %clang_cc1 -triple aarch64-linux-gnu -fallow-half-arguments-and-returns -target-feature +fullfp16 -target-feature +neon -ffreestanding -fsyntax-only %s -fno-lax-vector-conversions
+
 // RUN: %clang -fsyntax-only   -ffreestanding --target=aarch64-none-eabi -march=armv8.2-a+fp16 -std=c89 -xc %s
 // RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-none-eabi -march=armv8.2-a+fp16 -std=c99 -xc %s
 // RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding --target=aarch64-none-eabi -march=armv8.2-a+fp16 -std=c11 -xc %s
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61717: Fix arm_neon.h to be clean under -fno-lax-vector-conversions.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith created this revision.
rsmith added reviewers: t.p.northover, javed.absar, SjoerdMeijer, pbarrio.
Herald added a subscriber: kristof.beyls.
Herald added a project: clang.

arm_neon.h produces a large number of errors under -fno-lax-vector-conversions,
which disables some deeply dangerous implicit conversions and which we really
need to make the default (PR17164).

This patch fixes the immediate problem by removing a bunch of incorrect casts
generated in the header. But it's only the first part of the problem: the other
part is fixing the incorrect expectations in the corresponding CodeGen test
files (in particular, in two ~20K line test files). However, I don't want to
touch that until I'm confident this change is correct.


Repository:
  rC Clang

https://reviews.llvm.org/D61717

Files:
  test/Headers/arm-neon-header.c
  test/Sema/aarch64-neon-fp16-ranges.c
  utils/TableGen/NeonEmitter.cpp


Index: utils/TableGen/NeonEmitter.cpp
===
--- utils/TableGen/NeonEmitter.cpp
+++ utils/TableGen/NeonEmitter.cpp
@@ -1430,7 +1430,7 @@
 }
 
 // Check if an explicit cast is needed.
-if (CastToType.isVector()) {
+if (CastToType.isVector() && (LocalCK == ClassB || CastToType.isChar())) {
   CastToType.makeInteger(8, true);
   Arg = "(" + CastToType.str() + ")" + Arg;
 }
Index: test/Sema/aarch64-neon-fp16-ranges.c
===
--- test/Sema/aarch64-neon-fp16-ranges.c
+++ test/Sema/aarch64-neon-fp16-ranges.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -triple arm64-linux-gnu -fallow-half-arguments-and-returns 
-target-feature +neon -target-feature +fullfp16 -ffreestanding -fsyntax-only 
-verify %s
-// RUN: %clang_cc1 -triple aarch64-linux-gnu 
-fallow-half-arguments-and-returns -target-feature +fullfp16 -target-feature 
+neon -ffreestanding -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple arm64-linux-gnu -fallow-half-arguments-and-returns 
-target-feature +neon -target-feature +fullfp16 -ffreestanding -fsyntax-only 
-verify %s -fno-lax-vector-conversions
+// RUN: %clang_cc1 -triple aarch64-linux-gnu 
-fallow-half-arguments-and-returns -target-feature +fullfp16 -target-feature 
+neon -ffreestanding -fsyntax-only -verify %s -fno-lax-vector-conversions
 
 #include 
 #include 
Index: test/Headers/arm-neon-header.c
===
--- test/Headers/arm-neon-header.c
+++ test/Headers/arm-neon-header.c
@@ -2,6 +2,9 @@
 // RUN: %clang_cc1 -triple thumbv7-apple-darwin10 -target-cpu cortex-a8 
-fsyntax-only -fno-lax-vector-conversions -ffreestanding %s
 // RUN: %clang_cc1 -x c++ -triple thumbv7-apple-darwin10 -target-cpu cortex-a8 
-fsyntax-only -Wvector-conversions -ffreestanding %s
 
+// RUN: %clang_cc1 -triple arm64-linux-gnu -fallow-half-arguments-and-returns 
-target-feature +neon -target-feature +fullfp16 -ffreestanding -fsyntax-only %s 
-fno-lax-vector-conversions
+// RUN: %clang_cc1 -triple aarch64-linux-gnu 
-fallow-half-arguments-and-returns -target-feature +fullfp16 -target-feature 
+neon -ffreestanding -fsyntax-only %s -fno-lax-vector-conversions
+
 // RUN: %clang -fsyntax-only   -ffreestanding 
--target=aarch64-none-eabi -march=armv8.2-a+fp16 -std=c89 -xc %s
 // RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding 
--target=aarch64-none-eabi -march=armv8.2-a+fp16 -std=c99 -xc %s
 // RUN: %clang -fsyntax-only -Wall -Werror -ffreestanding 
--target=aarch64-none-eabi -march=armv8.2-a+fp16 -std=c11 -xc %s


Index: utils/TableGen/NeonEmitter.cpp
===
--- utils/TableGen/NeonEmitter.cpp
+++ utils/TableGen/NeonEmitter.cpp
@@ -1430,7 +1430,7 @@
 }
 
 // Check if an explicit cast is needed.
-if (CastToType.isVector()) {
+if (CastToType.isVector() && (LocalCK == ClassB || CastToType.isChar())) {
   CastToType.makeInteger(8, true);
   Arg = "(" + CastToType.str() + ")" + Arg;
 }
Index: test/Sema/aarch64-neon-fp16-ranges.c
===
--- test/Sema/aarch64-neon-fp16-ranges.c
+++ test/Sema/aarch64-neon-fp16-ranges.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -triple arm64-linux-gnu -fallow-half-arguments-and-returns -target-feature +neon -target-feature +fullfp16 -ffreestanding -fsyntax-only -verify %s
-// RUN: %clang_cc1 -triple aarch64-linux-gnu -fallow-half-arguments-and-returns -target-feature +fullfp16 -target-feature +neon -ffreestanding -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple arm64-linux-gnu -fallow-half-arguments-and-returns -target-feature +neon -target-feature +fullfp16 -ffreestanding -fsyntax-only -verify %s -fno-lax-vector-conversions
+// RUN: %clang_cc1 -triple aarch64-linux-gnu -fallow-half-arguments-and-returns -target-feature +fullfp16 -target-feature +neon -ffreestanding -fsyntax-only -verify %s -fno-lax-vector-conversions
 
 #include