Repository navigation
Unnecessary add emitted for repeated multiplication #75413
Description
Description
Noted during work with #74020 , that a series of multiplications generates unnecessary operations. Here is the example
I played with the code for a little bit and figured out that GT_MUL nodes were being optimized, but if GT_MUL op had the right child as a value of power of 2, then the operation converted into GT_LSH (here is the code).
So if we have a series of multiplications by 2 (for simplicity) as an example above, then we end up with a series of GT_LSH operations.
I asume that we can fold the series into one GT_LSH operation with a proper right hand value.
Reproduction Steps
Use snipets like this:
public int T(int u1) {
return u1*2*2*2*2;
}Expected behavior
public int T(int u1) {
return u1*2*2*2*2;
}Generates:
G_M63441_IG02:
mov eax, ecx
shl eax, 4
Actual behavior
public int T(int u1) {
return u1*2*2*2*2;
}Generates:
C.T(Int32)
L0000: add edx, edx
L0002: add edx, edx
L0004: lea eax, [rdx+rdx]
L0007: add eax, eax
L0009: ret
category:cq
theme:basic-cq
skill-level:beginner
cost:small
impact:small
Activity
Draft fix for a series of multiplications (dotnet#75413)
GSPP commentedon Oct 4, 2022
Hm, I might have posted my annotation to the wrong place (I'm not too familiar with the GitHub code review system). Repeating it here:
SkiFoD@e248d75#diff-5b83397bbbdd17bb9457998b520fdaaa474d165390985b66f32371561b6d0bacR12766
Is it possible that this computation would overflow? It could overflow like 30+30 > 32. But could this also cause undefined behavior in the C code of the JIT when computing int.MaxValue + int.MaxValue? Is such a large shift amount possible?
SkiFoD commentedon Oct 5, 2022
Your concerns are correct. I'm not sure if such a situation possible so the op tree would have int.max shifts. I think we need an opinion of someone from the team.
Draft fix for a series of multiplications (dotnet#75413)
danmoseley commentedon Dec 21, 2022
@EgorBo I think there is a question above.
Fix for division by zero (dotnet#75413)
Moved the logic to fgOptimizeCommutativeArithmetic (dotnet#75413)