Am 02.07.24 um 15:48 schrieb Richard Biener:
On Tue, Jul 2, 2024 at 3:43 PM Georg-Johann Lay <a...@gjlay.de> wrote:

Hi Jeff,

This is a patch to get correct code out of 64-bit
loads from address-space __memx.

The AVR address-spaces may require that move insns issue
calls to library support functions, a fact that -ftree-ter
doesn't account for.  tree-ssa-ter.cc then replaces an
expression across such a library call, resulting in wrong code.

This patch disables that pass per default on avr, as there is no
more fine grained way to avoid malicious optimizations.
The pass can still be re-enabled by means of explicit -ftree-ter.

Ok to apply?

I think this requires more details on what goes wrong - I assume
it's not stmt reordering that effectively happens but recursive
expand_expr on SSA defs when those invoke libcalls?  In that
case this would point to a deeper issue.

The difference is that with TER, we get a hard reg in .expand
for a movdi from 24-bit address-space __memx.

Such moves require library calls, which in turn require
specific hard registers.  As avr backend has no movdi, the
moddi gets expanded as 8 * movqi, and that does not work
when the target registers are hard regs, as some of them
are clobbered by the libcalls.

Moreover, even with TER, the code is no more efficient than
without it, so it's not clear what's the point in propagating
hard regs into expander operands. Later passes like fwprop1 and
combine can do that, too.

Requiring libcalls in a mov insn is quite special indeed,
and there is no way to tell that to TER.  TER itself does not
optimize code involving libcalls, so it knows they are fragile.

So - if the wrongness is already apparent in the RTL expansion
pass dump can you quote the respective pieces and explain why?

It expand a 64-bit move from __memx address-space to registers
R18...R25.  This is broken into 8 QI moves to these regs, but
the movqi requires a libcall in some situations, which pass their
arguments in R21...R25.  Hence the libcalls clobber some of the
destination regs.

It would already help when TER would not propagate hard-regs into
expander operands.

Johann

As an alternative, the option could be disabled permanently in
avr.cc::avr_option_override().

Johann

--

AVR: middle-end/87376 - Use -fno-tree-ter per default.

Temporary expression replacement might replace expressions across
library calls, for example with move insn from address-space __memx
like in PR87376.  -ftree-ter has no way where the backend could hook
in to avoid only problematic replacements, thus kick it out altogether.

         PR middle-end/87376
gcc/
         * common/config/avr/avr-common.cc (avr_option_optimization_table)
         <OPT_ftree_ter>: Set to 0.
gcc/testsuite/
         * gcc.target/avr/torture/pr87376-memx.c: New test.

Reply via email to