public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Andrew Stubbs <ams@codesourcery.com>
To: "gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>
Subject: [OG12 commit] vect: WORKAROUND vectorizer bug
Date: Mon, 24 Oct 2022 17:50:40 +0100	[thread overview]
Message-ID: <264e9c27-cef4-b2a5-8758-a8b621428e01@codesourcery.com> (raw)

[-- Attachment #1: Type: text/plain, Size: 1289 bytes --]

I've committed this to the OG12 branch to remove some test failures. We 
probably ought to have something on mainline also, but a proper fix 
would be better.

Without this. the libgomp.oacc-c-c++-common/private-variables.c testcase 
fails to compile due to an ICE.  The OpenACC worker broadcasting code is 
creating SLP optimizable loads and stores in amdgcn address-space-4. 
Previously this was "ok" as SLP didn't work with less that 64-lane 
vectors, but the newly implemented smaller vectors are working as 
intended and optimizing this.

Unfortunately the vectorizer is losing the address-space data from the 
intermediate types, and it all falls apart during expand when it tries 
the convert a 32-bit address into a 64-bit address and that's not 
something that works. At first sight it looks like we could possibly 
make that work with POINTERS_EXTEND_UNSIGNED, but that only changes the 
error message. Fundamentally we need to make sure that various instances 
of "vectype" have the correct address space, but my attempts to do so 
showed that that's a larger task than I have time for right now.

This patch simply prevents the vectorizer working in the case where it 
would break. This should not be a regression because this code didn't 
vectorize at all, previously.

Andrew

[-- Attachment #2: 221024-workarround-vec-addrspace-bug.patch --]
[-- Type: text/plain, Size: 2139 bytes --]

vect: WORKAROUND vectorizer bug

This patch disables vectorization of memory accesses to non-default address
spaces where the pointer size is different to the usual pointer size.  This
condition typically occurs in OpenACC programs on amdgcn, where LDS memory is
used for broadcasting gang-private variables between threads. In particular,
see libgomp.oacc-c-c++-common/private-variables.c

The problem is that the address space information is dropped from the various
types in the middle-end and eventually it triggers an ICE trying to do an
address conversion.  That ICE can be avoided by defining
POINTERS_EXTEND_UNSIGNED, but that just produces wrong RTL code later on.

A correct solution would ensure that all the vectypes have the correct address
spaces, but I don't have time for that right now.

gcc/ChangeLog:

	* tree-vect-data-refs.cc (vect_analyze_data_refs): Workaround an
	address-space bug.

diff --git a/gcc/tree-vect-data-refs.cc b/gcc/tree-vect-data-refs.cc
index 09223baf718..70b671ed94a 100644
--- a/gcc/tree-vect-data-refs.cc
+++ b/gcc/tree-vect-data-refs.cc
@@ -4598,7 +4598,21 @@ vect_analyze_data_refs (vec_info *vinfo, poly_uint64 *min_vf, bool *fatal)
       /* Set vectype for STMT.  */
       scalar_type = TREE_TYPE (DR_REF (dr));
       tree vectype = get_vectype_for_scalar_type (vinfo, scalar_type);
-      if (!vectype)
+
+      /* FIXME: If the object is in an address-space in which the pointer size
+	 is different to the default address space then vectorizing here will
+	 lead to an ICE down the road because the address space information
+	 gets lost.  This work-around fixes the problem until we have a proper
+	 solution.  */
+      tree base_object = DR_REF (dr);
+      tree op = (TREE_CODE (base_object) == COMPONENT_REF
+		 || TREE_CODE (base_object) == ARRAY_REF
+		 ? TREE_OPERAND (base_object, 0) : base_object);
+      addr_space_t as = TYPE_ADDR_SPACE (TREE_TYPE (op));
+      bool addr_space_bug = (!ADDR_SPACE_GENERIC_P (as)
+			     && targetm.addr_space.pointer_mode (as) != Pmode);
+
+      if (!vectype || addr_space_bug)
         {
           if (dump_enabled_p ())
             {

             reply	other threads:[~2022-10-24 16:50 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-24 16:50 Andrew Stubbs [this message]
2022-10-24 18:06 ` Richard Biener
2022-10-27 13:12   ` Andrew Stubbs

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=264e9c27-cef4-b2a5-8758-a8b621428e01@codesourcery.com \
    --to=ams@codesourcery.com \
    --cc=gcc-patches@gcc.gnu.org \
    /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).