[InstCombine] add extra-use tests for fmul+sqrt; NFC

[venkataramanan.kumar.llvm]

Summary

[InstCombine] add extra-use tests for fmul+sqrt; NFC

[spatel]

llvm/test/Transforms/InstCombine/fmul-sqrt.ll

121

This is not providing the test coverage that you intended - notice that the operands of fmul are swapped.
Please see my comment about complexity-based canonicalization in D86395.

[venkataramanan.kumar.llvm]

llvm/test/Transforms/InstCombine/fmul-sqrt.ll

121

Hi Sanjay,

The function "SimplifyAssociativeOrCommutative" is called before we try to fold x * 1/sqrt(x) to x/sqrt(x) in the function "visitFMul" .

This function swaps the operand x *1/sqrt(x) to 1/sqrt(x) *x based on complexity values.

Rule for commutative operator: LHS (more complexity value ) Binary RHS (less complexity value).
1/sqrt(x) has value 5 and x has value 2.

--Snip--
bool InstCombinerImpl::SimplifyAssociativeOrCommutative(BinaryOperator &I) {

Instruction::BinaryOps Opcode = I.getOpcode();
bool Changed = false;

  // Order operands such that they are listed from right (least complex) to
  // left (most complex).  This puts constants before unary operators before
  // binary operators.
  if (I.isCommutative() && getComplexity(I.getOperand(0)) <
      getComplexity(I.getOperand(1)))
    Changed = !I.swapOperands();

---Snip--

So looks like we don't need to match for x *1/sqrt(x) in D86726.
But for this test case patch , can we keep as an extra coverage test?

[venkataramanan.kumar.llvm]

llvm/test/Transforms/InstCombine/fmul-sqrt.ll

121

May be I should try a different test case for x * 1/sqrt(x) where x is an expression with same or higher complexity than 1/sqrt(x).

[venkataramanan.kumar.llvm]

As per the comment given by Sanjay, updated the coverage test for x*1/sqrt(x) pattern.

[spatel]

LGTM

llvm/test/Transforms/InstCombine/fmul-sqrt.ll

121

Right! Looks like you got it. Sorry if my earlier comment was not clear. I prefer to label that extra expression with a code comment (grep for "thwart" in this test directory), so other readers will have a better chance of understanding why the 'add' in this case is included in the test.

[venkataramanan.kumar.llvm]

llvm/test/Transforms/InstCombine/fmul-sqrt.ll

121

Oh!! I did not understand your earlier comment. Instead of adding "thwart", I changed the test case.

Also can you please commit on behalf of me.

[venkataramanan.kumar.llvm]

This comment was removed by venkataramanan.kumar.llvm.

[venkataramanan.kumar.llvm]

Added comment in the test case.

[spatel]

Committed here:
rG86163f885b5

Please request commit access so you're not waiting on others to push an approved patch.

llvm/test/Transforms/InstCombine/fmul-sqrt.ll

 ; CHECK-NEXT:    ret double [[SQUARED]]
 ;
   %sqrt = call fast double @llvm.sqrt.f64(double %x)
   %rsqrt = fdiv fast double 1.0, %sqrt
   %squared = fmul fast double %rsqrt, %rsqrt
   ret double %squared
 }
 
+define double @rsqrt_x_reassociate_extra_use(double %x, double * %p) {
+; CHECK-LABEL: @rsqrt_x_reassociate_extra_use(
+; CHECK-NEXT:    [[SQRT:%.*]] = call fast double @llvm.sqrt.f64(double [[X:%.*]])
+; CHECK-NEXT:    [[RSQRT:%.*]] = fdiv fast double 1.000000e+00, [[SQRT]]
+; CHECK-NEXT:    [[RES:%.*]] = fmul reassoc double [[RSQRT]], [[X:%.*]]
+; CHECK-NEXT:    store double [[RSQRT]], double* %p
+; CHECK-NEXT:    ret double [[RES]]
+;
+  %sqrt = call fast double @llvm.sqrt.f64(double %x)
+  %rsqrt = fdiv fast double 1.0, %sqrt
+  %res = fmul reassoc double %rsqrt, %x
+  store double %rsqrt, double* %p
+  ret double %res
+}
+
+define double @x_add_y_rsqrt_reassociate_extra_use(double %x, double %y,  double * %p) {
+; CHECK-LABEL: @x_add_y_rsqrt_reassociate_extra_use(
+; CHECK-NEXT:    [[ADD:%.*]] = fadd fast double %x, %y
+; CHECK-NEXT:    [[SQRT:%.*]] = call fast double @llvm.sqrt.f64(double [[ADD]])
+; CHECK-NEXT:    [[RSQRT:%.*]] = fdiv fast double 1.000000e+00, [[SQRT]]
+; CHECK-NEXT:    [[RES:%.*]] = fmul reassoc double [[ADD]], [[RSQRT]]
+; CHECK-NEXT:    store double [[RSQRT]], double* %p
+; CHECK-NEXT:    ret double [[RES]]
+;
+  %add = fadd fast double %x, %y ; thwart complexity-based canonicalization
+  %sqrt = call fast double @llvm.sqrt.f64(double %add)
+  %rsqrt = fdiv fast double 1.0, %sqrt
+  %res = fmul reassoc double %add, %rsqrt
+  store double %rsqrt, double* %p
+  ret double %res
+}
+
 define double @sqrt_divisor_squared(double %x, double %y) {
 ; CHECK-LABEL: @sqrt_divisor_squared(
 ; CHECK-NEXT:    [[TMP1:%.*]] = fmul reassoc nnan nsz double [[Y:%.*]], [[Y]]
 ; CHECK-NEXT:    [[SQUARED:%.*]] = fdiv reassoc nnan nsz double [[TMP1]], [[X:%.*]]
 ; CHECK-NEXT:    ret double [[SQUARED]]
 ;
   %sqrt = call double @llvm.sqrt.f64(double %x)
   %div = fdiv double %y, %sqrt