Page MenuHomePhabricator

[SelectionDAG] Add helper function to check whether a SDValue is neutral element. NFC.
ClosedPublic

Authored by fakepaper56 on Sep 14 2022, 8:19 AM.

Details

Summary

Using this helper makes work about neutral elements more easier. Although I only
find one case now, I think it will have more chance to be used since so many
combine works are related to neutral elements.

Diff Detail

Event Timeline

fakepaper56 created this revision.Sep 14 2022, 8:19 AM
fakepaper56 requested review of this revision.Sep 14 2022, 8:19 AM

Please can you look at whether this is compatible with something similar we already have in foldSelectWithIdentityConstant in DAGCombiner.cpp?

fakepaper56 added a comment.EditedSep 14 2022, 4:21 PM

I am not sure what is the compatibility issue. I think foldSelectWithIdentityConstant could be also used for operations which not have communicative property (e.g. div).

I am not sure what is the compatibility issue. I think foldSelectWithIdentityConstant could be also used for operations which not have communicative property (e.g. div).

I#m sorry - compatibility was a poor word to use - my hope is that we can remove the 'isIdentityConstantForOpcode' lambda inside foldSelectWithIdentityConstant and use isNeutralConstant instead - either as part of this patch or a future patch.

llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
10750

Can you add a OperandNo argument as well? It can default to 0 if necessary. This will allow us to handle sub/fsub, div/fdiv and shifts in future patches.

llvm::isNeutralConstant(unsigned Opcode, SDNodeFlags Flags, SDValue V, unsigned OperandNo = 0)

Rebase and address RKSimon's comment. I do not replace foldSelectWithIdentityConstant
since it makes some axv512 test fails.

RKSimon added inline comments.Sep 26 2022, 2:45 AM
llvm/include/llvm/CodeGen/SelectionDAGNodes.h
1696

OperandNo

llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
10783

Missing OperandNo == 1 check

Address RKSimon's comment.

fakepaper56 marked an inline comment as done.Sep 27 2022, 8:02 PM
fakepaper56 added inline comments.
llvm/include/llvm/CodeGen/SelectionDAGNodes.h
1696

Done.

llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
10783

Done.

Cheers - would it be possible to add additional riscv test coverage now that you're handling a lot more than just fadd?

fakepaper56 marked an inline comment as done.Sep 28 2022, 6:33 AM

Actually, there is no any new functional change in RISC-V. Those new opcodes are filtered out by BinOpToRVVReduce.

RKSimon accepted this revision.Sep 29 2022, 6:01 AM

OK - LGTM

This revision is now accepted and ready to land.Sep 29 2022, 6:01 AM
This revision was landed with ongoing or failed builds.Sep 29 2022, 8:29 PM
This revision was automatically updated to reflect the committed changes.