JIT: Handle GT_JTRUE nodes during post-layout condition reversal

[amanasifkhalid]

Fixes #103252. In the (rare?) case where a BBJ_COND block still ends with a GT_JTRUE node post-layout and lowering, and reversing the block's condition would eliminate a jump, find the conditional IR preceding the GT_JTRUE node, and reverse its condition instead of trying to wrap the GT_JTRUE node in a GT_NOT, and triggering asserts.

[dotnet-policy-service]

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

[jakobbotsch]

I don't think this is quite right. There is no invariant on the JTRUE node operand after lowering, so we do need to handle the GT_NOT inserted case (or just skip this optimization for these rare cases).

I think the code should rather be something like:

if (test->OperIs(GT_JTRUE))
{
  GenTree* const cond = gtReverseCond(test->gtGetOp1());
  if (cond != test->gtGetOp1())
  {
    LIR::AsRange(block).InsertAfter(test->gtGetOp1(), cond);
    test->AsUnOp()->gtOp1 = cond;
  }
}
else
{
  GenTree* const cond = gtReverseCond(test);
  assert(cond == test);
}

Skipping the transformation when the terminator node is not JCC, JCMP or JTEST would be fine to me as well.

It might be nice to add a stress mode that LowerJTrue uses to leave more JTRUEs in place (TryLowerConditionToFlagsNode is almost able to convert all JTRUEs today, as you pointed out).

[amanasifkhalid]

I see, thank you for the suggestion! Fixed.

It might be nice to add a stress mode that LowerJTrue uses to leave more JTRUEs in place (TryLowerConditionToFlagsNode is almost able to convert all JTRUEs today, as you pointed out).

I'll add a stress mode in a follow-up PR. The lack of diffs seems to further justify exercising these cases.

[amanasifkhalid]

Failure is unrelated timeout. No diffs