[PATCH] D118259: [AArch64] Adjust aarch64-neon-intrinsics-constrained test and un-XFAIL

2022-03-17 Thread Kevin P. Neal via Phabricator via cfe-commits 
kpn added a comment.

In D118259#3389246 , @fhahn wrote:

> In D118259#3389235 , @kpn wrote:
>
>> It's been a while, but I think the aarch64-neon-intrinsics-constrained.c 
>> test is trimmed down from the aarch64-neon-intrinsics.c test. Shouldn't the 
>> constrained and non-constrained end-to-end tests be treated the same?
>
> Are you referring to 
> https://github.com/llvm/llvm-project/blob/main/clang/test/CodeGen/aarch64-neon-intrinsics.c?
>  This one doesn't check assembly.

Ah, well, I guess that wasn't it, then. It's been a while, and I've been all 
over the place.

A clang-only test that just tests for the correct IR being emitted won't 
trigger the failures in the backend. An LLVM-only test that starts from that IR 
can show those failures, but it also runs the risk over time of getting out of 
sync with the IR emitted by clang. An end-to-end test will always be in sync, 
and it will show those failures that I ran into plus anything else that crops 
up over time. If there's another way, or if there is another approach, then it 
might be possible there's a way to get all the testing done without running 
into those issues. I just don't know what it is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118259

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

[PATCH] D118259: [AArch64] Adjust aarch64-neon-intrinsics-constrained test and un-XFAIL

2022-03-17 Thread Florian Hahn via Phabricator via cfe-commits 
fhahn added a comment.

In D118259#3389235 , @kpn wrote:

> It's been a while, but I think the aarch64-neon-intrinsics-constrained.c test 
> is trimmed down from the aarch64-neon-intrinsics.c test. Shouldn't the 
> constrained and non-constrained end-to-end tests be treated the same?

Are you referring to 
https://github.com/llvm/llvm-project/blob/main/clang/test/CodeGen/aarch64-neon-intrinsics.c?
 This one doesn't check assembly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118259

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

[PATCH] D118259: [AArch64] Adjust aarch64-neon-intrinsics-constrained test and un-XFAIL

2022-03-17 Thread Kevin P. Neal via Phabricator via cfe-commits 
kpn added a comment.

It's been a while, but I think the aarch64-neon-intrinsics-constrained.c test 
is trimmed down from the aarch64-neon-intrinsics.c test. Shouldn't the 
constrained and non-constrained end-to-end tests be treated the same?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118259

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

[PATCH] D118259: [AArch64] Adjust aarch64-neon-intrinsics-constrained test and un-XFAIL

2022-03-17 Thread John Brawn via Phabricator via cfe-commits 
john.brawn added a comment.

llvm/test/CodeGen/AArch64/fp-intrinsics-vector.ll is testing the asm 
generation, so it probably is fine to remove that here so I will. This test is 
specifically testing arm_neon.h intrinsics though, so wouldn't make sense 
unifying with other architectures.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118259

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

[PATCH] D118259: [AArch64] Adjust aarch64-neon-intrinsics-constrained test and un-XFAIL

2022-03-17 Thread Florian Hahn via Phabricator via cfe-commits 
fhahn added a comment.

In D118259#3343554 , @john.brawn 
wrote:

> In D118259#3275297 , @fhahn wrote:
>
>> Does this clang test actually need to check the generated assembly? 
>> Shouldn't it be enough to check that the correct intrinsics are generated?
>
> I could remove the CHECK-ASM checks, but the other xyz-constrained.c tests 
> (including non-aarch64 ones) all do this and it feels a bit weird to just do 
> that to this test.

I guess the question is if there is a convincing reason to check the assembly 
in this test? Unless it is required for the test, checking for assembly makes 
the test more fragile than they need to be. Also, is there any difference in 
the generated IR between X86 and AArch64? If not, the tests should probably be 
unified.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118259

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

[PATCH] D118259: [AArch64] Adjust aarch64-neon-intrinsics-constrained test and un-XFAIL

2022-03-17 Thread John Brawn via Phabricator via cfe-commits 
john.brawn added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118259

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

[PATCH] D118259: [AArch64] Adjust aarch64-neon-intrinsics-constrained test and un-XFAIL

2022-03-07 Thread John Brawn via Phabricator via cfe-commits 
john.brawn added a comment.
Herald added a project: All.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118259

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

