On Sun, Oct 01, 2023 at 07:03:51AM +0800, 钟居哲 wrote:
> LGTM.
>
> juzhe.zh...@rivai.ai
>
> diff --git a/gcc/config/riscv/riscv-vsetvl.cc
> b/gcc/config/riscv/riscv-vsetvl.cc
> index af8c31d873c..4b06d93e7f9 100644
> --- a/gcc/config/riscv/riscv-vsetvl.cc
> +++ b/gcc/config/riscv/riscv-vsetvl.cc
> @@ -2417,8 +2417,8 @@ vector_infos_manager::vector_infos_manager ()
> vector_antin = nullptr;
> vector_antout = nullptr;
> vector_earliest = nullptr;
> - vector_insn_infos.safe_grow (get_max_uid ());
> - vector_block_infos.safe_grow (last_basic_block_for_fn (cfun));
> + vector_insn_infos.safe_grow_cleared (get_max_uid ());
> + vector_block_infos.safe_grow_cleared (last_basic_block_for_fn (cfun));
> if (!optimize)
> {
> basic_block cfg_bb;
Note, while it works around the build failure, I strongly doubt it is the
right fix in this case. The other spots which have been changed similarly
are quite different, all the vec<bitmap_head> cases have been followed
(almost) immediately by bitmap_initialize of all the elements or just 1-3
elements and their actual uses.
The above is very different. Sizing a vector with get_max_uid ()
means it is very likely very sparse and so constructing every element in the
array seems undesirable. While it is true that e.g. IRA or DF use vectors
indexed by INSN_UID, I think the vectors pretty much always have pointer
elements and say pool allocate what it points to upon first access.
While vector_insn_info is huge (48 bytes on 64-bit hosts from what I can see)
even without much attempts to make it shorter (e.g. the vl_vtype_info member
ordering of 2xpointer sized member, 1 byte member, 4 byte member, 3 1 byte
members creates unnecessary padding).
The reason why arrays indexed by INSN_UID are sparse are 3:
1) we have --param min-nondebug-insn-uid= parameter (where, albeit usually
just for debugging, one can specify very high start for those, say
--param min-nondebug-insn-uid=100000 means debug insns will be created
with INSN_UID 1 and up, while non-DEBUG_INSNs only with INSN_UID 100000
and up, so even a function containing just 3-4 insns will have
get_max_uid () of 100004 or so; the above allocates in that case
100004 * 48 bytes (bad) and newly constructs all the elements in it
2) INSN_UIDs are given to all newly created insns during RTL optimizations,
when an insn is deleted, its INSN_UID is not reused, so there can be
large gaps; the more churn in RTL optimizations, the larger
3) INSN_UIDs are given even to BARRIERs, DEBUG_INSNs etc., plus the question
is if the algorithm really needs to track every "normal" insn as well,
whether it shouldn't track just those that are in some ways using vectors
or relevant to it, many scalar instructions might be uninteresting,
DEBUG_INSNs certainly should be uninteresting (they must not affect
code generation), etc.
So, I think having something lazily allocated and initialized on demand
might be a compile time memory and time win.
For basic blockswe perform some block number compactions during compilation,
so those shouldn't be denser. But the structure for each basic block is
even bigger (104 bytes, because it contains 2 x 48 plus probability).
E.g. DF uses for many purposes LUIDs instead, which need to be recomputed,
but are logical uids within each basic block.
Jakub
