public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: liuhongt <hongtao.liu@intel.com>, Richard Biener <rguenther@suse.de>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] Adjust more testcases for O2 vectorization enabling.
Date: Mon, 11 Oct 2021 11:28:55 +0200	[thread overview]
Message-ID: <20211011092855.GQ304296@tucnak> (raw)
In-Reply-To: <20211009045515.54589-1-hongtao.liu@intel.com>

On Sat, Oct 09, 2021 at 12:55:15PM +0800, liuhongt via Gcc-patches wrote:
> Pushed to trunk.
> 
> libgomp/ChangeLog:
> 
> 	* testsuite/libgomp.c++/scan-10.C: Add option -fvect-cost-model=cheap.
> 	* testsuite/libgomp.c++/scan-11.C: Ditto.
> 	* testsuite/libgomp.c++/scan-12.C: Ditto.
> 	* testsuite/libgomp.c++/scan-13.C: Ditto.
> 	* testsuite/libgomp.c++/scan-14.C: Ditto.
> 	* testsuite/libgomp.c++/scan-15.C: Ditto.
> 	* testsuite/libgomp.c++/scan-16.C: Ditto.
> 	* testsuite/libgomp.c++/scan-9.C: Ditto.
> 	* testsuite/libgomp.c-c++-common/lastprivate-conditional-7.c: Ditto.
> 	* testsuite/libgomp.c-c++-common/lastprivate-conditional-8.c: Ditto.
> 	* testsuite/libgomp.c/scan-11.c: Ditto.
> 	* testsuite/libgomp.c/scan-12.c: Ditto.
> 	* testsuite/libgomp.c/scan-13.c: Ditto.
> 	* testsuite/libgomp.c/scan-14.c: Ditto.
> 	* testsuite/libgomp.c/scan-15.c: Ditto.
> 	* testsuite/libgomp.c/scan-16.c: Ditto.
> 	* testsuite/libgomp.c/scan-17.c: Ditto.
> 	* testsuite/libgomp.c/scan-18.c: Ditto.
> 	* testsuite/libgomp.c/scan-19.c: Ditto.
> 	* testsuite/libgomp.c/scan-20.c: Ditto.
> 	* testsuite/libgomp.c/scan-21.c: Ditto.
> 	* testsuite/libgomp.c/scan-22.c: Ditto.

I don't think this is the right thing to do.
This just means that at some point between 2013 when -fsimd-cost-model has
been introduced and now -fsimd-cost-model= option at least partially stopped
working properly.
As documented, -fsimd-cost-model= overrides the -fvect-cost-model= setting
for OpenMP simd loops (loop->force_vectorize is true) if specified differently
from default.
In tree-vectorizer.h we have:
static inline bool
unlimited_cost_model (loop_p loop)
{
  if (loop != NULL && loop->force_vectorize
      && flag_simd_cost_model != VECT_COST_MODEL_DEFAULT)
    return flag_simd_cost_model == VECT_COST_MODEL_UNLIMITED;
  return (flag_vect_cost_model == VECT_COST_MODEL_UNLIMITED);
}
and use it in various places, but we also just use flag_vect_cost_model
in lots of places (and in one spot use flag_simd_cost_model, not sure if
we are sure it is a force_vectorize loop or what).

So, IMHO we should change the above inline function to
loop_cost_model and let it return the cost model and then just
reimplement unlimited_cost_model as
return loop_cost_model (loop) == VECT_COST_MODEL_UNLIMITED;
and then adjust the direct uses of the flag and revert these changes.

	Jakub


  parent reply	other threads:[~2021-10-11  9:29 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-09  4:55 liuhongt
2021-10-09 21:36 ` H.J. Lu
2021-10-11  2:10   ` [PATCH] Adjust testcase " liuhongt
2021-10-11  2:22     ` Kewen.Lin
2021-10-11  2:39       ` Hongtao Liu
2021-10-11  9:28 ` Jakub Jelinek [this message]
2021-10-11 12:15   ` [PATCH] vectorizer: Fix up -fsimd-cost-model= handling Jakub Jelinek
2021-10-11 12:24     ` Richard Biener
2021-10-11 12:27     ` Hongtao Liu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211011092855.GQ304296@tucnak \
    --to=jakub@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=hongtao.liu@intel.com \
    --cc=rguenther@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).