[PATCH] D118259: [AArch64] Adjust aarch64-neon-intrinsics-constrained test and un-XFAIL

2022-02-24 Thread John Brawn via Phabricator via cfe-commits 
john.brawn added a comment.

In D118259#3275297 , @fhahn wrote:

> Does this clang test actually need to check the generated assembly? Shouldn't 
> it be enough to check that the correct intrinsics are generated?

I could remove the CHECK-ASM checks, but the other xyz-constrained.c tests 
(including non-aarch64 ones) all do this and it feels a bit weird to just do 
that to this test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118259

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

[PATCH] D118259: [AArch64] Adjust aarch64-neon-intrinsics-constrained test and un-XFAIL

2022-01-27 Thread Florian Hahn via Phabricator via cfe-commits 
fhahn added a comment.

Does this clang test actually need to check the generated assembly? Shouldn't 
it be enough to check that the correct intrinsics are generated?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118259

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

[PATCH] D118259: [AArch64] Adjust aarch64-neon-intrinsics-constrained test and un-XFAIL

2022-01-26 Thread John Brawn via Phabricator via cfe-commits 
john.brawn created this revision.
john.brawn added reviewers: kpn, cameron.mcinally, dmgreen, t.p.northover.
Herald added a subscriber: kristof.beyls.
john.brawn requested review of this revision.
Herald added a project: clang.

This test no longer fails during isel, but does need the expected output to be 
adjusted to match the actual output.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118259

Files:
  clang/test/CodeGen/aarch64-neon-intrinsics-constrained.c

Index: clang/test/CodeGen/aarch64-neon-intrinsics-constrained.c
===
--- clang/test/CodeGen/aarch64-neon-intrinsics-constrained.c
+++ clang/test/CodeGen/aarch64-neon-intrinsics-constrained.c
@@ -1,27 +1,24 @@
 // RUN: %clang_cc1 -triple arm64-none-linux-gnu -target-feature +neon \
 // RUN: -fallow-half-arguments-and-returns -S -disable-O0-optnone \
 // RUN:  -flax-vector-conversions=none -emit-llvm -o - %s | opt -S -mem2reg \
-// RUN: | FileCheck --check-prefix=COMMON --check-prefix=COMMONIR --check-prefix=UNCONSTRAINED %s
+// RUN: | FileCheck --check-prefixes=COMMON,COMMONIR,UNCONSTRAINED %s
 // RUN: %clang_cc1 -triple arm64-none-linux-gnu -target-feature +neon \
 // RUN: -fallow-half-arguments-and-returns -S -disable-O0-optnone \
-// RUN:  -ffp-exception-behavior=strict \
+// RUN:  -ffp-exception-behavior=strict -fexperimental-strict-floating-point \
 // RUN:  -flax-vector-conversions=none -emit-llvm -o - %s | opt -S -mem2reg \
-// RUN: | FileCheck --check-prefix=COMMON --check-prefix=COMMONIR --check-prefix=CONSTRAINED %s
+// RUN: | FileCheck --check-prefixes=COMMON,COMMONIR,CONSTRAINED %s
 // RUN: %clang_cc1 -triple arm64-none-linux-gnu -target-feature +neon \
 // RUN: -fallow-half-arguments-and-returns -S -disable-O0-optnone \
 // RUN:  -flax-vector-conversions=none -o - %s \
-// RUN: | FileCheck --check-prefix=COMMON --check-prefix=CHECK-ASM %s
+// RUN: | FileCheck --check-prefixes=COMMON,CHECK-ASM,CHECK-ASM-UNCONSTRAINED %s
 // RUN: %clang_cc1 -triple arm64-none-linux-gnu -target-feature +neon \
 // RUN: -fallow-half-arguments-and-returns -S -disable-O0-optnone \
-// RUN:  -ffp-exception-behavior=strict \
+// RUN:  -ffp-exception-behavior=strict -fexperimental-strict-floating-point \
 // RUN:  -flax-vector-conversions=none -o - %s \
-// RUN: | FileCheck --check-prefix=COMMON --check-prefix=CHECK-ASM %s
+// RUN: | FileCheck --check-prefixes=COMMON,CHECK-ASM,CHECK-ASM-CONSTRAINED %s
 
 // REQUIRES: aarch64-registered-target
 
-// Fails during instruction selection:
-// XFAIL: *
-
 // Test new aarch64 intrinsics and types but constrained
 
 #include 
