This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Use RISCVISD::CZERO_EQZ/CZERO_NEZ for XVentanaCondOps.
ClosedPublic

Authored by craig.topper on Jul 15 2023, 11:48 PM.

Details

Summary

This makes Zicond and XVentanaCondOps use the same code path.
The instructions have identical semantics.

Diff Detail

Event Timeline

craig.topper created this revision.Jul 15 2023, 11:48 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 15 2023, 11:48 PM
craig.topper requested review of this revision.Jul 15 2023, 11:48 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 15 2023, 11:48 PM
Herald added subscribers: eopXD, MaskRay. · View Herald Transcript
wangpc added inline comments.Jul 16 2023, 8:52 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
5951–5956

Typo: rXVentanaCondOps->XVentanaCondOps

llvm/test/CodeGen/RISCV/condops.ll
1294–1296

Regression here? (though it is the same as zicond).

1442–1444

ditto.

Fix typo

llvm/test/CodeGen/RISCV/condops.ll
1294–1296

Yes. D155328 should fix it.

wangpc accepted this revision.Jul 16 2023, 8:21 PM

LGTM.

llvm/lib/Target/RISCV/RISCVISelLowering.h
374

What about renaming these two nodes to SELECT_EQZ/NEZ with another NFC patch?

This revision is now accepted and ready to land.Jul 16 2023, 8:21 PM
wangpc added inline comments.Jul 16 2023, 8:39 PM
llvm/lib/Target/RISCV/RISCVISelLowering.h
374

SELECT_ZERO_EQZ/NEZ would be better according to their semantics.

asb added a comment.Jul 17 2023, 8:15 AM

I was going to leave moving over the similar condops until we'd resolve questions about the extra combines, given these extensions aren't "experimental" like zicond is. But this looks fine to me - thanks.

Regarding the suggestion to rename the RISCVISD::CZERO_* nodes - I'm not sure there's much value in this, as the zicond naming is the "definitive" one and it's hard to see much use of xventacondops going forwards now there's a standard extension.

This revision was landed with ongoing or failed builds.Jul 18 2023, 10:18 AM
This revision was automatically updated to reflect the committed changes.