Assertion `Denominator > 0 && "Denominator cannot be 0!"' failed.

[danilaml]

Looks like LoopPredication doesn't properly handle the case where all of branch weights are zero.
Run with opt -loop-predication-skip-profitability-checks=false -passes='require<scalar-evolution>,loop-mssa(loop-predication)'

target triple = "x86_64-unknown-linux-gnu"

; Function Attrs: nocallback nofree nosync willreturn
declare void @llvm.experimental.guard(i1, ...) #0

define void @foo() {
entry:
  br label %Header

Header:                                           ; preds = %Latch, %entry
  %j2 = phi i64 [ 0, %entry ], [ %j.next, %Latch ]
  call void (i1, ...) @llvm.experimental.guard(i1 false, i32 0) [ "deopt"() ]
  %j.next = add i64 %j2, 1
  br i1 false, label %Latch, label %exit

Latch:                                            ; preds = %Header
  %speculate_trip_count = icmp ult i64 %j2, 0
  br i1 %speculate_trip_count, label %Header, label %common.ret, !prof !0

common.ret:                                       ; preds = %exit, %Latch
  ret void

exit:                                             ; preds = %Header
  br label %common.ret
}

attributes #0 = { nocallback nofree nosync willreturn }

!0 = !{!"branch_weights", i32 0, i32 0}

https://godbolt.org/z/oWGjWTaTY

[danilaml]

@dexonsmith just to confirm - all-zero branch weights are perfectly legal construct, right? They don't make much sense, but if you want to attach branch weights metadata to the basic block that hasn't been executed, I guess they can come up.

[dexonsmith]

@dexonsmith just to confirm - all-zero branch weights are perfectly legal construct, right? They don't make much sense, but if you want to attach branch weights metadata to the basic block that hasn't been executed, I guess they can come up.

I don't think that's expected, no. If a basic block hasn't been executed, then no branch weights should be attached, so that BranchProbabilityInfo can make a guess.

Probably worth adding an IR verifier check.

[danilaml]

@dexonsmith BPI appears to handle zero branch weight case here:

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

Lines 430 to 434 in 7472490

	if (WeightSum == 0 || ReachableIdxs.size() == 0) {
	for (unsigned I = 0, E = TI->getNumSuccessors(); I != E; ++I)
	Weights[I] = 1;
	WeightSum = TI->getNumSuccessors();
	}

Seems to be and old concern: 6a2c71a

[llvmbot]

@dexonsmith BPI appears to handle zero branch weight case here:

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

Lines 430 to 434 in 7472490

	if (WeightSum == 0 || ReachableIdxs.size() == 0) {
	for (unsigned I = 0, E = TI->getNumSuccessors(); I != E; ++I)
	Weights[I] = 1;
	WeightSum = TI->getNumSuccessors();
	}

Seems to be and old concern: 6a2c71a

Error: Command failed due to missing milestone.

[dexonsmith]

@dexonsmith BPI appears to handle zero branch weight case here:

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

Lines 430 to 434 in 7472490

	if (WeightSum == 0 || ReachableIdxs.size() == 0) {
	for (unsigned I = 0, E = TI->getNumSuccessors(); I != E; ++I)
	Weights[I] = 1;
	WeightSum = TI->getNumSuccessors();
	}

Seems to be and old concern: 6a2c71a

Perhaps that commit wasn't implemented fully, then! Seems like a bug.

[llvmbot]

@dexonsmith BPI appears to handle zero branch weight case here:

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

Lines 430 to 434 in 7472490

	if (WeightSum == 0 || ReachableIdxs.size() == 0) {
	for (unsigned I = 0, E = TI->getNumSuccessors(); I != E; ++I)
	Weights[I] = 1;
	WeightSum = TI->getNumSuccessors();
	}

Seems to be and old concern: 6a2c71a

Perhaps that commit wasn't implemented fully, then! Seems like a bug.

Error: Command failed due to missing milestone.

[danilaml]

@dexonsmith not sure what you mean by commit not being fully implemented. And what exactly do you count as a bug here? It looks like BPI was handling the case of zero branch weight since at least 2015. I was planning to update another pass that didn't handle this case assuming that zero branch weights are legal (if unexpected). As such, I hesitate to declare them illegal, since it's unclear how many places can produce such weights.

[dexonsmith]

It looks like BPI was handling the case of zero branch weight since at least 2015. I was planning to update another pass that didn't handle this case assuming that zero branch weights are legal (if unexpected). As such, I hesitate to declare them illegal, since it's unclear how many places can produce such weights.

I agree; your plan sounds good to me.

[llvmbot]

@dexonsmith BPI appears to handle zero branch weight case here:

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

Lines 430 to 434 in 7472490

	if (WeightSum == 0 || ReachableIdxs.size() == 0) {
	for (unsigned I = 0, E = TI->getNumSuccessors(); I != E; ++I)
	Weights[I] = 1;
	WeightSum = TI->getNumSuccessors();
	}

Seems to be and old concern: 6a2c71a

Perhaps that commit wasn't implemented fully, then! Seems like a bug.

Error: Command failed due to missing milestone.