That define_insn is making my eyes bleed! I think that's the most convincing
argument I've ever read on gcc-patches, and I can see now what Segher is so
opposed to. My new goal is to add sh_mul and uh_mul as RTL expression
codes for highpart multiplication, so that that pattern will go away [and
never ever been seen again].
Fortunately, the proposed patch is part of the solution, rather than part of
the problem. simplify_subreg is used to find a sensible alternate form for
a hypothetical SUBREG before it's created (and normally it'll only be created
if it's valid c.f. validate_subreg in simplify_gen_subreg). This function is
called
over 30 million times during stage2/stage3 of a bootstrap on x86_64, with
the most common "SUBREG" operand RTX codes being:
16160294 reg
5627157 const_int
3278692 mem
960839 lshiftrt
902685 plus
690246 and
430131 subreg
319407 zero_extract
319145 minus
255043 value
232959 ashift
225468 mult
217819 xor
207645 clobber
207454 ashiftrt
100213 ior
Adding a simplify_subreg for mult, replacing them with a highpart rtx should
avoid (reduce) combine's Frankenstein patterns appearing in machine
descriptions.
Presumably, the reason that this define_insn starts with a "*" even for a
recognized
named pattern, is that if expand ever attempted emitting this instruction, the
internal sanity checks for a sane/valid SUBREG would fail/ICE.
p.s. You're right that there's a temptation to add ugly patterns to a backend,
instead of fixing their simplification in the middle-end; but only when backend
patches get reviewed faster/easier than middle-end patches 😊
Awesome (counter-)example!
Roger
--
-----Original Message-----
From: Richard Sandiford <richard.sandif...@arm.com>
Sent: 21 September 2021 13:02
To: Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org>
Cc: Segher Boessenkool <seg...@kernel.crashing.org>; Richard Biener
<richard.guent...@gmail.com>; Roger Sayle <ro...@nextmovesoftware.com>
Subject: Re: [PATCH] Simplify paradoxical subreg extensions of TRUNCATE
[Using this is a convenient place to reply to the thread as a whole]
Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> On Mon, Sep 6, 2021 at 12:15 PM Segher Boessenkool
> <seg...@kernel.crashing.org> wrote:
>>
>> On Sun, Sep 05, 2021 at 11:28:30PM +0100, Roger Sayle wrote:
>> > This patch simplifies the RTX (subreg:HI (truncate:QI (reg:SI))) as
>> > (truncate:HI (reg:SI)), and closely related variants.
>>
>> Subregs of other than regs are undefined in RTL. You will first have
>> to define this (in documentation as well as in other code that
>> handles subregs). I doubt this is possible to do, subreg have so
>> many overloaded meanings already.
>
> I suppose (subreg:MODE1 (xyz:MODE2 ..)) where xyz is not REG or MEM is
> equal to
>
> (set (reg:MODE2) (xyz:MODE2 ..))
> (subreg:MODE1 (reg:MODE2) ...)
>
> with 'reg' being a pseudo reg is the (only?) sensible way of defining it.
Agreed. And I think that's already the de facto definition (and has been for a
long time). Subreg as an operation has to have defined semantics for all the
cases that simplify_subreg handles, otherwise we have GIGO and a lot of the
function could be deleted. We can (and do) choose to prevent some of those
operations becoming actual rtxes, but even there, the de facto rules are quite
broad. E.g.:
- Like you said later, simplify_gen_subreg is opt-out rather than opt-in
in terms of the subreg rtxes that it's prepared to create.
- Even rs6000.md has:
(define_insn "*<su>mul<mode>3_highpart"
[(set (match_operand:GPR 0 "gpc_reg_operand" "=r")
(subreg:GPR
(mult:<DMODE> (any_extend:<DMODE>
(match_operand:GPR 1 "gpc_reg_operand" "r"))
(any_extend:<DMODE>
(match_operand:GPR 2 "gpc_reg_operand" "r")))
0))]
"WORDS_BIG_ENDIAN && !(<MODE>mode == SImode && TARGET_POWERPC64)"
"mulh<wd><u> %0,%1,%2"
[(set_attr "type" "mul")
(set_attr "size" "<bits>")])
Many other ports have similar patterns.
The problem with “combine can generate invalid rtl but backends should reject
it” is that, generally, people write combine patterns by looking at what
combine _wants_ to generate and then writing .md patterns to match that. In
other words, combine in practice defines the (de facto) correct rtl
representation of a combined sequence.
Given:
Trying 10 -> 15:
10: r29:QI=trunc(r32:SI)
REG_DEAD r32:SI
15: r38:HI=r29:QI#0
REG_DEAD r29:QI
Failed to match this instruction:
(set (reg:HI 38)
(subreg:HI (truncate:QI (reg:SI 32)) 0))
I'm sure there's a temptation to add an .md pattern that matches the subreg. :-)
Thanks,
Richard
> That would make the simplification apply when substituting the pseudos
> definition inside the subreg which maybe is what this targets?
>
> Richard.
>
>>
>>
>> Segher
