Message ID | 70538ps9-3o3o-26r5-9op-r9n1q318s52@fhfr.qr |
---|---|
State | New |
Headers | show |
Series | middle-end/101530 - fix shufflevector lowering | expand |
On 1/5/2022 7:17 AM, Richard Biener via Gcc-patches wrote: > This makes __builtin_shufflevector lowering force the result > of the BIT_FIELD_REF lowpart operation to a temporary as to > fulfil the IL verifier constraint that BIT_FIELD_REFs should > be always in outermost handled component position. Trying to > enforce this during gimplification isn't as straight-forward > as here where we know we're dealing with an rvalue. > > Bootstrap & regtest running on x86_64-unknown-linux-gnu - OK for trunk? > > Thanks, > Richard. > > 2022-01-05 Richard Biener <rguenther@suse.de> > > PR middle-end/101530 > gcc/c-family/ > * c-common.c (c_build_shufflevector): Wrap the BIT_FIELD_REF > in a TARGET_EXPR to force a temporary. > > gcc/testsuite/ > * c-c++-common/builtin-shufflevector-3.c: New testcase. Seems quite reasonable to me. jeff
On Wed, 5 Jan 2022, Jeff Law wrote: > > > On 1/5/2022 7:17 AM, Richard Biener via Gcc-patches wrote: > > This makes __builtin_shufflevector lowering force the result > > of the BIT_FIELD_REF lowpart operation to a temporary as to > > fulfil the IL verifier constraint that BIT_FIELD_REFs should > > be always in outermost handled component position. Trying to > > enforce this during gimplification isn't as straight-forward > > as here where we know we're dealing with an rvalue. > > > > Bootstrap & regtest running on x86_64-unknown-linux-gnu - OK for trunk? > > > > Thanks, > > Richard. > > > > 2022-01-05 Richard Biener <rguenther@suse.de> > > > > PR middle-end/101530 > > gcc/c-family/ > > * c-common.c (c_build_shufflevector): Wrap the BIT_FIELD_REF > > in a TARGET_EXPR to force a temporary. > > > > gcc/testsuite/ > > * c-c++-common/builtin-shufflevector-3.c: New testcase. > Seems quite reasonable to me. So I forgot to mark the TARGET_EXPR as having TREE_SIDE_EFFECTS which makes the C++ FE side elide the initialization after wrapping the TARGET_EXPR inside a COMPOUND_EXPR. Fixed as follows, re-bootstrapped and tested on x86_64-unknown-linux-gnu and pushed. Richard. From be59671c5624fe8bf21ddb0192e97ebdfa4db381 Mon Sep 17 00:00:00 2001 From: Richard Biener <rguenther@suse.de> Date: Wed, 5 Jan 2022 15:13:33 +0100 Subject: [PATCH] middle-end/101530 - fix shufflevector lowering To: gcc-patches@gcc.gnu.org This makes __builtin_shufflevector lowering force the result of the BIT_FIELD_REF lowpart operation to a temporary as to fulfil the IL verifier constraint that BIT_FIELD_REFs should be always in outermost handled component position. Trying to enforce this during gimplification isn't as straight-forward as here where we know we're dealing with an rvalue. FAIL: c-c++-common/torture/builtin-shufflevector-1.c -O0 execution test 2022-01-05 Richard Biener <rguenther@suse.de> PR middle-end/101530 gcc/c-family/ * c-common.c (c_build_shufflevector): Wrap the BIT_FIELD_REF in a TARGET_EXPR to force a temporary. gcc/testsuite/ * c-c++-common/builtin-shufflevector-3.c: New testcase. --- gcc/c-family/c-common.c | 7 +++++++ .../c-c++-common/builtin-shufflevector-3.c | 15 +++++++++++++++ 2 files changed, 22 insertions(+) create mode 100644 gcc/testsuite/c-c++-common/builtin-shufflevector-3.c diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c index 13341fa315e..4a6a4edb763 100644 --- a/gcc/c-family/c-common.c +++ b/gcc/c-family/c-common.c @@ -1243,6 +1243,13 @@ c_build_shufflevector (location_t loc, tree v0, tree v1, tree lpartt = build_vector_type (TREE_TYPE (ret_type), mask.length ()); ret = build3_loc (loc, BIT_FIELD_REF, lpartt, ret, TYPE_SIZE (lpartt), bitsize_zero_node); + /* Wrap the lowpart operation in a TARGET_EXPR so it gets a separate + temporary during gimplification. See PR101530 for cases where + we'd otherwise end up with non-toplevel BIT_FIELD_REFs. */ + tree tem = create_tmp_var_raw (lpartt); + DECL_CONTEXT (tem) = current_function_decl; + ret = build4 (TARGET_EXPR, lpartt, tem, ret, NULL_TREE, NULL_TREE); + TREE_SIDE_EFFECTS (ret) = 1; } if (!c_dialect_cxx () && !wrap) diff --git a/gcc/testsuite/c-c++-common/builtin-shufflevector-3.c b/gcc/testsuite/c-c++-common/builtin-shufflevector-3.c new file mode 100644 index 00000000000..0c9bda689ef --- /dev/null +++ b/gcc/testsuite/c-c++-common/builtin-shufflevector-3.c @@ -0,0 +1,15 @@ +/* { dg-do compile } */ + +typedef int __attribute__((__vector_size__ (sizeof(int)*4))) V; + +int +foo(V v, int i) +{ + return __builtin_shufflevector (v, v, 2, 3)[i]; +} + +int +bar(V v, int i) +{ + return __builtin_shufflevector(v, v, 4)[0] & i; +}
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c index 13341fa315e..64209303d54 100644 --- a/gcc/c-family/c-common.c +++ b/gcc/c-family/c-common.c @@ -1243,6 +1243,11 @@ c_build_shufflevector (location_t loc, tree v0, tree v1, tree lpartt = build_vector_type (TREE_TYPE (ret_type), mask.length ()); ret = build3_loc (loc, BIT_FIELD_REF, lpartt, ret, TYPE_SIZE (lpartt), bitsize_zero_node); + /* Wrap the lowpart operation in a TARGET_EXPR so it gets a separate + temporary during gimplification. See PR101530 for cases where + we'd otherwise end up with non-toplevel BIT_FIELD_REFs. */ + tree tem = create_tmp_var_raw (lpartt); + ret = build4 (TARGET_EXPR, lpartt, tem, ret, NULL_TREE, NULL_TREE); } if (!c_dialect_cxx () && !wrap) diff --git a/gcc/testsuite/c-c++-common/builtin-shufflevector-3.c b/gcc/testsuite/c-c++-common/builtin-shufflevector-3.c new file mode 100644 index 00000000000..0c9bda689ef --- /dev/null +++ b/gcc/testsuite/c-c++-common/builtin-shufflevector-3.c @@ -0,0 +1,15 @@ +/* { dg-do compile } */ + +typedef int __attribute__((__vector_size__ (sizeof(int)*4))) V; + +int +foo(V v, int i) +{ + return __builtin_shufflevector (v, v, 2, 3)[i]; +} + +int +bar(V v, int i) +{ + return __builtin_shufflevector(v, v, 4)[0] & i; +}