From: Vineet Gupta <vineetg@rivosinc.com>
To: Jeff Law <jeffreyalaw@gmail.com>
Cc: gcc@gcc.gnu.org, Kito Cheng <kito.cheng@gmail.com>,
Palmer Dabbelt <palmer@rivosinc.com>,
gnu-toolchain <gnu-toolchain@rivosinc.com>,
pinskia@gmail.com, GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: Followup on PR/109279: large constants on RISCV
Date: Mon, 12 Jun 2023 12:32:30 -0700 [thread overview]
Message-ID: <00e36310-368d-3435-a4a2-b8afbd6b3bc2@rivosinc.com> (raw)
In-Reply-To: <80218276-fc3c-032b-eaa7-0a4b0e8a859f@gmail.com>
Hi Jeff,
Thx for the detailed explanation and insight.
On 6/7/23 16:44, Jeff Law wrote:
>> With 2e886eef7f2b, define_insn_and_split "*mvconst_internal" recog()
>> kicks in during cse1, eliding insns for a const_int.
>>
>> (insn 7 6 8 2 (set (reg:DI 137)
>> (const_int [0x1010101])) {*mvconst_internal}
>> (expr_list:REG_EQUAL (const_int [0x1010101])))
>> [...]
>>
>> (insn 11 10 12 2 (set (reg:DI 140)
>> (const_int [0x1010101_00000000])) {*mvconst_internal}
>> (expr_list:REG_EQUAL (const_int [0x1010101_00000000]) ))
> Understood. Not ideal, but we generally don't have good ways to limit
> patterns to being available at different times during the optimization
> phase. One thing you might want to try (which I thought we used at
> one point) was make the pattern conditional on cse_not_expected. The
> goal would be to avoid exposing the pattern until a later point in the
> optimizer pipeline. It may have been the case that we dropped that
> over time during development. It's all getting fuzzy at this point.
Gave this a try and it seems to fix Andrew's test, but then regresses
the actual large const case: 0x1010101_01010101 : the mem to const_int
transformation was being done in cse1 which no longer happens and the
const pool from initial expand remains all the way into asm generated. I
don't think we want to go back to that state
>
>>
>> Eventually split1 breaks it up using same mvconst_internal splitter,
>> but the cse opportunity has been lost.
> Right. I'd have to look at the pass definitions, but I suspect the
> splitting pass where this happens is after the last standard CSE pass.
> So we don't get a chance to CSE the constant synthesis.
Yep split1 and friends happen after cse1 and cse2. At -O2 gcse doesn't
kick in and if forced to, it is currently limited in what it can do more
so given this is post reload.
>
>> *This is a now a baseline for large consts handling for RV backend
>> which we all need to be aware of*.
> Understood. Though it's not as bad as you might think :-) You can
> spend an inordinate amount of time improving constant synthesis,
> generate code that looks really good, but in the end it may not make a
> bit of different in real performance. Been there, done that. I'm not
> saying we give up, but we need to keep in mind that we're often better
> off trading a bit on the constant synthesis if doing so helps code
> where those constants get used.
Understood :-) I was coming to same realization and this seems like a
good segway into switching topic and investigating post reload gcse for
Const Rematerialization, another pesky issue with RV and likely to have
bigger impact across a whole bunch of workloads.
>> FWIW, IRA for latter case only, emits additional REG_EQUIV notes
>> which could also be playing a role.
> REG_EQUAL notes get promoted to REG_EQUIV notes in some cases. And
> when other equivalences are discovered it may create a REG_EQUIV note
> out of thin air.
>
> The REG_EQUIV note essentially means that everywhere the register
> occurs you can validly (from a program semantics standpoint) replace
> the register with the value. It might require reloading, but it's a
> valid semantic transformation which may reduce register pressure --
> especially for constants that were subject to LICM.
>
> Contrast to REG_EQUAL which creates an equivalence at a particular
> point in the IL, but the equivalence may not hold elsewhere in the IL.
Ok. From reading gccint it seems REG_EQUIV is a stronger form of
equivalence and seems to be prefered by post reload passes, while
REG_EQUAL is more of use in pre-reload.
> I would also look at reload_cse_regs which should give us some
> chance at seeing the value reuse if/when IRA/LRA muck things up.
I'll be out of office for the rest of week, will look into this once I'm
back.
Thx,
-Vineet
next prev parent reply other threads:[~2023-06-12 19:32 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-02 2:38 Vineet Gupta
2023-06-07 23:44 ` Jeff Law
2023-06-12 19:32 ` Vineet Gupta [this message]
2023-06-17 22:13 ` Jeff Law
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=00e36310-368d-3435-a4a2-b8afbd6b3bc2@rivosinc.com \
--to=vineetg@rivosinc.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=gcc@gcc.gnu.org \
--cc=gnu-toolchain@rivosinc.com \
--cc=jeffreyalaw@gmail.com \
--cc=kito.cheng@gmail.com \
--cc=palmer@rivosinc.com \
--cc=pinskia@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).