On Sat, Jul 23, 2022 at 9:32 AM Roger Sayle <ro...@nextmovesoftware.com> wrote:
>
>
> This patch resolves PR target/106303 (and the related PRs 106347,
> 106404, 106407) which are ICEs caused by my improvements to x86_64's
> 128-bit TImode to V1TImode Scalar to Vector (STV) pass. My apologies
> for the breakage. The issue is that data flow analysis is used to
> partition usage of each TImode pseudo into "chains", where each
> chain is analyzed and if suitable converted to vector operations.
> The problems appears when some chains for a pseudo are converted,
> and others aren't as RTL sharing can result in some mode changes
> leaking into other instructions that aren't/shouldn't/can't be
> converted, which eventually leads to an ICE for mismatched modes.
>
> My first approach to a fix was to unify more of the STV infrastructure,
> reasoning that if TImode STV was exhibiting these problems, but DImode
> and SImode STV weren't, the issue was likely to be caused/resolved by
> these remaining differences. This appeared to fix some but not all of
> the reported PRs. A better solution was then proposed by H.J. Lu in
> Bugzilla (thanks!) that we need to iterate the removal of candidates in the
> function timode_remove_non_convertible_regs until there are no further
> changes. As each chain is removed from consideration, it in turn may
> affect whether other insns/chains can safely be converted.
>
> This patch has been tested on x86_64-pc-linux-gnu with make bootstrap
> and make -k check, both with and without --target_board=unix{-m32},
> with no new failures. Ok for mainline?
>
>
> 2022-07-23 Roger Sayle <ro...@nextmovesoftware.com>
> H.J. Lu <hjl.to...@gmail.com>
>
> gcc/ChangeLog
> PR target/106303
> PR target/106347
> * config/i386/i386-features.cc (make_vector_copies): Move from
> general_scalar_chain to scalar_chain.
> (convert_reg): Likewise.
> (convert_insn_common): New scalar_chain method split out from
> general_scalar_chain convert_insn.
> (convert_registers): Move from general_scalar_chain to
> scalar_chain.
> (scalar_chain::convert): Call convert_insn_common before calling
> convert_insn.
> (timode_remove_non_convertible_regs): Iterate until there are
> no further changes to the candidates.
> * config/i386/i386-features.h (scalar_chain::hash_map): Move
> from general_scalar_chain.
> (scalar_chain::convert_reg): Likewise.
> (scalar_chain::convert_insn_common): New shared method.
> (scalar_chain::make_vector_copies): Move from general_scalar_chain.
> (scalar_chain::convert_registers): Likewise. No longer virtual.
> (general_scalar_chain::hash_map): Delete. Moved to scalar_chain.
> (general_scalar_chain::convert_reg): Likewise.
> (general_scalar_chain::make_vector_copies): Likewise.
> (general_scalar_chain::convert_registers): Delete virtual method.
> (timode_scalar_chain::convert_registers): Likewise.
>
> gcc/testsuite/ChangeLog
> PR target/106303
> PR target/106347
> * gcc.target/i386/pr106303.c: New test case.
> * gcc.target/i386/pr106347.c: New test case.
OK. (git diff -w shows much clearly what was changed).
Thanks,
Uros.
