[InstCombine] Adding Z / (1.0 / Y) => (Y * Z)

[raghesh]

Summary

This is a special case of Z / (X / Y) => (Y * Z) / X, with X = 1.0. The m_OneUse check is avoided because even in the case of the multiple uses for 1.0/Y, the number of instructions remain the same and a division is replaced by a multiplication.

The following testcase is updated.

test/Transforms/InstCombine/fdiv.ll

[spatel]

llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp

1249

I don't think these constant checks are necessary. (I'm not sure why they are part of the existing code either - maybe some corner case with denorm values?)

If Y is a constant, 1.0/C will get constant folded and Z/C' will eventually get folded too, so this should just get to canonical form quicker. The 'Z' is constant pattern is similar.

FDiv with constant operand are generally handled within:
foldFDivConstantDivisor()
foldFDivConstantDividend()

llvm/test/Transforms/InstCombine/fdiv-to-fmul-opt.ll

15–21 ↗

(On Diff #236530)

1. Add a test within test/Transforms/InstCombine/fdiv.ll instead of creating a new file.
2. Create minimal test(s) to show the transform; in particular, this transform does not require any of alloca, load, store, so the test shouldn't have those.
3. Use this script to auto-generate the CHECK lines:

utils/update_test_checks.py

4. I prefer that you add the test *without* this code patch showing the current IR produced as a preliminary patch. That way, we will see the diff from the new transform when this patch is committed.
5. If you need help committing/pushing the patches, let me know.

[raghesh]

llvm/test/Transforms/InstCombine/fdiv-to-fmul-opt.ll

15–21 ↗

(On Diff #236530)

Thanks for the commets. I will address these.

However, point number 4 is not clear to me. Could you please explain it?

Yes. As I don't have commit access, I will need your help in committing it.

[craig.topper]

llvm/test/Transforms/InstCombine/fdiv-to-fmul-opt.ll

15–21 ↗

(On Diff #236530)

For point 4. Create a new review containing just the new test case with no other changes. This will show what the current behavior is. Then rebase this review to be on top of that so that this review just shows how the test changes with the transformation.

[raghesh]

llvm/test/Transforms/InstCombine/fdiv-to-fmul-opt.ll

15–21 ↗

(On Diff #236530)

Test case added with a new reivew https://reviews.llvm.org/D72388

[spatel]

llvm/test/Transforms/InstCombine/fdiv-to-fmul-opt.ll

15–21 ↗

(On Diff #236530)

Thanks - that was pushed here:
rG5dfd52398f5c

You can now rebase/update this patch.

[raghesh]

This revision is updated with the expected change in the testcase test/Transforms/InstCombine/fdiv-to-fmul-opt.ll. Other comments are addressed too.

[spatel]

LGTM