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

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/test/Transforms/InstCombine/fdiv-to-fmul-opt.ll:15-21
+define dso_local double @fun() #0 {
+  %r1 = alloca double, align 8
+  %r2 = alloca double, align 8
+  %r3 = alloca double, align 8
+  %r4 = alloca double, align 8
+  %r5 = alloca double, align 8
+  %r6 = alloca double, align 8
----------------
raghesh wrote:
> craig.topper wrote:
> > raghesh wrote:
> > > spatel wrote:
> > > > 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.
> > > 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.
> > 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.
> Test case added with a new reivew https://reviews.llvm.org/D72388
Thanks - that was pushed here:
rG5dfd52398f5c

You can now rebase/update this patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72319/new/

https://reviews.llvm.org/D72319