[NFC] Prefer isSafeToSpeculativelyExecute to div/rem

[AtariDreams]

No description provided.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: AtariDreams (AtariDreams)

Changes

---

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

1 Files Affected:

• (modified) llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp (+1-3)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
index 8818369e79452b..fa768554159f43 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
@@ -3800,11 +3800,9 @@ Instruction *InstCombinerImpl::visitSelectInst(SelectInst &SI) {
   // Try to simplify a binop sandwiched between 2 selects with the same
   // condition. This is not valid for div/rem because the select might be
   // preventing a division-by-zero.
-  // TODO: A div/rem restriction is conservative; use something like
-  //       isSafeToSpeculativelyExecute().
   // select(C, binop(select(C, X, Y), W), Z) -> select(C, binop(X, W), Z)
   BinaryOperator *TrueBO;
-  if (match(TrueVal, m_OneUse(m_BinOp(TrueBO))) && !TrueBO->isIntDivRem()) {
+  if (match(TrueVal, m_OneUse(m_BinOp(TrueBO))) && isSafeToSpeculativelyExecute(TrueBO)) {
     if (auto *TrueBOSI = dyn_cast<SelectInst>(TrueBO->getOperand(0))) {
       if (TrueBOSI->getCondition() == CondVal) {
         replaceOperand(*TrueBO, 0, TrueBOSI->getTrueValue());

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[topperc]

This isn't NFC. isSafeToSpeculativelyExecute checks the operands.

[dtcxzyw]

It is not a NFC. Please add some proofs and tests for this.

See

llvm-project/llvm/lib/Analysis/ValueTracking.cpp

Lines 6541 to 6604 in db0ed55

	bool llvm::isSafeToSpeculativelyExecute(const Instruction *Inst,
	const Instruction *CtxI,
	AssumptionCache *AC,
	const DominatorTree *DT,
	const TargetLibraryInfo *TLI) {
	return isSafeToSpeculativelyExecuteWithOpcode(Inst->getOpcode(), Inst, CtxI,
	AC, DT, TLI);
	}
	bool llvm::isSafeToSpeculativelyExecuteWithOpcode(
	unsigned Opcode, const Instruction *Inst, const Instruction *CtxI,
	AssumptionCache *AC, const DominatorTree *DT,
	const TargetLibraryInfo *TLI) {
	#ifndef NDEBUG
	if (Inst->getOpcode() != Opcode) {
	// Check that the operands are actually compatible with the Opcode override.
	auto hasEqualReturnAndLeadingOperandTypes =
	[](const Instruction *Inst, unsigned NumLeadingOperands) {
	if (Inst->getNumOperands() < NumLeadingOperands)
	return false;
	const Type *ExpectedType = Inst->getType();
	for (unsigned ItOp = 0; ItOp < NumLeadingOperands; ++ItOp)
	if (Inst->getOperand(ItOp)->getType() != ExpectedType)
	return false;
	return true;
	};
	assert(!Instruction::isBinaryOp(Opcode) ||
	hasEqualReturnAndLeadingOperandTypes(Inst, 2));
	assert(!Instruction::isUnaryOp(Opcode) ||
	hasEqualReturnAndLeadingOperandTypes(Inst, 1));
	}
	#endif
	switch (Opcode) {
	default:
	return true;
	case Instruction::UDiv:
	case Instruction::URem: {
	// x / y is undefined if y == 0.
	const APInt *V;
	if (match(Inst->getOperand(1), m_APInt(V)))
	return *V != 0;
	return false;
	}
	case Instruction::SDiv:
	case Instruction::SRem: {
	// x / y is undefined if y == 0 or x == INT_MIN and y == -1
	const APInt *Numerator, *Denominator;
	if (!match(Inst->getOperand(1), m_APInt(Denominator)))
	return false;
	// We cannot hoist this division if the denominator is 0.
	if (*Denominator == 0)
	return false;
	// It's safe to hoist if the denominator is not 0 or -1.
	if (!Denominator->isAllOnes())
	return true;
	// At this point we know that the denominator is -1. It is safe to hoist as
	// long we know that the numerator is not INT_MIN.
	if (match(Inst->getOperand(0), m_APInt(Numerator)))
	return !Numerator->isMinSignedValue();
	// The numerator *might* be MinSignedValue.
	return false;
	}
	case Instruction::Load: {