From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <gcc-bugzilla@gcc.gnu.org>
Received: by sourceware.org (Postfix, from userid 48)
	id C256C3858C62; Thu,  8 Jun 2023 15:22:49 +0000 (GMT)
DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org C256C3858C62
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org;
	s=default; t=1686237769;
	bh=p8pyXERlOAkUprNwSu/wazTud9/gyaWVxSrvhSDbuug=;
	h=From:To:Subject:Date:In-Reply-To:References:From;
	b=pngx9SdrreXj++zWB6lSKWEuzTf4LZFT02/ypMYaj/BW04ftD6Fbn8d2R8TNUCbWI
	 dZ1EbXTok2Oqpe5McSRSBKHEnCAnIpKgE/ggYSBvFVw+CBkTK4ZBCuBKUlxb28CVcx
	 +YqaI2NIXsAa2fJSiBkLbLaKMxNxWZvjVXgl9JIQ=
From: "klepikov.alex+bugs at gmail dot com" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug target/54089] [SH] Refactor shift patterns
Date: Thu, 08 Jun 2023 15:22:47 +0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: target
X-Bugzilla-Version: 4.8.0
X-Bugzilla-Keywords: 
X-Bugzilla-Severity: enhancement
X-Bugzilla-Who: klepikov.alex+bugs at gmail dot com
X-Bugzilla-Status: ASSIGNED
X-Bugzilla-Resolution: 
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: olegendo at gcc dot gnu.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags: 
X-Bugzilla-Changed-Fields: 
Message-ID: <bug-54089-4-XlNHdRN8uO@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-54089-4@http.gcc.gnu.org/bugzilla/>
References: <bug-54089-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
List-Id: <gcc-bugs.sourceware.org>

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D54089

--- Comment #71 from Alexander Klepikov <klepikov.alex+bugs at gmail dot co=
m> ---
> There are some formatting nits in your patch, please check again:

Thanks for pointing that out, I'll check again.

> But more interestingly:
> * Do we really need to add that new source file sh_loop.cc with the wrapp=
er
> classes?  Can't we just instantiate the passes that are needed in-place w=
hen
> they are registered?

Unfortunately, no.

First of all, classes are defined in loop-init.cc and thus cannot be access=
ed
directly.

There are wrappers called 'make_pass_rtl_blah_bla_blah' which create class
instances, but. Class .name field is hardcoded, there's no option to set it
when initializing, and pass manager checks if new pass name differs from any
already existing. Here's the code from passes.cc:

if (pass->type =3D=3D new_pass_info->pass->type
          && pass->name
          && !strcmp (pass->name, new_pass_info->reference_pass_name) //Name
check
          ...

Moreover, pass_rtl_loop_init::execute method calls a function that do the
following check:

gcc_assert (current_ir_type () =3D=3D IR_RTL_CFGLAYOUT);

And in our pass (that goes after split1 pass),
current_ir_type()!=3DIR_RTL_CFGLAYOUT, so we will get ICE. And even if we c=
ould
inherit the class, 'execute' method is final.

I'm not so good at C++, so may be there's another way. I know that the less
code is the better, but I'm afraid, not this time.

>=20
> * Can you add some tests to the SH specific tests?

Yes, of course! I'll add some 'tst #imm,0' presence tests. Should I add hoi=
st
test? I do not yet understand how hoisting check should be performed, but I
hope to find examples. Maybe some other tests? If I'm missing something, pl=
ease
tell me.

And as I understand, I should name test file something like
pr54089-next-free-number.c, right?

Thank you.=