[PATCH] D74847: [CodeGen][RISCV] Fix clang/test/CodeGen/atomic_ops.c for RISC-V

[Luís Marques via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0781e93a6eaa: [CodeGen][RISCV] Fix 
clang/test/CodeGen/atomic_ops.c for RISC-V (authored by luismarques).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74847

Files:
  clang/test/CodeGen/atomic_ops.c


Index: clang/test/CodeGen/atomic_ops.c
===
--- clang/test/CodeGen/atomic_ops.c
+++ clang/test/CodeGen/atomic_ops.c
@@ -1,7 +1,7 @@
-// XFAIL: hexagon,sparc
-//(due to not having native load atomic support)
-// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
-// RUN: %clang_cc1 -triple mips-linux-gnu -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64 -emit-llvm %s \
+// RUN:   -o - | FileCheck -check-prefixes=CHECK,NATIVE %s
+// RUN: %clang_cc1 -triple riscv32 -target-feature -a -emit-llvm %s \
+// RUN:   -o - | FileCheck -check-prefixes=CHECK,LIBCALL %s
 
 void foo(int x)
 {
@@ -9,32 +9,47 @@
   _Atomic(short) j = 0;
   // Check that multiply / divides on atomics produce a cmpxchg loop
   i *= 2;
-  // CHECK: mul nsw i32
-  // CHECK: {{(cmpxchg i32*|i1 @__atomic_compare_exchange\(i32 4,)}}
+  // NATIVE: mul nsw i32
+  // NATIVE: cmpxchg i32*
+  // LIBCALL: mul nsw i32
+  // LIBCALL: i1 @__atomic_compare_exchange(i32 4,
   i /= 2;
-  // CHECK: sdiv i32
-  // CHECK: {{(cmpxchg i32*|i1 @__atomic_compare_exchange\(i32 4, )}}
+  // NATIVE: sdiv i32
+  // NATIVE: cmpxchg i32*
+  // LIBCALL: sdiv i32
+  // LIBCALL: i1 @__atomic_compare_exchange(i32 4,
   j /= x;
-  // CHECK: sdiv i32
-  // CHECK: {{(cmpxchg i16*|i1 @__atomic_compare_exchange\(i32 2, )}}
+  // NATIVE: sdiv i32
+  // NATIVE: cmpxchg i16*
+  // LIBCALL: sdiv i32
+  // LIBCALL: i1 @__atomic_compare_exchange(i32 2,
 
 }
 
 extern _Atomic _Bool b;
 
 _Bool bar() {
-// CHECK-LABEL: @bar
-// CHECK: %[[load:.*]] = load atomic i8, i8* @b seq_cst
-// CHECK: %[[tobool:.*]] = trunc i8 %[[load]] to i1
-// CHECK: ret i1 %[[tobool]]
+// NATIVE-LABEL: @bar
+// NATIVE: %[[load:.*]] = load atomic i8, i8* @b seq_cst
+// NATIVE: %[[tobool:.*]] = trunc i8 %[[load]] to i1
+// NATIVE: ret i1 %[[tobool]]
+// LIBCALL-LABEL: @bar
+// LIBCALL: call void @__atomic_load(i32 1, i8* @b, i8* %atomic-temp, i32 5)
+// LIBCALL: %[[load:.*]] = load i8, i8* %atomic-temp
+// LIBCALL: %[[tobool:.*]] = trunc i8 %[[load]] to i1
+// LIBCALL: ret i1 %[[tobool]]
+
   return b;
 }
 
 extern _Atomic(_Complex int) x;
 
 void baz(int y) {
-// CHECK-LABEL: @baz
-// CHECK: {{store atomic|call void @__atomic_store}}
+// NATIVE-LABEL: @baz
+// NATIVE: store atomic
+// LIBCALL-LABEL: @baz
+// LIBCALL: call void @__atomic_store
+
   x += y;
 }
 
@@ -84,9 +99,11 @@
 }
 
 _Atomic(int) compound_mul(_Atomic(int) in) {
-// CHECK-LABEL: @compound_mul
-// CHECK: cmpxchg i32* {{%.*}}, i32 {{%.*}}, i32 [[NEW:%.*]] seq_cst seq_cst
-// CHECK: ret i32 [[NEW]]
+// NATIVE-LABEL: @compound_mul
+// NATIVE: cmpxchg i32* {{%.*}}, i32 {{%.*}}, i32 [[NEW:%.*]] seq_cst seq_cst
+// NATIVE: ret i32 [[NEW]]
+// LIBCALL-LABEL: @compound_mul
+// LIBCALL: i1 @__atomic_compare_exchange(i32 4,
 
   return (in *= 5);
 }


