[ValueTracking] Add missing check for two-value PN recurrance matching

[ivanradanov]

When InstTy is a type like IntrinsicInst which can have a variable number of arguments, we can encounter a case where Operation will have fewer than two arguments and error at the getOperand() calls.

I think this was introduced by c11ea449e59cf

An alternative fix would be to have a version of matchTwoInputRecurrence where we pass I to it here and check I == Operation in the if instead of Operation->getNumOperands() == 2 .

[llvmbot]

@llvm/pr-subscribers-llvm-analysis

Author: Ivan R. Ivanov (ivanradanov)

Changes

When InstTy is a type like IntrinsicInst which can have a variable number of arguments, we can encounter a case where Operation will have fewer than two arguments and error at the getOperand() calls.

I think this was introduced by c11ea449e59cf

An alternative fix would be to have a version of matchTwoInputRecurrence where we pass I to it here and check I == Operation in the if instead of Operation->getNumOperands() == 2 .

---

Full diff: https://github.com/llvm/llvm-project/pull/152700.diff

1 Files Affected:

• (modified) llvm/lib/Analysis/ValueTracking.cpp (+2-1)

diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 1e70228905c33..b142fc6cc1334 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -9147,7 +9147,8 @@ static bool matchTwoInputRecurrence(const PHINode *PN, InstTy *&Inst,
     return false;
 
   for (unsigned I = 0; I != 2; ++I) {
-    if (auto *Operation = dyn_cast<InstTy>(PN->getIncomingValue(I))) {
+    if (auto *Operation = dyn_cast<InstTy>(PN->getIncomingValue(I));
+        Operation && Operation->getNumOperands() == 2) {
       Value *LHS = Operation->getOperand(0);
       Value *RHS = Operation->getOperand(1);
       if (LHS != PN && RHS != PN)

[nikic]

Can you please add a test?

[ivanradanov]

I added a test. For reference, before the patch, this is how it crashes:

AnalysisTests: src/llvm-project/llvm/include/llvm/IR/InstrTypes.h:2353: Value *llvm::CallBase::getOperand(unsigned int) const: Assertion `i_nocapture < OperandTraits<CallBase>::operands(this) && "getOperand() out of range!"' failed.

I also noticed that the == 2 does not work for intrinsics as they also have the callee argument so I changed to >= 2

[nikic]

LGTM, thanks

[nikic]

Suggested change

	; RUN: opt < %s -passes=instcombine
	; RUN: opt < %s -passes=instcombine | FileCheck %s

and generate check lines with update_test_checks.py. We prefer checking the output even for crash tests.