subject:"\[PATCH\] D144447\: \[Clang\] Teach buildFMulAdd to peek through fneg to find fmul."

[PATCH] D144447: [Clang] Teach buildFMulAdd to peek through fneg to find fmul.

2023-02-23 Thread Craig Topper via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG83cd4bea015f: [Clang] Teach buildFMulAdd to peek through 
fneg to find fmul. (authored by craig.topper).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D17

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/CodeGen/constrained-math-builtins.c
  clang/test/CodeGen/fp-contract-pragma.cpp

Index: clang/test/CodeGen/fp-contract-pragma.cpp
===
--- clang/test/CodeGen/fp-contract-pragma.cpp
+++ clang/test/CodeGen/fp-contract-pragma.cpp
@@ -89,3 +89,54 @@
   #pragma STDC FP_CONTRACT ON
   return c - a * b;
 }
+
+float fp_contract_10(float a, float b, float c) {
+// CHECK: _Z14fp_contract_10fff
+// CHECK: fneg float %a
+// CHECK: tail call float @llvm.fmuladd
+  #pragma STDC FP_CONTRACT ON
+  return -(a * b) + c;
+}
+
+float fp_contract_11(float a, float b, float c) {
+// CHECK: _Z14fp_contract_11fff
+// CHECK: fneg float %a
+// CHECK: fneg float %c
+// CHECK: tail call float @llvm.fmuladd
+  #pragma STDC FP_CONTRACT ON
+  return -(a * b) - c;
+}
+
+float fp_contract_12(float a, float b, float c) {
+// CHECK: _Z14fp_contract_12fff
+// CHECK: fneg float %a
+// CHECK: tail call float @llvm.fmuladd
+  #pragma STDC FP_CONTRACT ON
+  return c + -(a * b);
+}
+
+float fp_contract_13(float a, float b, float c) {
+// CHECK: _Z14fp_contract_13fff
+// CHECK-NOT: fneg float %a
+// CHECK: tail call float @llvm.fmuladd
+  #pragma STDC FP_CONTRACT ON
+  return c - -(a * b);
+}
+
+float fp_contract_14(float a, float b, float c) {
+// CHECK: _Z14fp_contract_14fff
+// CHECK: %[[M:.+]] = fmul float %a, %b
+// CHECK-NEXT: %add = fsub float %c, %[[M]]
+  #pragma STDC FP_CONTRACT ON
+  float d;
+  return (d = -(a * b)) + c;
+}
+
+float fp_contract_15(float a, float b, float c) {
+// CHECK: _Z14fp_contract_15fff
+// CHECK: %[[M:.+]] = fmul float %a, %b
+// CHECK-NEXT: %add = fsub float %c, %[[M]]
+  #pragma STDC FP_CONTRACT ON
+  float d;
+  return -(d = (a * b)) + c;
+}
Index: clang/test/CodeGen/constrained-math-builtins.c
===
--- clang/test/CodeGen/constrained-math-builtins.c
+++ clang/test/CodeGen/constrained-math-builtins.c
@@ -300,10 +300,17 @@
   f * f + f;
   (double)f * f - f;
   (long double)-f * f + f;
+  -(f * f) - f;
+  f + -(f * f);
 
   // CHECK: call float @llvm.experimental.constrained.fmuladd.f32(float %{{.*}}, float %{{.*}}, float %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
   // CHECK: fneg
   // CHECK: call double @llvm.experimental.constrained.fmuladd.f64(double %{{.*}}, double %{{.*}}, double %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
   // CHECK: fneg
   // CHECK: call x86_fp80 @llvm.experimental.constrained.fmuladd.f80(x86_fp80 %{{.*}}, x86_fp80 %{{.*}}, x86_fp80 %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+  // CHECK: fneg
+  // CHECK: fneg
+  // CHECK: call float @llvm.experimental.constrained.fmuladd.f32(float %{{.*}}, float %{{.*}}, float %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+  // CHECK: fneg
+  // CHECK: call float @llvm.experimental.constrained.fmuladd.f32(float %{{.*}}, float %{{.*}}, float %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
 };
