Update the place where we assert about align presence while removing jump to next instr

[kunalspathak]

We track the jmp instructions in IG as potential candidate for removal, regardless of if the IG jumps to the next IG or if the IG contains alignment. In emitRemoveJumpToNextInst(), we first iterate over these candidate jmp instructions and expect the IG shouldn't have align instruction, which is false. Only if the IG was marked as IGF_HAS_REMOVABLE_JMP, we should assert that it shouldn't end up with align instruction. Moved the assert at right place.

Fix the problem reported in #95729 (comment)

[ghost]

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Issue Details

---

We track the jmp instructions in IG as potential candidate for removal, regardless of if the IG jumps to the next IG or if the IG contains alignment. In emitRemoveJumpToNextInst(), we first iterate over these candidate jmp instructions and expect the IG shouldn't have align instruction, which is false. Only if the IG was marked as IGF_HAS_REMOVABLE_JMP, we should assert that it shouldn't end up with align instruction. Moved the assert at right place.

Fix the problem reported in #95729 (comment)

Author:	kunalspathak
Assignees:	kunalspathak
Labels:	area-CodeGen-coreclr
Milestone:	-

[kunalspathak]

@dotnet/jit-contrib

[jakobbotsch]

LGTM.

Does it mean this check for hasAlign() is unnecessary?

runtime/src/coreclr/jit/codegenlinear.cpp

Lines 747 to 751 in 69bbdb8

	// If a block was selected to place an alignment instruction because it ended
	// with a jump, do not remove jumps from such blocks.
	// Do not remove a jump between hot and cold regions.
	bool isRemovableJmpCandidate =
	!block->hasAlign() && !compiler->fgInDifferentRegions(block, block->GetTarget());

When I was looking at the issue I kind of got the feeling that this check is what normally filters out the case, but in this particular scenario didn't because we had two basic blocks mapped to the same IG.

[kunalspathak]

Does it mean this check for hasAlign() is unnecessary?

ideally yes, that seems unnecessary. We will only attempt to remove the jmp if the block has the flag on it, regardless of what instruction flag idjIsRemovableJmpCandidate says. I will remove it in this PR.