@@ -278,7 +275,9 @@
 // COMMON-LABEL: test_vceq_f32
 // UNCONSTRAINED: [[CMP_I:%.*]] = fcmp oeq <2 x float> %v1, %v2
 // CONSTRAINED:   [[CMP_I:%.*]] = call <2 x i1> @llvm.experimental.constrained.fcmp.v2f32(<2 x float> %v1, <2 x float> %v2, metadata !"oeq", metadata !"fpexcept.strict")
-// CHECK-ASM: fcmeq v{{[0-9]+}}.2s, v{{[0-9]+}}.2s, v{{[0-9]+}}.2s
+// CHECK-ASM-UNCONSTRAINED: fcmeq v{{[0-9]+}}.2s, v{{[0-9]+}}.2s, v{{[0-9]+}}.2s
+// CHECK-ASM-CONSTRAINED: fcmp s{{[0-9]+}}, s{{[0-9]+}}
+// CHECK-ASM-CONSTRAINED: fcmp s{{[0-9]+}}, s{{[0-9]+}}
 // COMMONIR:  [[SEXT_I:%.*]] = sext <2 x i1> [[CMP_I]] to <2 x i32>
 // COMMONIR:  ret <2 x i32> [[SEXT_I]]
 uint32x2_t test_vceq_f32(float32x2_t v1, float32x2_t v2) {
@@ -299,7 +298,11 @@
 // COMMON-LABEL: test_vceqq_f32
 // UNCONSTRAINED: [[CMP_I:%.*]] = fcmp oeq <4 x float> %v1, %v2
 // CONSTRAINED:   [[CMP_I:%.*]] = call <4 x i1> @llvm.experimental.constrained.fcmp.v4f32(<4 x float> %v1, <4 x float> %v2, metadata !"oeq", metadata !"fpexcept.strict")
-// CHECK-ASM: fcmeq v{{[0-9]+}}.4s, v{{[0-9]+}}.4s, v{{[0-9]+}}.4s
+// CHECK-ASM-UNCONSTRAINED: fcmeq v{{[0-9]+}}.4s, v{{[0-9]+}}.4s, v{{[0-9]+}}.4s
+// CHECK-ASM-CONSTRAINED: fcmp s{{[0-9]+}}, s{{[0-9]+}}
+// CHECK-ASM-CONSTRAINED: fcmp s{{[0-9]+}}, s{{[0-9]+}}
+// CHECK-ASM-CONSTRAINED: fcmp s{{[0-9]+}}, s{{[0-9]+}}
+// CHECK-ASM-CONSTRAINED: fcmp s{{[0-9]+}}, s{{[0-9]+}}
 // COMMONIR:  [[SEXT_I:%.*]] = sext <4 x i1> [[CMP_I]] to <4 x i32>
 // COMMONIR:  ret <4 x i32> [[SEXT_I]]
 uint32x4_t test_vceqq_f32(float32x4_t v1, float32x4_t v2) {
@@ -309,7 +312,9 @@
 // COMMON-LABEL: test_vceqq_f64
 // UNCONSTRAINED: [[CMP_I:%.*]] = fcmp oeq <2 x double> %v1, %v2
 // CONSTRAINED:   [[CMP_I:%.*]] = call <2 x i1> @llvm.experimental.constrained.fcmp.v2f64(<2 x double> %v1, <2 x double> %v2, metadata !"oeq", metadata !"fpexcept.strict")
-// CHECK-ASM: fcmeq v{{[0-9]+}}.2d, v{{[0-9]+}}.2d, v{{[0-9]+}}.2d
+// CHECK-ASM-UNCONSTRAINED: fcmeq v{{[0-9]+}}.2d, v{{[0-9]+}}.2d, v{{[0-9]+}}.2d
+// CHECK-ASM-CONSTRAINED: fcmp d{{[0-9]+}}, d{{[0-9]+}}
+// CHECK-ASM-CONSTRAINED: fcmp d{{[0-9]+}}, d{{[0-9]+}}
 // COMMONIR:  [[SEXT_I:%.*]] = sext <2 x i1> [[CMP_I]] to <2 x i64>
 // COMMONIR:  ret <2 x i64> [[SEXT_I]]
 uint64x2_t test_vceqq_f64(float64x2_t v1, float64x2_t v2) {
@@ -319,7 +324,9 @@