From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 127399 invoked by alias); 6 Dec 2017 14:40:09 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 127384 invoked by uid 89); 6 Dec 2017 14:40:08 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-10.9 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_2,GIT_PATCH_3,KAM_ASCII_DIVIDERS,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-wr0-f170.google.com Received: from mail-wr0-f170.google.com (HELO mail-wr0-f170.google.com) (209.85.128.170) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 06 Dec 2017 14:40:06 +0000 Received: by mail-wr0-f170.google.com with SMTP id s66so4149944wrc.9 for ; Wed, 06 Dec 2017 06:40:06 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:mail-followup-to:cc:subject:references :date:in-reply-to:message-id:user-agent:mime-version; bh=7eLcMXtxHNMuxg+YP7xepjGw7xdNTs72EvmeLeDlPiw=; b=Zwmnwkv8+nSzkd8Kiiq8qYCjYgJ4ztVKZES7rseRo+wdXHhZQYBpaeyf2ap9A3j/lc vamU5/FT5NRqNya62W0Bzcch7h/3xRdtX4Mn7xC6/37z/+utZZqd9SAmtPFUN7jNvX6G XAR/4a7l84LmqqEZCr2gurlam8qL+Qx5Ay1OzxKrBc9saUAnB5+/D6C+Cu7sKCkgtrMe I6KYmPiEsqhRw/QzFdI5GGVv0U/3VtE3KSnP+QkLcgA0f8UgMRR3zVIQTIhzQw+mMjtH nqXgHGpeP6nKoamnMfDGYl1IxBpRAbnoWIdaZknL4uGf1jrNdi6BQ2SO+swHcrjmDGae qkmw== X-Gm-Message-State: AJaThX5f9VR1e0QmXsyH7y9TOMMooj7oxvcTC9xVZk7F3iDCrQbysEtf a/gmffs2ZXLNFdxJD7QHo661pD39aRg= X-Google-Smtp-Source: AGs4zMYPflaBT2pFLKBYHD0b+o4u6HZXQtt3f8ktgKyIUCuliTBl9uwCB9CGnYc5ZrDuXvZkjEF2Mw== X-Received: by 10.223.199.82 with SMTP id b18mr19108240wrh.78.1512571204182; Wed, 06 Dec 2017 06:40:04 -0800 (PST) Received: from localhost (188.29.165.206.threembb.co.uk. [188.29.165.206]) by smtp.gmail.com with ESMTPSA id c19sm112239wmd.5.2017.12.06.06.40.02 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 06 Dec 2017 06:40:03 -0800 (PST) From: Richard Sandiford To: David Malcolm Mail-Followup-To: David Malcolm ,gcc-patches@gcc.gnu.org, richard.sandiford@linaro.org Cc: gcc-patches@gcc.gnu.org Subject: Re: RFC: Variable-length VECTOR_CSTs References: <87shcxl2ka.fsf@linaro.org> <1511963747.27881.23.camel@redhat.com> Date: Wed, 06 Dec 2017 14:40:00 -0000 In-Reply-To: <1511963747.27881.23.camel@redhat.com> (David Malcolm's message of "Wed, 29 Nov 2017 08:55:47 -0500") Message-ID: <87indjc43i.fsf@linaro.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-SW-Source: 2017-12/txt/msg00313.txt.bz2 David Malcolm writes: > On Wed, 2017-11-29 at 11:57 +0000, Richard Sandiford wrote: > > [...] > > I can't really comment on the representation ideas, but I'm always > happy to see new selftests... > > *************** test_labels () >> *** 13954,13959 **** >> --- 14179,14350 ---- >> ASSERT_FALSE (FORCED_LABEL (label_decl)); >> } >> >> + /* Check that VECTOR_CST Y contains the elements in X. */ >> + >> + static void >> + check_vector_cst (vec x, tree y) >> + { >> + for (unsigned int i = 0; i < x.length (); ++i) >> + ASSERT_EQ (wi::to_wide (x[i]), wi::to_wide (vector_cst_elt (y, >> i))); >> + } > > ...a couple of nits/suggestions: > > (a) How about renaming "x" to "expected"? Maybe rename "y" to > "actual"? (to better document the sense of the test). Good idea. I keep getting the ASSERT_* argument order for actual vs. expected confused, so having more explicit names would help. Done in the updated patch. However, I needed the patch below to use those names, since "actual" and "expected" are also used internally by the macros. I tried to protect other "user-level" names too. Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64le-linux-gnu. Bordering on obvious, but just in case: OK to install? > (b) At first glance, I wondered if this routine should also have > something like: > > ASSERT_EQ (expected.length (), VECTOR_CST_NELTS (actual)); > > Though that seems to come from the vector type, and it's always 8 in > these examples, so I'm not sure. Guess it can't hurt :-). Done in the updated patch. Thanks, Richard gcc/ 2017-12-05 Richard Sandiford * selftest.h (ASSERT_TRUE_AT, ASSERT_FALSE_AT, ASSERT_EQ_AT) (ASSERT_NE, ASSERT_PRED1): Add underscores to local variable names * selftest-rtl.h (ASSERT_RTX_EQ, ASSERT_RTX_PTR_EQ): Likewise. Index: gcc/selftest.h =================================================================== --- gcc/selftest.h 2017-11-29 11:06:34.324730917 +0000 +++ gcc/selftest.h 2017-12-06 14:34:28.377084919 +0000 @@ -219,12 +219,12 @@ #define ASSERT_TRUE(EXPR) \ #define ASSERT_TRUE_AT(LOC, EXPR) \ SELFTEST_BEGIN_STMT \ - const char *desc = "ASSERT_TRUE (" #EXPR ")"; \ - bool actual = (EXPR); \ - if (actual) \ - ::selftest::pass ((LOC), desc); \ + const char *desc_ = "ASSERT_TRUE (" #EXPR ")"; \ + bool actual_ = (EXPR); \ + if (actual_) \ + ::selftest::pass ((LOC), desc_); \ else \ - ::selftest::fail ((LOC), desc); \ + ::selftest::fail ((LOC), desc_); \ SELFTEST_END_STMT /* Evaluate EXPR and coerce to bool, calling @@ -239,12 +239,12 @@ #define ASSERT_FALSE(EXPR) \ #define ASSERT_FALSE_AT(LOC, EXPR) \ SELFTEST_BEGIN_STMT \ - const char *desc = "ASSERT_FALSE (" #EXPR ")"; \ - bool actual = (EXPR); \ - if (actual) \ - ::selftest::fail ((LOC), desc); \ - else \ - ::selftest::pass ((LOC), desc); \ + const char *desc_ = "ASSERT_FALSE (" #EXPR ")"; \ + bool actual_ = (EXPR); \ + if (actual_) \ + ::selftest::fail ((LOC), desc_); \ + else \ + ::selftest::pass ((LOC), desc_); \ SELFTEST_END_STMT /* Evaluate EXPECTED and ACTUAL and compare them with ==, calling @@ -259,11 +259,11 @@ #define ASSERT_EQ(EXPECTED, ACTUAL) \ #define ASSERT_EQ_AT(LOC, EXPECTED, ACTUAL) \ SELFTEST_BEGIN_STMT \ - const char *desc = "ASSERT_EQ (" #EXPECTED ", " #ACTUAL ")"; \ + const char *desc_ = "ASSERT_EQ (" #EXPECTED ", " #ACTUAL ")"; \ if ((EXPECTED) == (ACTUAL)) \ - ::selftest::pass ((LOC), desc); \ + ::selftest::pass ((LOC), desc_); \ else \ - ::selftest::fail ((LOC), desc); \ + ::selftest::fail ((LOC), desc_); \ SELFTEST_END_STMT /* Evaluate EXPECTED and ACTUAL and compare them with !=, calling @@ -272,11 +272,11 @@ #define ASSERT_EQ_AT(LOC, EXPECTED, ACTU #define ASSERT_NE(EXPECTED, ACTUAL) \ SELFTEST_BEGIN_STMT \ - const char *desc = "ASSERT_NE (" #EXPECTED ", " #ACTUAL ")"; \ + const char *desc_ = "ASSERT_NE (" #EXPECTED ", " #ACTUAL ")"; \ if ((EXPECTED) != (ACTUAL)) \ - ::selftest::pass (SELFTEST_LOCATION, desc); \ + ::selftest::pass (SELFTEST_LOCATION, desc_); \ else \ - ::selftest::fail (SELFTEST_LOCATION, desc); \ + ::selftest::fail (SELFTEST_LOCATION, desc_); \ SELFTEST_END_STMT /* Evaluate EXPECTED and ACTUAL and compare them with strcmp, calling @@ -312,14 +312,14 @@ #define ASSERT_STR_CONTAINS(HAYSTACK, NE /* Evaluate PRED1 (VAL1), calling ::selftest::pass if it is true, ::selftest::fail if it is false. */ -#define ASSERT_PRED1(PRED1, VAL1) \ - SELFTEST_BEGIN_STMT \ - const char *desc = "ASSERT_PRED1 (" #PRED1 ", " #VAL1 ")"; \ - bool actual = (PRED1) (VAL1); \ - if (actual) \ - ::selftest::pass (SELFTEST_LOCATION, desc); \ - else \ - ::selftest::fail (SELFTEST_LOCATION, desc); \ +#define ASSERT_PRED1(PRED1, VAL1) \ + SELFTEST_BEGIN_STMT \ + const char *desc_ = "ASSERT_PRED1 (" #PRED1 ", " #VAL1 ")"; \ + bool actual_ = (PRED1) (VAL1); \ + if (actual_) \ + ::selftest::pass (SELFTEST_LOCATION, desc_); \ + else \ + ::selftest::fail (SELFTEST_LOCATION, desc_); \ SELFTEST_END_STMT #define SELFTEST_BEGIN_STMT do { Index: gcc/selftest-rtl.h =================================================================== --- gcc/selftest-rtl.h 2017-11-01 09:41:40.726615206 +0000 +++ gcc/selftest-rtl.h 2017-12-06 14:34:28.377084919 +0000 @@ -49,8 +49,8 @@ #define ASSERT_RTL_DUMP_EQ_WITH_REUSE(EX #define ASSERT_RTX_EQ(EXPECTED, ACTUAL) \ SELFTEST_BEGIN_STMT \ - const char *desc = "ASSERT_RTX_EQ (" #EXPECTED ", " #ACTUAL ")"; \ - ::selftest::assert_rtx_eq_at (SELFTEST_LOCATION, desc, (EXPECTED), \ + const char *desc_ = "ASSERT_RTX_EQ (" #EXPECTED ", " #ACTUAL ")"; \ + ::selftest::assert_rtx_eq_at (SELFTEST_LOCATION, desc_, (EXPECTED), \ (ACTUAL)); \ SELFTEST_END_STMT @@ -62,8 +62,8 @@ extern void assert_rtx_eq_at (const loca #define ASSERT_RTX_PTR_EQ(EXPECTED, ACTUAL) \ SELFTEST_BEGIN_STMT \ - const char *desc = "ASSERT_RTX_PTR_EQ (" #EXPECTED ", " #ACTUAL ")"; \ - ::selftest::assert_rtx_ptr_eq_at (SELFTEST_LOCATION, desc, (EXPECTED), \ + const char *desc_ = "ASSERT_RTX_PTR_EQ (" #EXPECTED ", " #ACTUAL ")"; \ + ::selftest::assert_rtx_ptr_eq_at (SELFTEST_LOCATION, desc_, (EXPECTED), \ (ACTUAL)); \ SELFTEST_END_STMT