From: Jeff Law <jeffreyalaw@gmail.com>
To: Robin Dapp <rdapp.gcc@gmail.com>,
"juzhe.zhong@rivai.ai" <juzhe.zhong@rivai.ai>,
"kito.cheng" <kito.cheng@gmail.com>
Cc: gcc-patches <gcc-patches@gcc.gnu.org>,
"Kito.cheng" <kito.cheng@sifive.com>
Subject: Re: [PATCH] RISC-V: Allow LICM hoist POLY_INT configuration code sequence
Date: Mon, 18 Mar 2024 22:09:05 -0600 [thread overview]
Message-ID: <9c4e07e8-0913-45a9-a717-00f9469e11ca@gmail.com> (raw)
In-Reply-To: <56e4737a-0e63-4110-886f-0d8d2c764e29@gmail.com>
On 2/6/24 6:14 AM, Robin Dapp wrote:
>> The root cause is this following RTL pattern, after fwprop1:
>>
>> (insn 82 78 84 9 (set (reg:DI 230)
>> (sign_extend:DI (minus:SI (subreg/s/v:SI (reg:DI 150 [ niters.10 ]) 0)
>> (subreg:SI (reg:DI 221) 0)))) 13 {subsi3_extended}
>> (expr_list:REG_EQUAL (sign_extend:DI (plus:SI (subreg/s/v:SI (reg:DI 150 [ niters.10 ]) 0)
>> *(const_poly_int:SI [-16, -16])*))
>> (nil)))
>>
>> The highlight *(const_poly_int:SI [-16, -16])*
>> causes ICE.
>>
>> This RTL is because:
>> (insn 69 68 71 8 (set (reg:DI 221)
>> (const_poly_int:DI [16, 16])) 208 {*movdi_64bit}
>> (nil))
>> (insn 82 78 84 9 (set (reg:DI 230)
>> (sign_extend:DI (minus:SI (subreg/s/v:SI (reg:DI 150 [ niters.10 ]) 0)
>> (subreg:SI (reg:DI 221) 0)))) 13 {subsi3_extended} ----> (subreg:SI (const_poly_int:SI [-16, -16])) fwprop1 add (const_poly_int:SI [-16, -16]) reg_equal
>> (expr_list:REG_EQUAL (sign_extend:DI (plus:SI (subreg/s/v:SI (reg:DI 150 [ niters.10 ]) 0)
>> (const_poly_int:SI [-16, -16])))
>> (nil)))
>
> I'm seeing a slightly different pattern but that doesn't change
> the problem.
>
>> (set (reg:SI) (subreg:SI (DI: poly value))) but it causes ICE that I
>> mentioned above.
>
> That's indeed a bit more idiomatic and I wouldn't oppose that.
>
> The problem causing the ICE is that we want to simplify a PLUS
> with (const_poly_int:SI [16, 16]) and (const_int 0) but the mode
> is DImode. My suspicion is that this is caused by our
> addsi3_extended pattern and we fail to deduce the proper mode
> for analysis.
Certainly possible. It didn't even occur to me that a POLY_INT would
slip through here.
>
> I'm just speculating but maybe that's because we assert that a
> plus is of the form simple_reg_p (op0) && CONSTANT_P (op1).
> Usually, constants don't have a mode and can just be used.
> poly_int_csts do have one and need to be explicitly converted
> (kind of).
>
> We can only analyze this zero_extended plus at all since Jeff
> added the addsi3_extended handling for loop-iv. Maybe we could
> punt like
>
> diff --git a/gcc/loop-iv.cc b/gcc/loop-iv.cc
> index eb7e923a38b..796413c25a3 100644
> --- a/gcc/loop-iv.cc
> +++ b/gcc/loop-iv.cc
> @@ -714,6 +714,9 @@ get_biv_step_1 (df_ref def, scalar_int_mode outer_mode, rtx reg,
> if (!simple_reg_p (op0) || !CONSTANT_P (op1))
> return false;
>
> + if (CONST_POLY_INT_P (op1) && GET_MODE (op1) != outer_mode)
> + return false;
> +
>
> This helps for your test case but I haven't done any further
> testing. I'd think this is relatively safe because it's only
> a missed analysis/optimization in the worst case.
> Still, generally, I don't see a reason why we wouldn't be able
> to analyze this?
I don't think it would significant hurt anything. IIRC bit of code was
to fix a minor regression caused by the backend changes.
I would ACK that patch given the usual testing cycle.
jeff
prev parent reply other threads:[~2024-03-19 4:09 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-01 15:45 Juzhe-Zhong
2024-02-02 8:50 ` Kito Cheng
2024-02-04 2:03 ` juzhe.zhong
2024-02-06 13:14 ` Robin Dapp
2024-02-18 2:49 ` juzhe.zhong
2024-03-19 4:09 ` Jeff Law [this message]
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=9c4e07e8-0913-45a9-a717-00f9469e11ca@gmail.com \
--to=jeffreyalaw@gmail.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=juzhe.zhong@rivai.ai \
--cc=kito.cheng@gmail.com \
--cc=kito.cheng@sifive.com \
--cc=rdapp.gcc@gmail.com \
/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).