* [Bug target/89954] missed optimization for signed extension for x86-64
[not found] <bug-89954-4@http.gcc.gnu.org/bugzilla/>
@ 2021-09-05 7:01 ` pinskia at gcc dot gnu.org
2021-09-09 5:13 ` crazylht at gmail dot com
` (8 subsequent siblings)
9 siblings, 0 replies; 10+ messages in thread
From: pinskia at gcc dot gnu.org @ 2021-09-05 7:01 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89954
Andrew Pinski <pinskia at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Severity|normal |enhancement
--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
So at combine we have:
(insn 5 2 6 2 (set (reg:QI 87 [ c ])
(mem/c:QI (symbol_ref:DI ("c") [flags 0x2] <var_decl 0x7fc1a1d62c60
c>) [0 c+0 S1 A8])) "/app/example.cpp":5:14 77 {*movqi_internal}
(nil))
(insn 6 5 7 2 (parallel [
(set (reg:QI 86)
(xor:QI (reg:QI 87 [ c ])
(const_int 1 [0x1])))
(clobber (reg:CC 17 flags))
]) "/app/example.cpp":5:14 546 {*xorqi_1}
(expr_list:REG_DEAD (reg:QI 87 [ c ])
(expr_list:REG_UNUSED (reg:CC 17 flags)
(expr_list:REG_EQUAL (xor:QI (mem/c:QI (symbol_ref:DI ("c") [flags
0x2] <var_decl 0x7fc1a1d62c60 c>) [0 c+0 S1 A8])
(const_int 1 [0x1]))
(nil)))))
(insn 7 6 12 2 (set (reg:SI 85)
(sign_extend:SI (reg:QI 86))) "/app/example.cpp":5:14 154 {extendqisi2}
(expr_list:REG_DEAD (reg:QI 86)
(nil)))
The load is not loaded into the full SI/DI register yet. The change to SI mode
for xor happens after reload in split2.
So yes it is a target issue.
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bug target/89954] missed optimization for signed extension for x86-64
[not found] <bug-89954-4@http.gcc.gnu.org/bugzilla/>
2021-09-05 7:01 ` [Bug target/89954] missed optimization for signed extension for x86-64 pinskia at gcc dot gnu.org
@ 2021-09-09 5:13 ` crazylht at gmail dot com
2021-09-11 12:59 ` ubizjak at gmail dot com
` (7 subsequent siblings)
9 siblings, 0 replies; 10+ messages in thread
From: crazylht at gmail dot com @ 2021-09-09 5:13 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89954
--- Comment #4 from Hongtao.liu <crazylht at gmail dot com> ---
-------- aarch64 dump-------
Failed to match this instruction:
(set (reg:SI 95)
(xor:SI (zero_extend:SI (mem/c:QI (lo_sum:DI (reg/f:DI 97)
(symbol_ref:DI ("*.LANCHOR0") [flags 0x182])) [0 c+0 S1
A8]))
(const_int 1 [0x1])))
Successfully matched this instruction:
(set (reg:SI 99)
(zero_extend:SI (mem/c:QI (lo_sum:DI (reg/f:DI 97)
(symbol_ref:DI ("*.LANCHOR0") [flags 0x182])) [0 c+0 S1 A8])))
Successfully matched this instruction:
(set (reg:SI 95)
(xor:SI (reg:SI 99)
(const_int 1 [0x1])))
---------dump end------------
It looks like there's splitter in aarch64 which combines load+xor+zero_extend
to zero_extend(mem) + xor, x86 doesn't have. The simple way is to add
corresponding define_split for x86.
---------x86 dump-----------
Failed to match this instruction:
(set (reg:SI 85)
(sign_extend:SI (xor:QI (mem/c:QI (symbol_ref:DI ("c") [flags 0x2]
<var_decl 0x7f94fd2e8c60 c>) [0 c+0 S1 A8])
(const_int 1 [0x1]))))
-----------dump end----------
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bug target/89954] missed optimization for signed extension for x86-64
[not found] <bug-89954-4@http.gcc.gnu.org/bugzilla/>
2021-09-05 7:01 ` [Bug target/89954] missed optimization for signed extension for x86-64 pinskia at gcc dot gnu.org
2021-09-09 5:13 ` crazylht at gmail dot com
@ 2021-09-11 12:59 ` ubizjak at gmail dot com
2021-09-11 14:24 ` crazylht at gmail dot com
` (6 subsequent siblings)
9 siblings, 0 replies; 10+ messages in thread
From: ubizjak at gmail dot com @ 2021-09-11 12:59 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89954
--- Comment #5 from Uroš Bizjak <ubizjak at gmail dot com> ---
(In reply to Hongtao.liu from comment #4)
> It looks like there's splitter in aarch64 which combines
> load+xor+zero_extend to zero_extend(mem) + xor, x86 doesn't have. The simple
> way is to add corresponding define_split for x86.
>
> ---------x86 dump-----------
> Failed to match this instruction:
> (set (reg:SI 85)
> (sign_extend:SI (xor:QI (mem/c:QI (symbol_ref:DI ("c") [flags 0x2]
> <var_decl 0x7f94fd2e8c60 c>) [0 c+0 S1 A8])
> (const_int 1 [0x1]))))
> -----------dump end----------
Maybe I'm missing something, but I don't think this transformation is correct.
Please consider the following analysis with the emphasis on the sign bit of the
QImode operation:
r = sext:HI (xor:QI (a, b)); b IMM
a 0xxxxxxx
b 0xxxxxxx
r 00000000 0xxxxxxx
a 1xxxxxxx
b 0xxxxxxx
r 11111111 1xxxxxxx
a 0xxxxxxx
b 1xxxxxxx
r 11111111 1xxxxxxx
a 1xxxxxxx
b 1xxxxxxx
r 00000000 0xxxxxxx
r = xor:HI ((a, b); a ZEXT, b IMM
a 00000000 0xxxxxxx
b 00000000 0xxxxxxx
r 00000000 0xxxxxxx
a 00000000 1xxxxxxx
b 00000000 0xxxxxxx
r 00000000 1xxxxxxx
a 00000000 0xxxxxxx
b 11111111 1xxxxxxx
r 11111111 1xxxxxxx
a 00000000 1xxxxxxx
b 11111111 1xxxxxxx
r 11111111 0xxxxxxx
As demonstrated above, results differ when sign bit of the value a is set.
The conversion works when the value a is loaded with a sign-extend operation.
r = xor:HI ((a, b); a SEXT, b IMM
a 00000000 0xxxxxxx
b 00000000 0xxxxxxx
r 00000000 0xxxxxxx
a 11111111 1xxxxxxx
b 00000000 0xxxxxxx
r 11111111 1xxxxxxx
a 00000000 0xxxxxxx
b 11111111 1xxxxxxx
r 11111111 1xxxxxxx
a 11111111 1xxxxxxx
b 11111111 1xxxxxxx
r 00000000 0xxxxxxx
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bug target/89954] missed optimization for signed extension for x86-64
[not found] <bug-89954-4@http.gcc.gnu.org/bugzilla/>
` (2 preceding siblings ...)
2021-09-11 12:59 ` ubizjak at gmail dot com
@ 2021-09-11 14:24 ` crazylht at gmail dot com
2021-09-22 19:04 ` ubizjak at gmail dot com
` (5 subsequent siblings)
9 siblings, 0 replies; 10+ messages in thread
From: crazylht at gmail dot com @ 2021-09-11 14:24 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89954
--- Comment #6 from Hongtao.liu <crazylht at gmail dot com> ---
(In reply to Uroš Bizjak from comment #5)
> (In reply to Hongtao.liu from comment #4)
> > It looks like there's splitter in aarch64 which combines
> > load+xor+zero_extend to zero_extend(mem) + xor, x86 doesn't have. The simple
> > way is to add corresponding define_split for x86.
> >
> > ---------x86 dump-----------
> > Failed to match this instruction:
> > (set (reg:SI 85)
> > (sign_extend:SI (xor:QI (mem/c:QI (symbol_ref:DI ("c") [flags 0x2]
> > <var_decl 0x7f94fd2e8c60 c>) [0 c+0 S1 A8])
> > (const_int 1 [0x1]))))
> > -----------dump end----------
>
> Maybe I'm missing something, but I don't think this transformation is
> correct. Please consider the following analysis with the emphasis on the
> sign bit of the QImode operation:
>
> r = sext:HI (xor:QI (a, b)); b IMM
>
> a 0xxxxxxx
> b 0xxxxxxx
> r 00000000 0xxxxxxx
>
> a 1xxxxxxx
> b 0xxxxxxx
> r 11111111 1xxxxxxx
>
> a 0xxxxxxx
> b 1xxxxxxx
> r 11111111 1xxxxxxx
>
> a 1xxxxxxx
> b 1xxxxxxx
> r 00000000 0xxxxxxx
>
> r = xor:HI ((a, b); a ZEXT, b IMM
movzbl maybe confuse you, there's no ZEXT here, just movqi_internal. also at
the stage of combine it is xorqi_1 not xorsi_1, and b here is const_int 1, the
most significant bit is zero.
here's dump before combine.
(insn 5 2 6 2 (set (reg:QI 87 [ c ])
(mem/c:QI (symbol_ref:DI ("c") [flags 0x2] <var_decl 0x7ff23f550c60
c>) [0 c+0 S1 A8])) "test.c":4:14 79 {*movqi_internal}
(nil))
(insn 6 5 7 2 (parallel [
(set (reg:QI 86)
(xor:QI (reg:QI 87 [ c ])
(const_int 1 [0x1])))
(clobber (reg:CC 17 flags))
]) "test.c":4:14 556 {*xorqi_1}
(expr_list:REG_DEAD (reg:QI 87 [ c ])
(expr_list:REG_UNUSED (reg:CC 17 flags)
(expr_list:REG_EQUAL (xor:QI (mem/c:QI (symbol_ref:DI ("c") [flags
0x2] <var_decl 0x7ff23f550c60 c>) [0 c+0 S1 A8])
(const_int 1 [0x1]))
(nil)))))
(insn 7 6 12 2 (set (reg:SI 85)
(sign_extend:SI (reg:QI 86))) "test.c":4:14 156 {extendqisi2}
(expr_list:REG_DEAD (reg:QI 86)
>
> a 00000000 0xxxxxxx
> b 00000000 0xxxxxxx
> r 00000000 0xxxxxxx
>
> a 00000000 1xxxxxxx
> b 00000000 0xxxxxxx
> r 00000000 1xxxxxxx
>
> a 00000000 0xxxxxxx
> b 11111111 1xxxxxxx
> r 11111111 1xxxxxxx
>
> a 00000000 1xxxxxxx
> b 11111111 1xxxxxxx
> r 11111111 0xxxxxxx
>
> As demonstrated above, results differ when sign bit of the value a is set.
>
> The conversion works when the value a is loaded with a sign-extend operation.
>
> r = xor:HI ((a, b); a SEXT, b IMM
>
> a 00000000 0xxxxxxx
> b 00000000 0xxxxxxx
> r 00000000 0xxxxxxx
>
> a 11111111 1xxxxxxx
> b 00000000 0xxxxxxx
> r 11111111 1xxxxxxx
>
> a 00000000 0xxxxxxx
> b 11111111 1xxxxxxx
> r 11111111 1xxxxxxx
>
> a 11111111 1xxxxxxx
> b 11111111 1xxxxxxx
> r 00000000 0xxxxxxx
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bug target/89954] missed optimization for signed extension for x86-64
[not found] <bug-89954-4@http.gcc.gnu.org/bugzilla/>
` (3 preceding siblings ...)
2021-09-11 14:24 ` crazylht at gmail dot com
@ 2021-09-22 19:04 ` ubizjak at gmail dot com
2021-09-23 5:06 ` crazylht at gmail dot com
` (4 subsequent siblings)
9 siblings, 0 replies; 10+ messages in thread
From: ubizjak at gmail dot com @ 2021-09-22 19:04 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89954
--- Comment #7 from Uroš Bizjak <ubizjak at gmail dot com> ---
Created attachment 51496
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=51496&action=edit
Prototype patch
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bug target/89954] missed optimization for signed extension for x86-64
[not found] <bug-89954-4@http.gcc.gnu.org/bugzilla/>
` (4 preceding siblings ...)
2021-09-22 19:04 ` ubizjak at gmail dot com
@ 2021-09-23 5:06 ` crazylht at gmail dot com
2021-09-23 5:17 ` crazylht at gmail dot com
` (3 subsequent siblings)
9 siblings, 0 replies; 10+ messages in thread
From: crazylht at gmail dot com @ 2021-09-23 5:06 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89954
--- Comment #8 from Hongtao.liu <crazylht at gmail dot com> ---
(In reply to Uroš Bizjak from comment #7)
> Created attachment 51496 [details]
> Prototype patch
+;; convert (sign_extend:WIDE (any_logic:NARROW (memory, immediate)))
+;; to (any_logic:WIDE (sign_extend (memory)), (sign_extend (immediate))).
+;; This eliminates sign extension after logic operation.
+
+(define_split
+ [(set (match_operand:SWI248 0 "register_operand")
+ (sign_extend:SWI248
+ (any_logic:QI (match_operand:QI 1 "memory_operand")
+ (match_operand:QI 2 "const_int_operand"))))]
+ ""
+ [(set (match_dup 0) (any_logic:SWI248 (match_dup 0) (match_dup 2)))]
+ "convert_move (operands[0], operands[1], false);")
Shouldn't we make sure (any_logic op1 const_int) would change sign bit of op1?
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bug target/89954] missed optimization for signed extension for x86-64
[not found] <bug-89954-4@http.gcc.gnu.org/bugzilla/>
` (5 preceding siblings ...)
2021-09-23 5:06 ` crazylht at gmail dot com
@ 2021-09-23 5:17 ` crazylht at gmail dot com
2021-09-30 8:00 ` ubizjak at gmail dot com
` (2 subsequent siblings)
9 siblings, 0 replies; 10+ messages in thread
From: crazylht at gmail dot com @ 2021-09-23 5:17 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89954
--- Comment #9 from Hongtao.liu <crazylht at gmail dot com> ---
(In reply to Hongtao.liu from comment #8)
> (In reply to Uroš Bizjak from comment #7)
> > Created attachment 51496 [details]
> > Prototype patch
>
> +;; convert (sign_extend:WIDE (any_logic:NARROW (memory, immediate)))
> +;; to (any_logic:WIDE (sign_extend (memory)), (sign_extend (immediate))).
> +;; This eliminates sign extension after logic operation.
> +
> +(define_split
> + [(set (match_operand:SWI248 0 "register_operand")
> + (sign_extend:SWI248
> + (any_logic:QI (match_operand:QI 1 "memory_operand")
> + (match_operand:QI 2 "const_int_operand"))))]
> + ""
> + [(set (match_dup 0) (any_logic:SWI248 (match_dup 0) (match_dup 2)))]
> + "convert_move (operands[0], operands[1], false);")
>
> Shouldn't we make sure (any_logic op1 const_int) would change sign bit of
> op1?
Typo, would not change sign bit of op1.
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bug target/89954] missed optimization for signed extension for x86-64
[not found] <bug-89954-4@http.gcc.gnu.org/bugzilla/>
` (6 preceding siblings ...)
2021-09-23 5:17 ` crazylht at gmail dot com
@ 2021-09-30 8:00 ` ubizjak at gmail dot com
2021-09-30 17:35 ` cvs-commit at gcc dot gnu.org
2021-09-30 17:38 ` ubizjak at gmail dot com
9 siblings, 0 replies; 10+ messages in thread
From: ubizjak at gmail dot com @ 2021-09-30 8:00 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89954
--- Comment #10 from Uroš Bizjak <ubizjak at gmail dot com> ---
(In reply to Hongtao.liu from comment #9)
> (In reply to Hongtao.liu from comment #8)
> > (In reply to Uroš Bizjak from comment #7)
> > > Created attachment 51496 [details]
> > > Prototype patch
> >
> > +;; convert (sign_extend:WIDE (any_logic:NARROW (memory, immediate)))
> > +;; to (any_logic:WIDE (sign_extend (memory)), (sign_extend (immediate))).
> > +;; This eliminates sign extension after logic operation.
> > +
> > +(define_split
> > + [(set (match_operand:SWI248 0 "register_operand")
> > + (sign_extend:SWI248
> > + (any_logic:QI (match_operand:QI 1 "memory_operand")
> > + (match_operand:QI 2 "const_int_operand"))))]
> > + ""
> > + [(set (match_dup 0) (any_logic:SWI248 (match_dup 0) (match_dup 2)))]
> > + "convert_move (operands[0], operands[1], false);")
> >
> > Shouldn't we make sure (any_logic op1 const_int) would change sign bit of
> > op1?
> Typo, would not change sign bit of op1.
It doesn't matter, since all constants are implicitly sign-extended by the
insn.
For example, convert sext:HI (xor:QI (op1, -1)) to xor:HI (sext:HI (op1), -1):
(a = op1, b = -1)
sext:HI (xor:QI (op1, -1)):
a 0xxxxxxx
b 1xxxxxxx
r 11111111 1xxxxxxx
a 1xxxxxxx
b 1xxxxxxx
r 00000000 0xxxxxxx
xor:HI (sext:HI (op1), -1):
a 00000000 0xxxxxxx
b 11111111 1xxxxxxx
r 11111111 1xxxxxxx
a 11111111 1xxxxxxx
b 11111111 1xxxxxxx
r 00000000 0xxxxxxx
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bug target/89954] missed optimization for signed extension for x86-64
[not found] <bug-89954-4@http.gcc.gnu.org/bugzilla/>
` (7 preceding siblings ...)
2021-09-30 8:00 ` ubizjak at gmail dot com
@ 2021-09-30 17:35 ` cvs-commit at gcc dot gnu.org
2021-09-30 17:38 ` ubizjak at gmail dot com
9 siblings, 0 replies; 10+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2021-09-30 17:35 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89954
--- Comment #11 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Uros Bizjak <uros@gcc.gnu.org>:
https://gcc.gnu.org/g:6f4459c478b1c09e4b5e7d629fbf46d2a4fe4560
commit r12-3991-g6f4459c478b1c09e4b5e7d629fbf46d2a4fe4560
Author: Uros Bizjak <ubizjak@gmail.com>
Date: Thu Sep 30 19:33:49 2021 +0200
i386: Eliminate sign extension after logic operation [PR89954]
Convert (sign_extend:WIDE (any_logic:NARROW (memory, immediate)))
to (any_logic:WIDE (sign_extend (memory)), (sign_extend (immediate))).
This eliminates sign extension after logic operation.
2021-09-30 Uroš Bizjak <ubizjak@gmail.com>
gcc/
PR target/89954
* config/i386/i386.md
(sign_extend:WIDE (any_logic:NARROW (memory, immediate))
splitters):
New splitters.
gcc/testsuite/
PR target/89954
* gcc.target/i386/pr89954.c: New test.
^ permalink raw reply [flat|nested] 10+ messages in thread* [Bug target/89954] missed optimization for signed extension for x86-64
[not found] <bug-89954-4@http.gcc.gnu.org/bugzilla/>
` (8 preceding siblings ...)
2021-09-30 17:35 ` cvs-commit at gcc dot gnu.org
@ 2021-09-30 17:38 ` ubizjak at gmail dot com
9 siblings, 0 replies; 10+ messages in thread
From: ubizjak at gmail dot com @ 2021-09-30 17:38 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89954
Uroš Bizjak <ubizjak at gmail dot com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Assignee|unassigned at gcc dot gnu.org |ubizjak at gmail dot com
Target Milestone|--- |12.0
Resolution|--- |FIXED
Status|NEW |RESOLVED
--- Comment #12 from Uroš Bizjak <ubizjak at gmail dot com> ---
Implemented for gcc-12.
^ permalink raw reply [flat|nested] 10+ messages in thread