On 7/10/24 3:05 AM, Georg-Johann Lay wrote:
The previous change to avr.md was several days ago, and should not
interfere with this one. Anyway, I rebased the patch against
master and attached it below. The patch is atop the ref in the patch
file name : https://gcc.gnu.org/r15-1935
I'll throw this version in. While avr isn't deeply tested additional
testing never hurts.
It looks like avr exposes the CC register early, creating references
to it during expansion to RTL. Presumably this means you've got a
reasonable way to reload values, particularly address arithmetic
without impacting the CC state?
No. CC only comes into existence after reload in .split2 and in some
cases in .avr-ifelse. reloads may clobber CC, so there is no way
to split cbranch insn prior to relaod.
Clearly I missed all those reload_completed checks. Thanks for clarifying.
It looks like you're relying heavily on peep2 patterns. Did you
explore using cmpelim?
Some question I have:
- compare-elim.cc states that the comparisons must look like
[(set (reg:CC) (compare:CC (reg) (reg_or_immediate)))]
which is not always the case, in particular the patterns
may have clobbers. compare-elim uses single_set, which
is true for the pattern with a clobber / scratch. However,
presence / absence of a clobber reg has to be taken into
account when deciding whether a transformation into cc
other than CCmode is possible. compare-elim only supplies
the SET_SRC, but not the scratch_operand to SELECT_CC_MODE.
We could probably fix that. I don't think we've had a target where the
clobbered object affects CC like this. Could we perhaps pass in the
full insn? That would require fixing the various targets that already
use the compare-elim infrastructure, but I think it'd be a pretty
mechanical change.
- The internals says that some optimizations / transforms
will happen prior to reload (e.g. .combine). This is not
possible since CCmode exists only since .split2.
- Insn combine may have transformed some comparisons, e.g.
sign test are represented as a zero_extract + skip like
in the sbrx_branch insns. This means that compares might
have a non-canonical form at .cmpelim. But a CCNmode
compare is better still than an sbrx_branch. Moreover,
CANONICALIZE_COMPARISON also runs already at insn combine,
so that compares have a form not recognized by cmpelim.
There's probably going to be some cases we can't necessarily handle due
to intricate target dependencies. That's fine. I'm more concerned
about whether or not we're utilizing the existing infrastructure. If we
can use the generic infrastructure we should. If we can use the generic
infrastructure after making some improvements we should. If the
problems aren't tractable with the generic infrastructure, then a
bespoke target solution may be the only path forward.
- Comparisons may look like if (0 <code> ref) which is
a form not supported by compare-elim.
That's not canonical RTL. If the backend is generating that it probably
needs to be fixed. Various passes assume the constant will be in the
other position.
Not really ready to ACK or NAK at this point.
jeff
So I guess mainly due to the cmpelim (non-)alternative?
Largely yes. I'll try to take a look at this now that I've got more
background info. Thanks.
jeff