Index: clang/lib/CodeGen/CGExprScalar.cpp
===
--- clang/lib/CodeGen/CGExprScalar.cpp
+++ clang/lib/CodeGen/CGExprScalar.cpp
@@ -3734,8 +3734,6 @@
 static Value* buildFMulAdd(llvm::Instruction *MulOp, Value *Addend,
const CodeGenFunction &CGF, CGBuilderTy &Builder,
bool negMul, bool negAdd) {
-  assert(!(negMul && negAdd) && "Only one of negMul and negAdd should be set.");
-
   Value *MulOp0 = MulOp->getOperand(0);
   Value *MulOp1 = MulOp->getOperand(1);
   if (negMul)
@@ -3780,31 +3778,70 @@
   if (!op.FPFeatures.allowFPContractWithinStatement())
 return nullptr;
 
+  Value *LHS = op.LHS;
+  Value *RHS = op.RHS;
+
+  // Peek through fneg to look for fmul. Make sure fneg has no users, and that
+  // it is the only use of its operand.
+  bool NegLHS = false;
+  if (auto *LHSUnOp = dyn_cast(LHS)) {
+if (LHSUnOp->getOpcode() == llvm::Instruction::FNeg &&
+LHSUnOp->use_empty() && LHSUnOp->getOperand(0)->hasOneUse()) {
+  LHS = LHSUnOp->getOperand(0);
+  NegLHS = true;
+}
+  }
+
+  bool NegRHS = false;
+  if (auto *RHSUnOp = dyn_cast(RHS)) {
+if (RHSUnOp->getOpcode() == llvm::Instruction::FNeg &&
+RHSUnOp->use_empty() && RHSUnOp->getOperand(0)->hasOneUse()) {
+  RHS = RHSUnOp->getOperand(0);
+  NegRHS = true;
+}
+  }
+
   // We have a potentially fusable op. Look for a mul on one of the operands.
   // Also, make sure that

[PATCH] D144447: [Clang] Teach buildFMulAdd to peek through fneg to find fmul.

2023-02-23 Thread Kevin P. Neal via Phabricator via cfe-commits

kpn accepted this revision.
kpn added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D17

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144447: [Clang] Teach buildFMulAdd to peek through fneg to find fmul.

2023-02-22 Thread Lang Hames via Phabricator via cfe-commits

lhames added inline comments.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3738
-  assert(!(negMul && negAdd) && "Only one of negMul and negAdd should be 
set.");
-
   Value *MulOp0 = MulOp->getOperand(0);

craig.topper wrote:
> kpn wrote:
> > If I'm reading this right it looks like the assert() wasn't needed before. 
> > Do we know why it was added in the first place?
> I don't. I've added @lhames who wrote the code originally, but's been 10 
> years.
It was probably included as a guard rail during bringup (from memory we didn't 
have any patterns in LLVM that matched both negations) and was never removed. I 
think it should be fine to remove.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D17

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144447: [Clang] Teach buildFMulAdd to peek through fneg to find fmul.

2023-02-21 Thread Craig Topper via Phabricator via cfe-commits

craig.topper added inline comments.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3738
-  assert(!(negMul && negAdd) && "Only one of negMul and negAdd should be 
set.");
-
   Value *MulOp0 = MulOp->getOperand(0);

kpn wrote:
> If I'm reading this right it looks like the assert() wasn't needed before. Do 
> we know why it was added in the first place?
I don't. I've added @lhames who wrote the code originally, but's been 10 years.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D17

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144447: [Clang] Teach buildFMulAdd to peek through fneg to find fmul.

2023-02-21 Thread Kevin P. Neal via Phabricator via cfe-commits

kpn added inline comments.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3738
-  assert(!(negMul && negAdd) && "Only one of negMul and negAdd should be 
set.");
-
   Value *MulOp0 = MulOp->getOperand(0);

If I'm reading this right it looks like the assert() wasn't needed before. Do 
we know why it was added in the first place?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D17

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144447: [Clang] Teach buildFMulAdd to peek through fneg to find fmul.

2023-02-20 Thread Craig Topper via Phabricator via cfe-commits

craig.topper created this revision.
craig.topper added reviewers: efriedma, aaron.ballman, andrew.w.kaylor, kpn, 
spatel, uweigand.
Herald added a subscriber: StephenFan.
Herald added a project: All.
craig.topper requested review of this revision.
Herald added a project: clang.

Allows us to handle expressions like -(a * b) + c

Based on the examples from D144366  that gcc 
seems to get.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D17

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/CodeGen/constrained-math-builtins.c
  clang/test/CodeGen/fp-contract-pragma.cpp

Index: clang/test/CodeGen/fp-contract-pragma.cpp
===
--- clang/test/CodeGen/fp-contract-pragma.cpp
+++ clang/test/CodeGen/fp-contract-pragma.cpp
@@ -89,3 +89,54 @@
   #pragma STDC FP_CONTRACT ON
   return c - a * b;
 }
+
+float fp_contract_10(float a, float b, float c) {
+// CHECK: _Z14fp_contract_10fff
+// CHECK: fneg float %a
+// CHECK: tail call float @llvm.fmuladd
+  #pragma STDC FP_CONTRACT ON
+  return -(a * b) + c;
+}
+
+float fp_contract_11(float a, float b, float c) {
+// CHECK: _Z14fp_contract_11fff
+// CHECK: fneg float %a
+// CHECK: fneg float %c
+// CHECK: tail call float @llvm.fmuladd
+  #pragma STDC FP_CONTRACT ON
+  return -(a * b) - c;
+}
+
+float fp_contract_12(float a, float b, float c) {
+// CHECK: _Z14fp_contract_12fff
+// CHECK: fneg float %a
+// CHECK: tail call float @llvm.fmuladd
+  #pragma STDC FP_CONTRACT ON
+  return c + -(a * b);
+}
+
+float fp_contract_13(float a, float b, float c) {
+// CHECK: _Z14fp_contract_13fff
+// CHECK-NOT: fneg float %a
+// CHECK: tail call float @llvm.fmuladd
+  #pragma STDC FP_CONTRACT ON
+  return c - -(a * b);
+}
+
+float fp_contract_14(float a, float b, float c) {
+// CHECK: _Z14fp_contract_14fff
+// CHECK: %[[M:.+]] = fmul float %a, %b
+// CHECK-NEXT: %add = fsub float %c, %[[M]]
+  #pragma STDC FP_CONTRACT ON
+  float d;
+  return (d = -(a * b)) + c;
+}
+
+float fp_contract_15(float a, float b, float c) {
+// CHECK: _Z14fp_contract_15fff
+// CHECK: %[[M:.+]] = fmul float %a, %b
+// CHECK-NEXT: %add = fsub float %c, %[[M]]
+  #pragma STDC FP_CONTRACT ON
+  float d;
+  return -(d = (a * b)) + c;
+}
Index: clang/test/CodeGen/constrained-math-builtins.c
===
--- clang/test/CodeGen/constrained-math-builtins.c
+++ clang/test/CodeGen/constrained-math-builtins.c
@@ -300,10 +300,17 @@
   f * f + f;
   (double)f * f - f;
   (long double)-f * f + f;
+  -(f * f) - f;
+  f + -(f * f);
 
   // CHECK: call float @llvm.experimental.constrained.fmuladd.f32(float %{{.*}}, float %{{.*}}, float %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
   // CHECK: fneg
   // CHECK: call double @llvm.experimental.constrained.fmuladd.f64(double %{{.*}}, double %{{.*}}, double %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
   // CHECK: fneg
   // CHECK: call x86_fp80 @llvm.experimental.constrained.fmuladd.f80(x86_fp80 %{{.*}}, x86_fp80 %{{.*}}, x86_fp80 %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+  // CHECK: fneg
+  // CHECK: fneg
+  // CHECK: call float @llvm.experimental.constrained.fmuladd.f32(float %{{.*}}, float %{{.*}}, float %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
+  // CHECK: fneg
+  // CHECK: call float @llvm.experimental.constrained.fmuladd.f32(float %{{.*}}, float %{{.*}}, float %{{.*}}, metadata !"round.tonearest", metadata !"fpexcept.strict")
 };
Index: clang/lib/CodeGen/CGExprScalar.cpp
===
--- clang/lib/CodeGen/CGExprScalar.cpp
+++ clang/lib/CodeGen/CGExprScalar.cpp
@@ -3734,8 +3734,6 @@
 static Value* buildFMulAdd(llvm::Instruction *MulOp, Value *Addend,
const CodeGenFunction &CGF, CGBuilderTy &Builder,
bool negMul, bool negAdd) {
-  assert(!(negMul && negAdd) && "Only one of negMul and negAdd should be set.");
-
   Value *MulOp0 = MulOp->getOperand(0);
   Value *MulOp1 = MulOp->getOperand(1);
   if (negMul)
@@ -3780,31 +3778,70 @@
   if (!op.FPFeatures.allowFPContractWithinStatement())
 return nullptr;
 
+  Value *LHS = op.LHS;
+  Value *RHS = op.RHS;
+
+  // Peek through fneg to look for fmul. Make sure fneg has no users, and that
+  // it is the only use of its operand.
+  bool NegLHS = false;
+  if (auto *LHSUnOp = dyn_cast(LHS)) {
+if (LHSUnOp->getOpcode() == llvm::Instruction::FNeg &&
+LHSUnOp->use_empty() && LHSUnOp->getOperand(0)->hasOneUse()) {
+  LHS = LHSUnOp->getOperand(0);
+  NegLHS = true;
+}
+  }
+
+  bool NegRHS = false;
+  if (auto *RHSUnOp = dyn_cast(RHS)) {
+if (RHSUnOp->getOpcode() == llvm::Instruction::FNeg &&
+RHSUnOp->use_empty() && RHSUnOp->getOperand(0)->hasOneUse()) {
+  RHS = RHSUnOp->getOperand(0);
+  NegRHS = true;
+}
+  }
+

[PATCH] D144447: [Clang] Teach buildFMulAdd to peek through fneg to find fmul.

[PATCH] D144447: [Clang] Teach buildFMulAdd to peek through fneg to find fmul.

[PATCH] D144447: [Clang] Teach buildFMulAdd to peek through fneg to find fmul.

[PATCH] D144447: [Clang] Teach buildFMulAdd to peek through fneg to find fmul.

[PATCH] D144447: [Clang] Teach buildFMulAdd to peek through fneg to find fmul.

[PATCH] D144447: [Clang] Teach buildFMulAdd to peek through fneg to find fmul.

6 matches

Site Navigation

Mail list logo

Footer information