On Wed, Jan 04, 2017 at 06:42:23PM +0000, Richard Sandiford wrote:
1. reload has a bug that no-one really wants to fix (understandable)
2. the bug is triggered by paradoxical subregs of mems
3. those subregs are normally disabled on targets that support insn
scheduling
4. therefore, define an insn scheduler
5. we don't actually want insn scheduling, so either
(a) make sure the insn scheduler is never actually used for insn
scheduling, or
(b) allow the insn scheduler to run anyway but encourage it to do nothing
(other than take compile time)
(4) and (5) feel like too much of a hack to me. They're going to have
other consequences, e.g. we'll no longer give the warning:
instruction scheduling not supported on this target machine
if users try to use -fschedule-insns. And since we don't support
meaningful insn scheduling even after this patch, giving the warning
seems more user-friendly than dropping it.
I think the consensus is that we don't want these subregs for AVR
regardless of whether scheduling is used, and probably wouldn't want
them even without this bug.
Right, and the same is true for most targets. Subregs of memory are not
something you want. As rtl.texi says:
@item mem
@code{subreg}s of @code{mem} were common in earlier versions of GCC and
are still supported. During the reload pass these are replaced by plain
@code{mem}s. On machines that do not do instruction scheduling, use of
@code{subreg}s of @code{mem} are still used, but this is no longer
recommended. Such @code{subreg}s are considered to be
@code{register_operand}s rather than @code{memory_operand}s before and
during reload. Because of this, the scheduling passes cannot properly
schedule instructions with @code{subreg}s of @code{mem}, so for machines
that do scheduling, @code{subreg}s of @code{mem} should never be used.
To support this, the combine and recog passes have explicit code to
inhibit the creation of @code{subreg}s of @code{mem} when
@code{INSN_SCHEDULING} is defined.
So why not instead change the condition
used by general_operand, like we were talking about yesterday?
It seems simpler and more direct.
We should split off a new "SUBREGS_OF_MEM_ALLOWED" from !INSN_SCHEDULING,
and then probably even default it to false.