From: Richard Sandiford <richard.sandiford@arm.com>
To: Hongtao Liu <crazylht@gmail.com>
Cc: Hongtao Liu via Gcc-patches <gcc-patches@gcc.gnu.org>,
ebotcazou@libertysurf.fr, steven@gcc.gnu.org,
Jakub Jelinek <jakub@redhat.com>
Subject: Re: [PATCH] [PR rtl/optimization/98694] Fix incorrect optimization by cprop_hardreg.
Date: Tue, 19 Jan 2021 12:38:47 +0000 [thread overview]
Message-ID: <mpteeih3xuw.fsf@arm.com> (raw)
In-Reply-To: <CAMZc-bwJwbnWafSQsWXoSR7-0PysyfXNTTGqajpUmgoREgvVUQ@mail.gmail.com> (Hongtao Liu's message of "Tue, 19 Jan 2021 08:59:25 +0800")
Hongtao Liu <crazylht@gmail.com> writes:
> On Mon, Jan 18, 2021 at 7:10 PM Richard Sandiford
> <richard.sandiford@arm.com> wrote:
>>
>> Hongtao Liu <crazylht@gmail.com> writes:
>> > On Mon, Jan 18, 2021 at 6:18 PM Richard Sandiford
>> > <richard.sandiford@arm.com> wrote:
>> >>
>> >> Hongtao Liu via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
>> >> > Hi:
>> >> > If SRC had been assigned a mode narrower than the copy, we can't link
>> >> > DEST into the chain even they have same
>> >> > hard_regno_nregs(i.e. HImode/SImode in i386 backend).
>> >>
>> >> In general, changes between modes within the same hard register are OK.
>> >> Could you explain in more detail what's going wrong?
>> >>
>> >
>> > cprop hardreg change
>> >
>> > (insn 457 499 460 33 (set (reg:SI 39 r11 [orig:86 _11 ] [86])
>> > (reg:SI 37 r9 [orig:86 _11 ] [86])) "test.c":29:36 75 {*movsi_internal}
>> > (expr_list:REG_DEAD (reg:SI 37 r9 [orig:86 _11 ] [86])
>> > (nil)))
>> >
>> > to
>> >
>> > (insn 457 499 460 33 (set (reg:SI 39 r11 [orig:86 _11 ] [86])
>> > (reg:SI 22 xmm2 [orig:86 _11 ] [86])) "test.c":29:36 75
>> > {*movsi_internal}
>> > (expr_list:REG_DEAD (reg:SI 22 xmm2 [orig:86 _11 ] [86])
>> > (nil)))
>> >
>> > since (reg:SI 22 xmm2) and (reg:SI r9) are in the same value chain in
>> > which the oldest regno is k0.
>> >
>> > but with xmm2 defined as
>> >
>> > kmovw %k0, %edi # 69 [c=4 l=4] *movhi_internal/6----- kmovw move the
>> > lower 16bits to %edi, and clear the upper 16 bits.
>> > vmovd %edi, %xmm2 # 489 *movsi_internal --- vmovd move 32bits from
>> > %edi to %xmm2.
>> >
>> > (insn 69 68 70 12 (set (reg:HI 5 di [orig:96 _52 ] [96])
>> > (reg:HI 68 k0 [orig:82 var_6.0_1 ] [82])) "test.c":21:23 76
>> > {*movhi_internal}
>> > (nil))
>> >
>> > (insn 489 75 78 12 (set (reg:SI 22 xmm2 [297])
>> > (reg:SI 5 di [orig:96 _52 ] [96])) 75 {*movsi_internal}
>> > (nil))
>>
>> The sequence is OK in itself, but insn 489 can't make any assumptions
>> about what's in the upper 16 bits of %edi. In other words, as far as
>> RTL semantics are concerned, insn 489 only leaves bits 0-15 of %xmm2
>> with defined values; the other bits are undefined.
>>
>> If the target wants all 32 bits of %edi to be carried over to insn 489
>> then it needs to make insn 69 an SImode set instead of a HImode set.
>>
>
> actually only the lower 16bits are needed, the original insn is like
>
> .294.r.ira
> (insn 69 68 70 13 (set (reg:HI 96 [ _52 ])
> (subreg:HI (reg:DI 82 [ var_6.0_1 ]) 0)) "test.c":21:23 76
> {*movhi_internal}
> (nil))
> (insn 78 75 82 13 (set (reg:V4HI 140 [ _283 ])
> (vec_duplicate:V4HI (truncate:HI (subreg:SI (reg:HI 96 [ _52
> ]) 0)))) 1412 {*vec_dupv4hi}
> (nil))
>
> .295r.reload
> (insn 69 68 70 13 (set (reg:HI 5 di [orig:96 _52 ] [96])
> (reg:HI 68 k0 [orig:82 var_6.0_1 ] [82])) "test.c":21:23 76
> {*movhi_internal}
> (nil))
> (insn 489 75 78 13 (set (reg:SI 22 xmm2 [297])
> (reg:SI 5 di [orig:96 _52 ] [96])) 75 {*movsi_internal}
> (nil))
> (insn 78 489 490 13 (set (reg:V4HI 20 xmm0 [orig:140 _283 ] [140])
> (vec_duplicate:V4HI (truncate:HI (reg:SI 22 xmm2 [297]))))
> 1412 {*vec_dupv4hi}
> (nil))
>
> and insn 489 is created by lra/reload which seems ok for the sequence,
> but problemistic with considering the logic of hardreg_cprop.
It looks OK even with the regcprop behaviour though:
- insn 69 defines only the low 16 bits of di,
- insn 489 defines only the low 16 bits of xmm2, but copies bits 16-31
too (with unknown contents)
- insn 78 uses only the low 16 bits of xmm2 (the unknown contents
introduced by insn 489 are truncated away)
So where do bits 16-31 become significant? What goes wrong if they're
not zero?
Thanks,
Richard
next prev parent reply other threads:[~2021-01-19 12:38 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-18 9:16 Hongtao Liu
2021-01-18 10:18 ` Richard Sandiford
2021-01-18 10:43 ` Hongtao Liu
2021-01-18 10:51 ` Hongtao Liu
2021-01-18 11:10 ` Richard Sandiford
2021-01-19 0:59 ` Hongtao Liu
2021-01-19 12:38 ` Richard Sandiford [this message]
2021-01-19 14:45 ` Jakub Jelinek
2021-01-19 16:10 ` Richard Sandiford
2021-01-20 4:35 ` Hongtao Liu
2021-01-20 4:40 ` Hongtao Liu
2021-01-20 12:56 ` H.J. Lu
2021-01-20 14:14 ` Richard Sandiford
2021-01-21 5:25 ` Hongtao Liu
2021-05-05 17:44 ` [PATCH] regcprop: Fix another cprop_hardreg bug [PR100342] Jakub Jelinek
2021-05-06 8:50 ` Jakub Jelinek
2021-05-11 10:59 ` Richard Sandiford
2021-05-13 15:37 ` Jakub Jelinek
2021-05-13 17:01 ` Jakub Jelinek
2021-05-14 9:09 ` Richard Sandiford
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=mpteeih3xuw.fsf@arm.com \
--to=richard.sandiford@arm.com \
--cc=crazylht@gmail.com \
--cc=ebotcazou@libertysurf.fr \
--cc=gcc-patches@gcc.gnu.org \
--cc=jakub@redhat.com \
--cc=steven@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).