public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug target/108038] New: GCC generates poor code for select of consecutive constants
@ 2022-12-09 21:17 law at gcc dot gnu.org
2022-12-14 15:51 ` [Bug target/108038] GCC generates poor code for select of consecutive constants on riscv-64 rzinsly at ventanamicro dot com
0 siblings, 1 reply; 2+ messages in thread
From: law at gcc dot gnu.org @ 2022-12-09 21:17 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108038
Bug ID: 108038
Summary: GCC generates poor code for select of consecutive
constants
Product: gcc
Version: 13.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: target
Assignee: unassigned at gcc dot gnu.org
Reporter: law at gcc dot gnu.org
CC: rzinsly at ventanamicro dot com
Target Milestone: ---
Target: riscv-64
This testcase:
typedef signed long int __int64_t;
#define X(OP, OPN) __int64_t test##OPN(__int64_t x, __int64_t y) { __int64_t
cmp; cmp = (x OP y) ? 2 : 3; return cmp; }
X(>,GT)
X(<,LT)
typedef unsigned long int __uint64_t;
#define XU(OP, OPN) __uint64_t test##OPN(__uint64_t x, __uint64_t y) {
__uint64_t cmp; cmp = (x OP y) ? 2 : 3; return cmp; }
X(>,GTU)
X(<,LTU)
Compiled for rv64 with -O2 -march=rv64gc_zba gets this code for the first test.
The others are similar in having the extraneous xori.
testGT:
sgt a0,a0,a1 # 25 [c=4 l=4] *sgt_didi
xori a0,a0,1 # 27 [c=4 l=4] xordi3/1
addi a0,a0,2 # 16 [c=4 l=4] adddi3/1
ret # 35 [c=0 l=4] simple_return
But ISTM we could generate this instead:
testGT:
sgt a0,a1,a0 # 24 [c=4 l=4] *riscv.md:2535
addi a0,a0,2 # 16 [c=4 l=4] adddi3/1
ret # 32 [c=0 l=4] simple_return
I think the key is this:
Trying 25 -> 27:
25: r139:DI=r140:DI>r141:DI
REG_DEAD r141:DI
REG_DEAD r140:DI
27: r134:DI=r139:DI^0x1
REG_DEAD r139:DI
Failed to match this instruction:
(set (reg:DI 134 [ <retval> ])
(le:DI (reg:DI 140)
(reg:DI 141)))
We can invert the condition and swap the operands with something like this:
(define_insn ""
[(set (match_operand:GPR 0 "register_operand" "=r")
(any_le:GPR (match_operand:X 1 "register_operand" " r")
(match_operand:X 2 "register_operand" "r")))]
""
"sgt<u>\t%0,%2,%1"
[(set_attr "type" "slt")
(set_attr "mode" "<X:MODE>")])
We probably need another pattern to handle the LT/LTU cases which want to
match:
rying 24 -> 25:
24: r139:DI=r140:DI<r141:DI
REG_DEAD r141:DI
REG_DEAD r140:DI
25: r138:SI=r139:DI==0
REG_DEAD r139:DI
Failed to match this instruction:
(set (reg:SI 138)
(ge:SI (reg:DI 140)
(reg:DI 141)))
Again, I think by inverting the condition and swapping the operands we should
be good to go.
^ permalink raw reply [flat|nested] 2+ messages in thread
* [Bug target/108038] GCC generates poor code for select of consecutive constants on riscv-64
2022-12-09 21:17 [Bug target/108038] New: GCC generates poor code for select of consecutive constants law at gcc dot gnu.org
@ 2022-12-14 15:51 ` rzinsly at ventanamicro dot com
0 siblings, 0 replies; 2+ messages in thread
From: rzinsly at ventanamicro dot com @ 2022-12-14 15:51 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108038
--- Comment #1 from Raphael M Zinsly <rzinsly at ventanamicro dot com> ---
(In reply to Jeffrey A. Law from comment #0)
> testGT:
> sgt a0,a0,a1 # 25 [c=4 l=4] *sgt_didi
> xori a0,a0,1 # 27 [c=4 l=4] xordi3/1
> addi a0,a0,2 # 16 [c=4 l=4] adddi3/1
> ret # 35 [c=0 l=4] simple_return
>
>
> But ISTM we could generate this instead:
>
> testGT:
> sgt a0,a1,a0 # 24 [c=4 l=4] *riscv.md:2535
> addi a0,a0,2 # 16 [c=4 l=4] adddi3/1
> ret # 32 [c=0 l=4] simple_return
Unless I'm missing something this seems wrong: 'sgt a0,a0,a1' with 'xori
a0,a0,1' would be equivalent to 'sge a0,a1,a0' or to 'sle a0,a0,a1' and not to
'sgt a0,a1,a0'.
For x == y the result would be 2 with your asm.
> We can invert the condition and swap the operands with something like this:
>
> (define_insn ""
> [(set (match_operand:GPR 0 "register_operand" "=r")
> (any_le:GPR (match_operand:X 1 "register_operand" " r")
> (match_operand:X 2 "register_operand" "r")))]
> ""
> "sgt<u>\t%0,%2,%1"
> [(set_attr "type" "slt")
> (set_attr "mode" "<X:MODE>")])
This works as you intended. However, I think the right patter should be:
(define_insn "*sge<u>_<X:mode><GPR:mode>_inv"
[(set (match_operand:GPR 0 "register_operand" "=r")
(any_le:GPR (match_operand:X 1 "register_operand" " r")
(match_operand:X 2 "register_operand" "r")))]
""
"sge<u>\t%0,%2,%1"
[(set_attr "type" "slt")
(set_attr "mode" "<X:MODE>")])
It produces the output:
testGT:
sge a0,a1,a0 # 24 [c=4 l=4] *sge_didi_inv
addi a0,a0,2 # 16 [c=4 l=4] adddi3/1
ret # 32 [c=0 l=4] simple_return
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-12-14 15:51 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-09 21:17 [Bug target/108038] New: GCC generates poor code for select of consecutive constants law at gcc dot gnu.org
2022-12-14 15:51 ` [Bug target/108038] GCC generates poor code for select of consecutive constants on riscv-64 rzinsly at ventanamicro dot com
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).