On 4/5/23 10:48, Jakub Jelinek wrote:
On Wed, Apr 05, 2023 at 10:17:59AM -0600, Jeff Law wrote:
It is true that an instruction like
(insn 8 7 9 2 (set (reg:HI 141)
(subreg:HI (reg:SI 142) 0)) "aauu.c":6:18 181 {*movhi_internal}
(nil))
can appear in the IL on WORD_REGISTER_OPERATIONS target, but I think the
upper bits shouldn't be random garbage in that case, it should be zero
extended or sign extended.
Well, that's one of the core questions here. What are the state of the
upper 16 bits of (reg:HI 141)? The WORD_REGISTER_OPERATIONS docs aren't
100% clear as we're not really doing any operation.
So again, I think we need to decide if the DSE transformation is correct or
not. I *think* we can aggree that insn 39 is OK. It's really the semantics
of insn 47 that I think we need to agree on. What is the state of the upper
16 bits of (reg:HI 175) after insn 47?
I'm afraid I don't know the answers here, I think Eric is
WORD_REGISTER_OPERATIONS expert here I think these days (most of the major
targets are !WORD_REGISTER_OPERATIONS).
Hopefully he'll chime in.
Intuitively, WORD_REGISTER_OPERATIONS from the description would be
that
(insn 47 35 39 2 (set (reg:HI 175)
(subreg:HI (reg:SI 166) 0)) "pr109040.c":9:11 179 {*movhi_internal}
(expr_list:REG_DEAD (reg:SI 166)
(nil)))
would copy not just the low 16-bits of pseudo 166 to 175, but also the upper
16-bits.
I've gone back and forth repeatedly on this.
Originally I didn't really see this as an operation. But the more and
more I ponder it feels like it's an operation and thus should be subject
to WORD_REGISTER_OPERATIONS.
While it's not really binding on RTL semantics, if we look at how some
architectures implement reg->reg copies, they're actually implemented
with an ADD or IOR -- so a real operation under the hood.
If we accept a subreg copy as an operation and thus subject to
WORD_REGISTER_OPERATIONS then that would seem to imply the combine is
the real problem here. Otherwise dse is the culprit.
But if that is so, then something is broken in the code below.
Some archeology shows that we were doing that on all arches initially
and then
Wed May 13 17:38:35 1998 Richard Kenner <ken...@vlsi1.ultra.nyu.edu>
* combine.c (simplify_comparison, case AND): Don't commute AND
with SUBREG if constant is whole mode and don't do if lowpart
and not WORD_REGISTER_OPERATIONS.
restricted that to WORD_REGISTER_OPERATIONS only.
The whole optimization is then likely
Wed Mar 18 05:54:25 1998 Richard Kenner <ken...@vlsi1.ultra.nyu.edu>
* combine.c (gen_binary): Don't make AND that does nothing.
(simplify_comparison, case AND): Commute AND and SUBREG.
* i386.h (CONST_CONSTS, case CONST_INT): One-byte integers are cost 0.
but the code has been tweaked further many times since then.
Sigh. 1998 and probably directly from Kenner's tree at the time. So no
public discussion, likely no testcases either. Off to my private
archives. Yup, no discussion of Kenner's patch or testcase.
Interestingly enough I did stumble across Andreas Schwab reporting a bug
in Kenner's change, though on the m68k and unrelated to
WORD_REGISTER_OPERATIONS
jeff
