On 7/16/24 12:26 PM, Vineet Gupta wrote:
Hi Jeff,
On 7/13/24 07:54, Vineet Gupta wrote:
Changes since v4:
- No functional changes.
- Use int iterator and attr to implement expanders in md
(inspired by loongarch patch. Thx Xi Ruoyao)
Changes since v3:
- Remove '*' from define_insn for fclass
- Remove the dummy expander for fclass.
- De-duplicate the expanders code by using a helper which takes fclass
val.
Changes since v2:
- fclass define_insn tightened to check op0 mode "X" with additional
expander w/o mode for callers.
- builtins expander explicit mode check and FAIL if mode not appropriate.
- subreg promoted handling to elide potential extension of ret val.
- Added isinf builtin with bimodal retun value as others.
Changes since v1:
- Removed UNSPEC_{INFINITE,ISNORMAL}
- Don't hardcode SI in patterns, try to keep X to avoid potential
sign extension pitfalls. Implementation wise requires skipping
:MODE specifier in match_operand which is flagged as missing mode
warning.
---
Currently thsse builtins use float compare instructions which require
FP flags to be save/restore around them.
Our perf team complained this could be costly in uarch.
RV Base ISA already has FCLASS.{d,s,h} instruction to compare/identify FP
values w/o disturbing FP exception flags.
Coincidently, upstream very recently got support for the corresponding
optabs. So this just requires wiring up in the backend.
Tested for rv64, one additioal failure g++.dg/opt/pr107569.C needs
upstream ranger fix for the new optab.
Pre-commit CI reports [1] three additional failures which are related to
ranger patches in this space, same as what Loongarch folks are seeing [2]
FAIL: gcc.dg/tree-ssa/range-sincos.c scan-tree-dump-not evrp "link_error"
FAIL: gcc.dg/tree-ssa/vrp-float-abs-1.c scan-tree-dump-not evrp "link_error"
FAIL: g++.dg/opt/pr107569.C -std=gnu++20 scan-tree-dump-times vrp1
"return 1;" 2
Here's my testing with trunk of this morning, which corroborates with
the same:
2024-07-16 fec38d7987dd rtl-ssa: Fix removal of order_nodes
[PR115929]
rv64imafdc_zba_zbb_zbs_zicond_zfa/ lp64d/ medlow | 321 / 56 | 0
/ 0 | 6 / 1 | # upstream
rv64imafdc_zba_zbb_zbs_zicond_zfa/ lp64d/ medlow | 323 / 58 | 1
/ 1 | 6 / 1 | # upstream + mypatch
rv64imafdc_zba_zbb_zbs_zicond_zfa/ lp64d/ medlow | 321 / 56 | 0
/ 0 | 6 / 1 | # upstream + mypatch + ranger patches
Having said that I don't mind if you prefer the ranger patches make it
in first.
Thx,
-Vineet
[1] https://github.com/ewlu/gcc-precommit-ci/issues/1908
[2] https://gcc.gnu.org/pipermail/gcc-patches/2024-July/656972.html
Thanks. I didn't make the connection between Hao's patch and these
failures, particularly the range-sincos.c case. Not sure how I could
have missed it given that test is explicitly mentioned in the first
paragraph of Hao's message.
I actually gave Hao a bit of feeback last week on a couple things that
didn't look quite right. So hopefully we'll have that patch moving
forward shortly.
Let's get closure on Hao's patch, then this one can go forward.
Thanks!
jeff
