[llvm] [RISCV] Add FMA support to combineOp_VLToVWOp_VL. (PR #100454)
Craig Topper via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 24 13:27:38 PDT 2024
================
@@ -14883,19 +14904,26 @@ static SDValue combineBinOp_VLToVWBinOp_VL(SDNode *N,
while (!Worklist.empty()) {
SDNode *Root = Worklist.pop_back_val();
- if (!NodeExtensionHelper::isSupportedRoot(Root, Subtarget))
- return SDValue();
NodeExtensionHelper LHS(Root, 0, DAG, Subtarget);
NodeExtensionHelper RHS(Root, 1, DAG, Subtarget);
- auto AppendUsersIfNeeded = [&Worklist,
+ auto AppendUsersIfNeeded = [&Worklist, &Subtarget,
&Inserted](const NodeExtensionHelper &Op) {
if (Op.needToPromoteOtherUsers()) {
- for (SDNode *TheUse : Op.OrigOperand->uses()) {
+ for (SDNode::use_iterator UI = Op.OrigOperand->use_begin(),
+ UE = Op.OrigOperand->use_end();
+ UI != UE; ++UI) {
+ SDNode *TheUse = *UI;
+ if (!NodeExtensionHelper::isSupportedRoot(TheUse, Subtarget))
+ return false;
+ // We only support the first 2 operands of FMA.
+ if (UI.getOperandNo() >= 2)
----------------
topperc wrote:
I had that in the code originally with an `isFMA` function. I removed it because it was the only use of the function. I figured if anyone added a new supported root that didn't use only operand 0 and 1, they'd also have to change the creation of the two `NodeExtensionHelper` objects 12 or so lines above this. So maybe they'd notice they needed to change this too.
Upon further review, I just realized that all of the binary ops have a third passthru vector operand that we should have been checking for and excluding all along. It's very often `undef` so it will take a more detailed review to figure out how to test that case.
https://github.com/llvm/llvm-project/pull/100454
More information about the llvm-commits
mailing list