Index: clang/test/CodeGen/atomic_ops.c
===
--- clang/test/CodeGen/atomic_ops.c
+++ clang/test/CodeGen/atomic_ops.c
@@ -1,7 +1,7 @@
-// XFAIL: hexagon,sparc
-//(due to not having native load atomic support)
-// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
-// RUN: %clang_cc1 -triple mips-linux-gnu -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64 -emit-llvm %s \
+// RUN:   -o - | FileCheck -check-prefixes=CHECK,NATIVE %s
+// RUN: %clang_cc1 -triple riscv32 -target-feature -a -emit-llvm %s \
+// RUN:   -o - | FileCheck -check-prefixes=CHECK,LIBCALL %s
 
 void foo(int x)
 {
@@ -9,32 +9,47 @@
   _Atomic(short) j = 0;
   // Check that multiply / divides on atomics produce a cmpxchg loop
   i *= 2;
-  // CHECK: mul nsw i32
-  // CHECK: {{(cmpxchg i32*|i1 @__atomic_compare_exchange\(i32 4,)}}
+  // NATIVE: mul nsw i32
+  // NATIVE: cmpxchg i32*
+  // LIBCALL: mul nsw i32
+  // LIBCALL: i1 @__atomic_compare_exchange(i32 4,
   i /= 2;
-  // CHECK: sdiv i32
-  // CHECK: {{(cmpxchg i32*|i1 @__atomic_compare_exchange\(i32 4, )}}
+  // NATIVE: sdiv i32
+  // NATIVE: cmpxchg i32*
+  // LIBCALL: sdiv i32
+  // LIBCALL: i1 @__atomic_compare_exchange(i32 4,
   j /= x;
-  // CHECK: sdiv i32
-  // CHECK: {{(cmpxchg i16*|i1 @__atomic_compare_exchange\(i32 2, )}}
+  // NATIVE: sdiv i32
+  // NATIVE: cmpxchg i16*
+  // LIBCALL: sdiv i32
+  // LIBCALL: i1 @__atomic_compare_exchange(i32 2,
 
 }
 
 extern _Atomic _Bool b;
 
 _Bool bar() {
-// CHECK-LABEL: @bar
-// CHECK: %[[load:.*]] = load atomic i8, i8* @b seq_cst
-// CHECK: %[[tobool:.*]] = trunc i8 %[[load]] to i1
-// CHECK: ret i1 %[[tobool]]
+// NATIVE-LABEL: @bar
+// NATIVE: %[[load:.*]] = load atomic i8, i8* @b seq_cst

[PATCH] D74847: [CodeGen][RISCV] Fix clang/test/CodeGen/atomic_ops.c for RISC-V

[Eli Friedman via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74847



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

[PATCH] D74847: [CodeGen][RISCV] Fix clang/test/CodeGen/atomic_ops.c for RISC-V

[Luís Marques via Phabricator via cfe-commits]

luismarques updated this revision to Diff 245881.
luismarques edited the summary of this revision.
luismarques added a comment.
Herald added a subscriber: fedor.sergeev.

As suggested by @efriedma, the patch was reworked to have one target with 
native atomics, and one without. No RUN run with a default target remains.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74847

Files:
  clang/test/CodeGen/atomic_ops.c


Index: clang/test/CodeGen/atomic_ops.c
===
--- clang/test/CodeGen/atomic_ops.c
+++ clang/test/CodeGen/atomic_ops.c
@@ -1,7 +1,7 @@
-// XFAIL: hexagon,sparc
-//(due to not having native load atomic support)
-// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
-// RUN: %clang_cc1 -triple mips-linux-gnu -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64 -emit-llvm %s \
+// RUN:   -o - | FileCheck -check-prefixes=CHECK,NATIVE %s
+// RUN: %clang_cc1 -triple riscv32 -target-feature -a -emit-llvm %s \
+// RUN:   -o - | FileCheck -check-prefixes=CHECK,LIBCALL %s
 
 void foo(int x)
 {
@@ -9,32 +9,47 @@
   _Atomic(short) j = 0;
   // Check that multiply / divides on atomics produce a cmpxchg loop
   i *= 2;
-  // CHECK: mul nsw i32
-  // CHECK: {{(cmpxchg i32*|i1 @__atomic_compare_exchange\(i32 4,)}}
+  // NATIVE: mul nsw i32
+  // NATIVE: cmpxchg i32*
+  // LIBCALL: mul nsw i32
+  // LIBCALL: i1 @__atomic_compare_exchange(i32 4,
   i /= 2;
-  // CHECK: sdiv i32
-  // CHECK: {{(cmpxchg i32*|i1 @__atomic_compare_exchange\(i32 4, )}}
+  // NATIVE: sdiv i32
+  // NATIVE: cmpxchg i32*
+  // LIBCALL: sdiv i32
+  // LIBCALL: i1 @__atomic_compare_exchange(i32 4,
   j /= x;
-  // CHECK: sdiv i32
-  // CHECK: {{(cmpxchg i16*|i1 @__atomic_compare_exchange\(i32 2, )}}
+  // NATIVE: sdiv i32
+  // NATIVE: cmpxchg i16*
+  // LIBCALL: sdiv i32
+  // LIBCALL: i1 @__atomic_compare_exchange(i32 2,
 
 }
 
 extern _Atomic _Bool b;
 
 _Bool bar() {
-// CHECK-LABEL: @bar
-// CHECK: %[[load:.*]] = load atomic i8, i8* @b seq_cst
-// CHECK: %[[tobool:.*]] = trunc i8 %[[load]] to i1
-// CHECK: ret i1 %[[tobool]]
+// NATIVE-LABEL: @bar
+// NATIVE: %[[load:.*]] = load atomic i8, i8* @b seq_cst
+// NATIVE: %[[tobool:.*]] = trunc i8 %[[load]] to i1
+// NATIVE: ret i1 %[[tobool]]
+// LIBCALL-LABEL: @bar
+// LIBCALL: call void @__atomic_load(i32 1, i8* @b, i8* %atomic-temp, i32 5)
+// LIBCALL: %[[load:.*]] = load i8, i8* %atomic-temp
+// LIBCALL: %[[tobool:.*]] = trunc i8 %[[load]] to i1
+// LIBCALL: ret i1 %[[tobool]]
+
   return b;
 }
 
 extern _Atomic(_Complex int) x;
 
 void baz(int y) {
-// CHECK-LABEL: @baz
-// CHECK: {{store atomic|call void @__atomic_store}}
+// NATIVE-LABEL: @baz
+// NATIVE: store atomic
+// LIBCALL-LABEL: @baz
+// LIBCALL: call void @__atomic_store
+
   x += y;
 }
 
@@ -84,9 +99,11 @@
 }
 
 _Atomic(int) compound_mul(_Atomic(int) in) {
-// CHECK-LABEL: @compound_mul
-// CHECK: cmpxchg i32* {{%.*}}, i32 {{%.*}}, i32 [[NEW:%.*]] seq_cst seq_cst
-// CHECK: ret i32 [[NEW]]
+// NATIVE-LABEL: @compound_mul
+// NATIVE: cmpxchg i32* {{%.*}}, i32 {{%.*}}, i32 [[NEW:%.*]] seq_cst seq_cst
+// NATIVE: ret i32 [[NEW]]
+// LIBCALL-LABEL: @compound_mul
+// LIBCALL: i1 @__atomic_compare_exchange(i32 4,
 
   return (in *= 5);
 }


Index: clang/test/CodeGen/atomic_ops.c
===
--- clang/test/CodeGen/atomic_ops.c
+++ clang/test/CodeGen/atomic_ops.c
@@ -1,7 +1,7 @@
-// XFAIL: hexagon,sparc
-//(due to not having native load atomic support)
-// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
-// RUN: %clang_cc1 -triple mips-linux-gnu -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64 -emit-llvm %s \
+// RUN:   -o - | FileCheck -check-prefixes=CHECK,NATIVE %s
+// RUN: %clang_cc1 -triple riscv32 -target-feature -a -emit-llvm %s \
+// RUN:   -o - | FileCheck -check-prefixes=CHECK,LIBCALL %s
 
 void foo(int x)
 {
@@ -9,32 +9,47 @@
   _Atomic(short) j = 0;
   // Check that multiply / divides on atomics produce a cmpxchg loop
   i *= 2;
-  // CHECK: mul nsw i32
-  // CHECK: {{(cmpxchg i32*|i1 @__atomic_compare_exchange\(i32 4,)}}
+  // NATIVE: mul nsw i32
+  // NATIVE: cmpxchg i32*
+  // LIBCALL: mul nsw i32
+  // LIBCALL: i1 @__atomic_compare_exchange(i32 4,
   i /= 2;
-  // CHECK: sdiv i32
-  // CHECK: {{(cmpxchg i32*|i1 @__atomic_compare_exchange\(i32 4, )}}
+  // NATIVE: sdiv i32
+  // NATIVE: cmpxchg i32*
+  // LIBCALL: sdiv i32
+  // LIBCALL: i1 @__atomic_compare_exchange(i32 4,
   j /= x;
-  // CHECK: sdiv i32
-  // CHECK: {{(cmpxchg i16*|i1 @__atomic_compare_exchange\(i32 2, )}}
+  // NATIVE: sdiv i32
+  // NATIVE: cmpxchg i16*
+  // LIBCALL: sdiv i32
+  // LIBCALL: i1 @__atomic_compare_exchange(i32 2,
 
 }
 
 extern _Atomic _Bool b;
 
 _Bool bar() {
-// CHECK-LABEL: @bar
-// CHECK: %[[load:.*]] = load atomic i8, i8* @b seq_cst
-// CHECK: %[[tobool:.*]] = trunc i8 

[PATCH] D74847: [CodeGen][RISCV] Fix clang/test/CodeGen/atomic_ops.c for RISC-V

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Yes, makes sense


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74847



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

[PATCH] D74847: [CodeGen][RISCV] Fix clang/test/CodeGen/atomic_ops.c for RISC-V

[Luís Marques via Phabricator via cfe-commits]

luismarques added a comment.

In D74847#1883028 , @efriedma wrote:

> I'm not really a big fan of running tests with the host target triple, 
> anyway; it seems to create work with almost no benefit.  I'd be happy to just 
> run the test with one target that has native atomics, and one target that 
> doesn't.  (The relevant code is all target-independent, aside from the atomic 
> widths, so we aren't really gaining anything by testing more targets.)


That sounds fine to me. I might need to rewrite the tests with two different 
prefixes though (e.g. `NATIVE-` vs `LIBCALL-`), since I can't see how to mix 
substitution blocks with regex alternatives. For instance, we have this:

  // CHECK: %[[load:.*]] = {{load atomic i8, i8\* @b seq_cst|}}

You need to add an alternative for an `__atomic_load` libcall instead of the 
`load atomic`, but then the "assignment" to `%atomic-temp` moves to the right, 
as an out argument, so you'd need the variable capture to move to inside the 
regex, which I don't think is supported.
Is that OK, using different prefixes? Arguably that's a better test anyway, 
since you can then check your expectation that certain targets triples match 
certain alternative matches.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74847



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

[PATCH] D74847: [CodeGen][RISCV] Fix clang/test/CodeGen/atomic_ops.c for RISC-V

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

On a side-note, we could enhance the test runner to support "XFAIL: 
riscv32-default-target" if we thought it would be useful.  But again, I'm not a 
fan of tests that depend on the default target in the first place.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74847



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

[PATCH] D74847: [CodeGen][RISCV] Fix clang/test/CodeGen/atomic_ops.c for RISC-V

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

I'm not really a big fan of running tests with the host target triple, anyway; 
it seems to create work with almost no benefit.  I'd be happy to just run the 
test with one target that has native atomics, and one target that doesn't.  
(The relevant code is all target-independent, aside from the atomic widths, so 
we aren't really gaining anything by testing more targets.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74847



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

[PATCH] D74847: [CodeGen][RISCV] Fix clang/test/CodeGen/atomic_ops.c for RISC-V

[Luís Marques via Phabricator via cfe-commits]

luismarques created this revision.
luismarques added reviewers: jyknight, eli.friedman, lenary.
Herald added subscribers: cfe-commits, evandro, sameer.abuasal, s.egerton, Jim, 
benna, psnobl, PkmX, jfb, rkruppe, rogfer01, shiva0217, kito-cheng, simoncook.
Herald added a project: clang.

By default the RISC-V target doesn't have the atomics standard extension 
enabled. The first RUN line in `clang/test/CodeGen/atomic_ops.c` doesn't 
specify a target triple, which means that on RISC-V Linux hosts it will target 
RISC-V, but because we use clang cc1 we don't get the toolchain driver 
functionality to automatically turn on the extensions implied by the target 
triple (riscv64-linux includes atomics). This causes the test to fail on RISC-V 
hosts.

I waffled a bit regarding the best way to fix this. In the end I decided to 1) 
use XFAIL to blacklist RISC-V hosts and 2) add explicitly run lines for 
riscv32/64, with the atomics extension enabled. With that choice we 
//effectively// don't do the test on RISC-V hosts, but still get to test those 
two RISC-V targets on other hosts.

An alternative approach would be to eliminate the first RUN line and explicitly 
list all of the target triples we want to test. But looking at the other clang 
tests, there doesn't seem to be any test that comprehensively lists all of the 
targets, and it wasn't quite clear which should be listed on this test. Running 
clang with the non-cc1 interface would also be an option, but in principle that 
could still fail in some hypothetical scenarios, and the tests seem to prefer 
to use cc1.

(It's a shame that XFAIL necessarily means blacklisting host triples, and not 
target triples. That doesn't interact very well with cross-compilers.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D74847

Files:
  clang/test/CodeGen/atomic_ops.c


Index: clang/test/CodeGen/atomic_ops.c
===
--- clang/test/CodeGen/atomic_ops.c
+++ clang/test/CodeGen/atomic_ops.c
@@ -1,7 +1,13 @@
 // XFAIL: hexagon,sparc
 //(due to not having native load atomic support)
+// XFAIL: riscv
+//(due to requiring an explicit -target-feature +a)
 // RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
 // RUN: %clang_cc1 -triple mips-linux-gnu -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple riscv64 -target-feature +a -emit-llvm %s \
+// RUN:   -o - | FileCheck %s
+// RUN: %clang_cc1 -triple riscv32 -target-feature +a -emit-llvm %s \
+// RUN:   -o - | FileCheck %s
 
 void foo(int x)
 {


Index: clang/test/CodeGen/atomic_ops.c
===
--- clang/test/CodeGen/atomic_ops.c
+++ clang/test/CodeGen/atomic_ops.c
@@ -1,7 +1,13 @@
 // XFAIL: hexagon,sparc
 //(due to not having native load atomic support)
+// XFAIL: riscv
+//(due to requiring an explicit -target-feature +a)
 // RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s
 // RUN: %clang_cc1 -triple mips-linux-gnu -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple riscv64 -target-feature +a -emit-llvm %s \
+// RUN:   -o - | FileCheck %s
+// RUN: %clang_cc1 -triple riscv32 -target-feature +a -emit-llvm %s \
+// RUN:   -o - | FileCheck %s
 
 void foo(int x)
 {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits