JIT: Fix invalid smnegl transformation #93003
Conversation
The newly introduced TryLowerNegToMulLongOp would always lower to a signed multiplication, even for unsigned multiplications. Also apply a couple of other fixes to the transformation: * Replace uses of GenTree::ReplaceWith with LIR::Use::ReplaceWith * Add some necessary interference checks Fix dotnet#92537
dotnet-issue-labeler
bot
added
the
area-CodeGen-coreclr
|
/azp run Fuzzlyn |
ghost
assigned 
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsThe newly introduced TryLowerNegToMulLongOp would always lower to a signed multiplication, even for unsigned multiplications. Also apply a couple of other fixes to the transformation:
Fix #92537
|
|
Azure Pipelines successfully started running 1 pipeline(s). |
| if (!IsInvariantInRange(mul, op)) | ||
| return nullptr; |
There was a problem hiding this comment.
This interference check is needed, otherwise we will produce bad codegen for a case like
private static long Foo(int a, int b)
{
return (long)a * (long)b + Bar(ref a);
}
[MethodImpl(MethodImplOptions.NoInlining)]
private static long Bar(ref int x)
{
x = 0;
return 0;
}
I pointed that out in the PR #91886 (comment) but seems like we still missed some cases. |
|
Interestingly there are no diffs, so seems we don't have coverage for this scenario. Can you please add a test case too? |
Added the test, can you PTAL? |
|
superpmi failures are known. |
The newly introduced TryLowerNegToMulLongOp would always lower to a signed multiplication, even for unsigned multiplications.
Also apply a couple of other fixes to the transformation:
Fix #92537