[IRBuilder] Fold binary intrinsics #80743

agentcooper · 2024-02-05T21:27:28Z

github-actions · 2024-02-05T21:27:47Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2024-02-05T21:28:14Z

@llvm/pr-subscribers-backend-amdgpu

@llvm/pr-subscribers-llvm-transforms

Author: Artem Tyurin (agentcooper)

Changes

Draft.

Fixes #61240.

Full diff: https://github.com/llvm/llvm-project/pull/80743.diff

7 Files Affected:

(modified) llvm/include/llvm/Analysis/InstSimplifyFolder.h (+6)
(modified) llvm/include/llvm/Analysis/TargetFolder.h (+5)
(modified) llvm/include/llvm/IR/ConstantFolder.h (+20)
(modified) llvm/include/llvm/IR/IRBuilderFolder.h (+3)
(modified) llvm/include/llvm/IR/NoFolder.h (+4)
(modified) llvm/lib/IR/IRBuilder.cpp (+2)
(modified) llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp (-8)

diff --git a/llvm/include/llvm/Analysis/InstSimplifyFolder.h b/llvm/include/llvm/Analysis/InstSimplifyFolder.h
index 23e2ea80e8cbe..d8ec313b49467 100644
--- a/llvm/include/llvm/Analysis/InstSimplifyFolder.h
+++ b/llvm/include/llvm/Analysis/InstSimplifyFolder.h
@@ -117,6 +117,12 @@ class InstSimplifyFolder final : public IRBuilderFolder {
     return simplifyCastInst(Op, V, DestTy, SQ);
   }
 
+  Value *FoldBinaryIntrinsics(Intrinsic::ID ID, Value *LHS,
+                              Value *RHS) const override {
+    // TODO: should this be defined?
+    return nullptr;
+  }
+
   //===--------------------------------------------------------------------===//
   // Cast/Conversion Operators
   //===--------------------------------------------------------------------===//
diff --git a/llvm/include/llvm/Analysis/TargetFolder.h b/llvm/include/llvm/Analysis/TargetFolder.h
index 978e1002515fc..cabce12541435 100644
--- a/llvm/include/llvm/Analysis/TargetFolder.h
+++ b/llvm/include/llvm/Analysis/TargetFolder.h
@@ -191,6 +191,11 @@ class TargetFolder final : public IRBuilderFolder {
     return nullptr;
   }
 
+  Value *FoldBinaryIntrinsics(Intrinsic::ID ID, Value *LHS, Value *RHS) const override {
+    // TODO: should this be defined?
+    return nullptr;
+  }
+
   //===--------------------------------------------------------------------===//
   // Cast/Conversion Operators
   //===--------------------------------------------------------------------===//
diff --git a/llvm/include/llvm/IR/ConstantFolder.h b/llvm/include/llvm/IR/ConstantFolder.h
index c2b30a65e32e2..001d7b1fdf44e 100644
--- a/llvm/include/llvm/IR/ConstantFolder.h
+++ b/llvm/include/llvm/IR/ConstantFolder.h
@@ -183,6 +183,26 @@ class ConstantFolder final : public IRBuilderFolder {
     return nullptr;
   }
 
+  Value *FoldBinaryIntrinsics(Intrinsic::ID ID, Value *LHS,
+                              Value *RHS) const override {
+    auto *LC = dyn_cast<Constant>(LHS);
+    auto *RC = dyn_cast<Constant>(RHS);
+    if (LC && RC) {
+      if (ID == Intrinsic::maxnum) {
+        return ConstantFP::get(LHS->getType(),
+                               maxnum(cast<ConstantFP>(LHS)->getValueAPF(),
+                                      cast<ConstantFP>(RHS)->getValueAPF()));
+      }
+      if (ID == Intrinsic::minnum) {
+        return ConstantFP::get(LHS->getType(),
+                               minnum(cast<ConstantFP>(LHS)->getValueAPF(),
+                                      cast<ConstantFP>(RHS)->getValueAPF()));
+      }
+      // TODO: use switch, handle more intrinsics
+    }
+    return nullptr;
+  }
+
   //===--------------------------------------------------------------------===//
   // Cast/Conversion Operators
   //===--------------------------------------------------------------------===//
diff --git a/llvm/include/llvm/IR/IRBuilderFolder.h b/llvm/include/llvm/IR/IRBuilderFolder.h
index bd2324dfc5f1b..3e0fd093b3f66 100644
--- a/llvm/include/llvm/IR/IRBuilderFolder.h
+++ b/llvm/include/llvm/IR/IRBuilderFolder.h
@@ -73,6 +73,9 @@ class IRBuilderFolder {
   virtual Value *FoldCast(Instruction::CastOps Op, Value *V,
                           Type *DestTy) const = 0;
 
+  virtual Value *FoldBinaryIntrinsics(Intrinsic::ID ID, Value *LHS,
+                                      Value *RHS) const = 0;
+
   //===--------------------------------------------------------------------===//
   // Cast/Conversion Operators
   //===--------------------------------------------------------------------===//
diff --git a/llvm/include/llvm/IR/NoFolder.h b/llvm/include/llvm/IR/NoFolder.h
index a612f98465aea..787dd10b4cbaa 100644
--- a/llvm/include/llvm/IR/NoFolder.h
+++ b/llvm/include/llvm/IR/NoFolder.h
@@ -112,6 +112,10 @@ class NoFolder final : public IRBuilderFolder {
     return nullptr;
   }
 
+  Value *FoldBinaryIntrinsics(Intrinsic::ID ID, Value *LHS, Value *RHS) const override {
+    return nullptr;
+  }
+
   //===--------------------------------------------------------------------===//
   // Cast/Conversion Operators
   //===--------------------------------------------------------------------===//
diff --git a/llvm/lib/IR/IRBuilder.cpp b/llvm/lib/IR/IRBuilder.cpp
index b09b80f95871a..e5015c1368cea 100644
--- a/llvm/lib/IR/IRBuilder.cpp
+++ b/llvm/lib/IR/IRBuilder.cpp
@@ -922,6 +922,8 @@ CallInst *IRBuilderBase::CreateBinaryIntrinsic(Intrinsic::ID ID, Value *LHS,
                                                Value *RHS,
                                                Instruction *FMFSource,
                                                const Twine &Name) {
+  if (Value *V = Folder.FoldBinaryIntrinsics(ID, LHS, RHS))
+    return (CallInst *) V; // TODO: should return value be changed to Value *?
   Module *M = BB->getModule();
   Function *Fn = Intrinsic::getDeclaration(M, ID, { LHS->getType() });
   return createCallHelper(Fn, {LHS, RHS}, Name, FMFSource);
diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index b8d04322de298..b88f6d060e28e 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -14071,16 +14071,8 @@ class HorizontalReduction {
       return Builder.CreateBinOp((Instruction::BinaryOps)RdxOpcode, LHS, RHS,
                                  Name);
     case RecurKind::FMax:
-      if (IsConstant)
-        return ConstantFP::get(LHS->getType(),
-                               maxnum(cast<ConstantFP>(LHS)->getValueAPF(),
-                                      cast<ConstantFP>(RHS)->getValueAPF()));
       return Builder.CreateBinaryIntrinsic(Intrinsic::maxnum, LHS, RHS);
     case RecurKind::FMin:
-      if (IsConstant)
-        return ConstantFP::get(LHS->getType(),
-                               minnum(cast<ConstantFP>(LHS)->getValueAPF(),
-                                      cast<ConstantFP>(RHS)->getValueAPF()));
       return Builder.CreateBinaryIntrinsic(Intrinsic::minnum, LHS, RHS);
     case RecurKind::FMaximum:
       if (IsConstant)

llvmbot · 2024-02-05T21:28:14Z

@llvm/pr-subscribers-llvm-analysis

Author: Artem Tyurin (agentcooper)

Changes

Draft.

Fixes #61240.

Full diff: https://github.com/llvm/llvm-project/pull/80743.diff

7 Files Affected:

(modified) llvm/include/llvm/Analysis/InstSimplifyFolder.h (+6)
(modified) llvm/include/llvm/Analysis/TargetFolder.h (+5)
(modified) llvm/include/llvm/IR/ConstantFolder.h (+20)
(modified) llvm/include/llvm/IR/IRBuilderFolder.h (+3)
(modified) llvm/include/llvm/IR/NoFolder.h (+4)
(modified) llvm/lib/IR/IRBuilder.cpp (+2)
(modified) llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp (-8)

diff --git a/llvm/include/llvm/Analysis/InstSimplifyFolder.h b/llvm/include/llvm/Analysis/InstSimplifyFolder.h
index 23e2ea80e8cbe..d8ec313b49467 100644
--- a/llvm/include/llvm/Analysis/InstSimplifyFolder.h
+++ b/llvm/include/llvm/Analysis/InstSimplifyFolder.h
@@ -117,6 +117,12 @@ class InstSimplifyFolder final : public IRBuilderFolder {
     return simplifyCastInst(Op, V, DestTy, SQ);
   }
 
+  Value *FoldBinaryIntrinsics(Intrinsic::ID ID, Value *LHS,
+                              Value *RHS) const override {
+    // TODO: should this be defined?
+    return nullptr;
+  }
+
   //===--------------------------------------------------------------------===//
   // Cast/Conversion Operators
   //===--------------------------------------------------------------------===//
diff --git a/llvm/include/llvm/Analysis/TargetFolder.h b/llvm/include/llvm/Analysis/TargetFolder.h
index 978e1002515fc..cabce12541435 100644
--- a/llvm/include/llvm/Analysis/TargetFolder.h
+++ b/llvm/include/llvm/Analysis/TargetFolder.h
@@ -191,6 +191,11 @@ class TargetFolder final : public IRBuilderFolder {
     return nullptr;
   }
 
+  Value *FoldBinaryIntrinsics(Intrinsic::ID ID, Value *LHS, Value *RHS) const override {
+    // TODO: should this be defined?
+    return nullptr;
+  }
+
   //===--------------------------------------------------------------------===//
   // Cast/Conversion Operators
   //===--------------------------------------------------------------------===//
diff --git a/llvm/include/llvm/IR/ConstantFolder.h b/llvm/include/llvm/IR/ConstantFolder.h
index c2b30a65e32e2..001d7b1fdf44e 100644
--- a/llvm/include/llvm/IR/ConstantFolder.h
+++ b/llvm/include/llvm/IR/ConstantFolder.h
@@ -183,6 +183,26 @@ class ConstantFolder final : public IRBuilderFolder {
     return nullptr;
   }
 
+  Value *FoldBinaryIntrinsics(Intrinsic::ID ID, Value *LHS,
+                              Value *RHS) const override {
+    auto *LC = dyn_cast<Constant>(LHS);
+    auto *RC = dyn_cast<Constant>(RHS);
+    if (LC && RC) {
+      if (ID == Intrinsic::maxnum) {
+        return ConstantFP::get(LHS->getType(),
+                               maxnum(cast<ConstantFP>(LHS)->getValueAPF(),
+                                      cast<ConstantFP>(RHS)->getValueAPF()));
+      }
+      if (ID == Intrinsic::minnum) {
+        return ConstantFP::get(LHS->getType(),
+                               minnum(cast<ConstantFP>(LHS)->getValueAPF(),
+                                      cast<ConstantFP>(RHS)->getValueAPF()));
+      }
+      // TODO: use switch, handle more intrinsics
+    }
+    return nullptr;
+  }
+
   //===--------------------------------------------------------------------===//
   // Cast/Conversion Operators
   //===--------------------------------------------------------------------===//
diff --git a/llvm/include/llvm/IR/IRBuilderFolder.h b/llvm/include/llvm/IR/IRBuilderFolder.h
index bd2324dfc5f1b..3e0fd093b3f66 100644
--- a/llvm/include/llvm/IR/IRBuilderFolder.h
+++ b/llvm/include/llvm/IR/IRBuilderFolder.h
@@ -73,6 +73,9 @@ class IRBuilderFolder {
   virtual Value *FoldCast(Instruction::CastOps Op, Value *V,
                           Type *DestTy) const = 0;
 
+  virtual Value *FoldBinaryIntrinsics(Intrinsic::ID ID, Value *LHS,
+                                      Value *RHS) const = 0;
+
   //===--------------------------------------------------------------------===//
   // Cast/Conversion Operators
   //===--------------------------------------------------------------------===//
diff --git a/llvm/include/llvm/IR/NoFolder.h b/llvm/include/llvm/IR/NoFolder.h
index a612f98465aea..787dd10b4cbaa 100644
--- a/llvm/include/llvm/IR/NoFolder.h
+++ b/llvm/include/llvm/IR/NoFolder.h
@@ -112,6 +112,10 @@ class NoFolder final : public IRBuilderFolder {
     return nullptr;
   }
 
+  Value *FoldBinaryIntrinsics(Intrinsic::ID ID, Value *LHS, Value *RHS) const override {
+    return nullptr;
+  }
+
   //===--------------------------------------------------------------------===//
   // Cast/Conversion Operators
   //===--------------------------------------------------------------------===//
diff --git a/llvm/lib/IR/IRBuilder.cpp b/llvm/lib/IR/IRBuilder.cpp
index b09b80f95871a..e5015c1368cea 100644
--- a/llvm/lib/IR/IRBuilder.cpp
+++ b/llvm/lib/IR/IRBuilder.cpp
@@ -922,6 +922,8 @@ CallInst *IRBuilderBase::CreateBinaryIntrinsic(Intrinsic::ID ID, Value *LHS,
                                                Value *RHS,
                                                Instruction *FMFSource,
                                                const Twine &Name) {
+  if (Value *V = Folder.FoldBinaryIntrinsics(ID, LHS, RHS))
+    return (CallInst *) V; // TODO: should return value be changed to Value *?
   Module *M = BB->getModule();
   Function *Fn = Intrinsic::getDeclaration(M, ID, { LHS->getType() });
   return createCallHelper(Fn, {LHS, RHS}, Name, FMFSource);
diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index b8d04322de298..b88f6d060e28e 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -14071,16 +14071,8 @@ class HorizontalReduction {
       return Builder.CreateBinOp((Instruction::BinaryOps)RdxOpcode, LHS, RHS,
                                  Name);
     case RecurKind::FMax:
-      if (IsConstant)
-        return ConstantFP::get(LHS->getType(),
-                               maxnum(cast<ConstantFP>(LHS)->getValueAPF(),
-                                      cast<ConstantFP>(RHS)->getValueAPF()));
       return Builder.CreateBinaryIntrinsic(Intrinsic::maxnum, LHS, RHS);
     case RecurKind::FMin:
-      if (IsConstant)
-        return ConstantFP::get(LHS->getType(),
-                               minnum(cast<ConstantFP>(LHS)->getValueAPF(),
-                                      cast<ConstantFP>(RHS)->getValueAPF()));
       return Builder.CreateBinaryIntrinsic(Intrinsic::minnum, LHS, RHS);
     case RecurKind::FMaximum:
       if (IsConstant)

llvmbot · 2024-02-05T21:28:14Z

@llvm/pr-subscribers-llvm-ir

Author: Artem Tyurin (agentcooper)

Changes

Draft.

Fixes #61240.

Full diff: https://github.com/llvm/llvm-project/pull/80743.diff

7 Files Affected:

(modified) llvm/include/llvm/Analysis/InstSimplifyFolder.h (+6)
(modified) llvm/include/llvm/Analysis/TargetFolder.h (+5)
(modified) llvm/include/llvm/IR/ConstantFolder.h (+20)
(modified) llvm/include/llvm/IR/IRBuilderFolder.h (+3)
(modified) llvm/include/llvm/IR/NoFolder.h (+4)
(modified) llvm/lib/IR/IRBuilder.cpp (+2)
(modified) llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp (-8)

diff --git a/llvm/include/llvm/Analysis/InstSimplifyFolder.h b/llvm/include/llvm/Analysis/InstSimplifyFolder.h
index 23e2ea80e8cbe..d8ec313b49467 100644
--- a/llvm/include/llvm/Analysis/InstSimplifyFolder.h
+++ b/llvm/include/llvm/Analysis/InstSimplifyFolder.h
@@ -117,6 +117,12 @@ class InstSimplifyFolder final : public IRBuilderFolder {
     return simplifyCastInst(Op, V, DestTy, SQ);
   }
 
+  Value *FoldBinaryIntrinsics(Intrinsic::ID ID, Value *LHS,
+                              Value *RHS) const override {
+    // TODO: should this be defined?
+    return nullptr;
+  }
+
   //===--------------------------------------------------------------------===//
   // Cast/Conversion Operators
   //===--------------------------------------------------------------------===//
diff --git a/llvm/include/llvm/Analysis/TargetFolder.h b/llvm/include/llvm/Analysis/TargetFolder.h
index 978e1002515fc..cabce12541435 100644
--- a/llvm/include/llvm/Analysis/TargetFolder.h
+++ b/llvm/include/llvm/Analysis/TargetFolder.h
@@ -191,6 +191,11 @@ class TargetFolder final : public IRBuilderFolder {
     return nullptr;
   }
 
+  Value *FoldBinaryIntrinsics(Intrinsic::ID ID, Value *LHS, Value *RHS) const override {
+    // TODO: should this be defined?
+    return nullptr;
+  }
+
   //===--------------------------------------------------------------------===//
   // Cast/Conversion Operators
   //===--------------------------------------------------------------------===//
diff --git a/llvm/include/llvm/IR/ConstantFolder.h b/llvm/include/llvm/IR/ConstantFolder.h
index c2b30a65e32e2..001d7b1fdf44e 100644
--- a/llvm/include/llvm/IR/ConstantFolder.h
+++ b/llvm/include/llvm/IR/ConstantFolder.h
@@ -183,6 +183,26 @@ class ConstantFolder final : public IRBuilderFolder {
     return nullptr;
   }
 
+  Value *FoldBinaryIntrinsics(Intrinsic::ID ID, Value *LHS,
+                              Value *RHS) const override {
+    auto *LC = dyn_cast<Constant>(LHS);
+    auto *RC = dyn_cast<Constant>(RHS);
+    if (LC && RC) {
+      if (ID == Intrinsic::maxnum) {
+        return ConstantFP::get(LHS->getType(),
+                               maxnum(cast<ConstantFP>(LHS)->getValueAPF(),
+                                      cast<ConstantFP>(RHS)->getValueAPF()));
+      }
+      if (ID == Intrinsic::minnum) {
+        return ConstantFP::get(LHS->getType(),
+                               minnum(cast<ConstantFP>(LHS)->getValueAPF(),
+                                      cast<ConstantFP>(RHS)->getValueAPF()));
+      }
+      // TODO: use switch, handle more intrinsics
+    }
+    return nullptr;
+  }
+
   //===--------------------------------------------------------------------===//
   // Cast/Conversion Operators
   //===--------------------------------------------------------------------===//
diff --git a/llvm/include/llvm/IR/IRBuilderFolder.h b/llvm/include/llvm/IR/IRBuilderFolder.h
index bd2324dfc5f1b..3e0fd093b3f66 100644
--- a/llvm/include/llvm/IR/IRBuilderFolder.h
+++ b/llvm/include/llvm/IR/IRBuilderFolder.h
@@ -73,6 +73,9 @@ class IRBuilderFolder {
   virtual Value *FoldCast(Instruction::CastOps Op, Value *V,
                           Type *DestTy) const = 0;
 
+  virtual Value *FoldBinaryIntrinsics(Intrinsic::ID ID, Value *LHS,
+                                      Value *RHS) const = 0;
+
   //===--------------------------------------------------------------------===//
   // Cast/Conversion Operators
   //===--------------------------------------------------------------------===//
diff --git a/llvm/include/llvm/IR/NoFolder.h b/llvm/include/llvm/IR/NoFolder.h
index a612f98465aea..787dd10b4cbaa 100644
--- a/llvm/include/llvm/IR/NoFolder.h
+++ b/llvm/include/llvm/IR/NoFolder.h
@@ -112,6 +112,10 @@ class NoFolder final : public IRBuilderFolder {
     return nullptr;
   }
 
+  Value *FoldBinaryIntrinsics(Intrinsic::ID ID, Value *LHS, Value *RHS) const override {
+    return nullptr;
+  }
+
   //===--------------------------------------------------------------------===//
   // Cast/Conversion Operators
   //===--------------------------------------------------------------------===//
diff --git a/llvm/lib/IR/IRBuilder.cpp b/llvm/lib/IR/IRBuilder.cpp
index b09b80f95871a..e5015c1368cea 100644
--- a/llvm/lib/IR/IRBuilder.cpp
+++ b/llvm/lib/IR/IRBuilder.cpp
@@ -922,6 +922,8 @@ CallInst *IRBuilderBase::CreateBinaryIntrinsic(Intrinsic::ID ID, Value *LHS,
                                                Value *RHS,
                                                Instruction *FMFSource,
                                                const Twine &Name) {
+  if (Value *V = Folder.FoldBinaryIntrinsics(ID, LHS, RHS))
+    return (CallInst *) V; // TODO: should return value be changed to Value *?
   Module *M = BB->getModule();
   Function *Fn = Intrinsic::getDeclaration(M, ID, { LHS->getType() });
   return createCallHelper(Fn, {LHS, RHS}, Name, FMFSource);
diff --git a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
index b8d04322de298..b88f6d060e28e 100644
--- a/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
+++ b/llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
@@ -14071,16 +14071,8 @@ class HorizontalReduction {
       return Builder.CreateBinOp((Instruction::BinaryOps)RdxOpcode, LHS, RHS,
                                  Name);
     case RecurKind::FMax:
-      if (IsConstant)
-        return ConstantFP::get(LHS->getType(),
-                               maxnum(cast<ConstantFP>(LHS)->getValueAPF(),
-                                      cast<ConstantFP>(RHS)->getValueAPF()));
       return Builder.CreateBinaryIntrinsic(Intrinsic::maxnum, LHS, RHS);
     case RecurKind::FMin:
-      if (IsConstant)
-        return ConstantFP::get(LHS->getType(),
-                               minnum(cast<ConstantFP>(LHS)->getValueAPF(),
-                                      cast<ConstantFP>(RHS)->getValueAPF()));
       return Builder.CreateBinaryIntrinsic(Intrinsic::minnum, LHS, RHS);
     case RecurKind::FMaximum:
       if (IsConstant)

github-actions · 2024-02-05T21:29:46Z

✅ With the latest revision this PR passed the C/C++ code formatter.

nikic

The high level approach here should be to not hard-code constant folding for specific intrinsics, but rather make use of generic APIs like ConstantFoldCall or simplifyCall.

This is not entirely straightforward because these APIs currently require an already existing call, so some refactoring would be necessary to allow calling them with e.g. just a function, function type and arguments or something like that.

agentcooper · 2024-02-06T17:00:42Z

@nikic Thanks for the guidance!

I've just pushed a commit where I am able to reuse simplifyBinaryIntrinsic in case of the InstSimplifyFolder. I am still not entirely sure what should I do in the ConstantFolder – the ConstantFoldCall you've mentioned is a part of Analysis and the ConstantFolder is in the IR.

nikic · 2024-02-06T19:38:23Z

It's not possible to use it in ConstantFolder, but you can use it in TargetFolder. We could try to move parts of it into IR, but I don't think that's worthwhile (it would be easier to switch SLP to use TargetFolder).

agentcooper · 2024-02-11T12:00:35Z

I've implemented FoldBinaryIntrinsic in all 3 folders: ConstantFolder, TargetFolder, InstSimplifyFolder.

In case of the TargetFolder I split the ConstantFoldScalarCall2 into 2 functions: ConstantFoldLibCall2 that handles LibFunc and ConstantFoldIntrinsicCall2 that handles intrinsics and is reused.

I've also switched the return type for CreateBinaryIntrinsic from CallInst to Value to reflect the fact that it can be folded.

In case of ConstantFolder I've created a new ConstantFoldBinaryIntrinsicInstruction which now has the code that was previously in SLPVectorizer.

Not sure why Windows build is failing, but Linux one is passing.

@nikic Could you please take another look?

agentcooper · 2024-02-21T12:18:01Z

@nikic ping :-)

llvm/include/llvm/Analysis/ConstantFolding.h

llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp

llvm/lib/IR/ConstantFold.cpp

llvm/include/llvm/Analysis/InstSimplifyFolder.h

llvm/lib/IR/IRBuilder.cpp

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

llvm/include/llvm/Analysis/InstSimplifyFolder.h

llvm/include/llvm/Analysis/TargetFolder.h

dtcxzyw · 2024-02-22T15:17:14Z

llvm/include/llvm/Analysis/TargetFolder.h

    auto *C1 = dyn_cast<Constant>(LHS);
    auto *C2 = dyn_cast<Constant>(RHS);
    if (C1 && C2)
-      return ConstantFoldBinaryIntrinsic(ID, C1, C2, Ty);
+      return ConstantFoldBinaryIntrinsic(ID, C1, C2, Ty, FMFSource);


It doesn't help constant folding :(

Oh, my bad. It is used to fold constrained fp intrinsics.

llvm/include/llvm/IR/ConstantFolder.h

llvm/include/llvm/IR/NoFolder.h

dtcxzyw

LGTM. Thanks!
Please wait for additional approval from other reviewers.

dtcxzyw · 2024-02-22T15:55:55Z

llvm/lib/Analysis/ConstantFolding.cpp

+      if (Call) {
+        if (const auto *ConstrIntr = dyn_cast<ConstrainedFPIntrinsic>(Call)) {


Suggested change

if (Call) {

if (const auto *ConstrIntr = dyn_cast<ConstrainedFPIntrinsic>(Call)) {

if (const auto *ConstrIntr = dyn_cast_if_present<ConstrainedFPIntrinsic>(Call)) {

dtcxzyw · 2024-02-22T15:57:09Z

llvm/lib/Analysis/ConstantFolding.cpp

+    if (auto *FoldedLibCall = ConstantFoldLibCall2(Name, Ty, Operands, TLI)) {
+      return FoldedLibCall;
+    }


Suggested change

if (auto *FoldedLibCall = ConstantFoldLibCall2(Name, Ty, Operands, TLI)) {

return FoldedLibCall;

}

if (Constant *FoldedLibCall = ConstantFoldLibCall2(Name, Ty, Operands, TLI))

return FoldedLibCall;

llvm/lib/IR/ConstantFold.cpp

dtcxzyw · 2024-02-22T15:58:50Z

llvm/lib/IR/ConstantFold.cpp

+  auto *RC = dyn_cast<ConstantFP>(RHS);
+  if (!RC)
+    return nullptr;
+  auto LVal = LC->getValueAPF();


Suggested change

auto LVal = LC->getValueAPF();

const APFloat &LVal = LC->getValueAPF();

dtcxzyw · 2024-02-22T15:59:11Z

llvm/lib/IR/IRBuilder.cpp

  Module *M = BB->getModule();
  Function *Fn = Intrinsic::getDeclaration(M, ID, { LHS->getType() });
+  if (auto *V = Folder.FoldBinaryIntrinsic(ID, LHS, RHS, Fn->getReturnType(),


Suggested change

if (auto *V = Folder.FoldBinaryIntrinsic(ID, LHS, RHS, Fn->getReturnType(),

if (Value *V = Folder.FoldBinaryIntrinsic(ID, LHS, RHS, Fn->getReturnType(),

llvm/lib/Target/AMDGPU/AMDGPUInstCombineIntrinsic.cpp

nikic

Please remove the changes to IR/ConstantFold. I'm not willing to duplicate constant folding logic in there. If we really wanted to we could move logic there, but we should not copy it.

agentcooper · 2024-02-24T13:20:18Z

@nikic So the suggestion is to keep the SLPVectorizer code as it is right now and maybe create a separate PR to move SLPVectorizer to TargetFolder instead? Wouldn't you consider it an improvement to extract the existing folding code from SLPVectorizer into IR/ConstantFolder?

RKSimon · 2024-03-05T13:09:10Z

@agentcooper reverse-ping?

nikic · 2024-03-05T20:44:30Z

@nikic So the suggestion is to keep the SLPVectorizer code as it is right now and maybe create a separate PR to move SLPVectorizer to TargetFolder instead?

Yes, exactly! (Or move SLPVectorizer to TargetFolder first.)

Wouldn't you consider it an improvement to extract the existing folding code from SLPVectorizer into IR/ConstantFolder?

No, because it sets a bad precedent. The ConstantFolding / ConstantFold split is already annoying enough as it is, without introducing actual duplication between them.

agentcooper · 2024-03-07T20:46:45Z

@nikic Got it. I updated the PR to exclude changes to IR/ConstantFold and SLPVectorizer (FYI @RKSimon).

agentcooper · 2024-03-14T15:30:24Z

@nikic ping :-)

nikic

LGTM

agentcooper · 2024-03-14T18:07:23Z

@nikic @dtcxzyw If this has enough reviews, could you please merge it for me? I don't have commit access.

github-actions · 2024-03-15T08:58:44Z

@agentcooper Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested
by our build bots. If there is a problem with a build, you may recieve a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as
the builds can include changes from many authors. It is not uncommon for your
change to be included in a build that fails due to someone else's changes, or
infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself.
This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!

[IRBuilder] Fold binary intrinsics

84a1fd5

Fixes llvm#61240.

llvmbot added vectorization llvm:ir llvm:analysis llvm:transforms labels Feb 5, 2024

agentcooper mentioned this pull request Feb 5, 2024

[IR] IRBuilderBase::CreateBinaryIntrinsic - attempt constant folding #61240

Closed

nikic self-requested a review February 5, 2024 22:05

nikic reviewed Feb 6, 2024

View reviewed changes

Reuse simplifyBinaryIntrinsic

4044d93

agentcooper added 2 commits February 11, 2024 01:07

Merge branch 'llvm:main' into fold-binary-intrinsics

7533c55

Change return type of CreateBinaryIntrinsic from CallInst to Value

f3c7393

llvmbot added the backend:AMDGPU label Feb 11, 2024

Code formatting

a6423b6

agentcooper requested a review from nikic February 12, 2024 11:32

RKSimon requested review from RKSimon, alexey-bataev and dtcxzyw February 21, 2024 16:46

dtcxzyw requested changes Feb 22, 2024

View reviewed changes

Add type parameter to FoldBinaryIntrinsic

4050290

RKSimon reviewed Feb 22, 2024

View reviewed changes

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp Outdated Show resolved Hide resolved

RKSimon reviewed Feb 22, 2024

View reviewed changes

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp Show resolved Hide resolved

agentcooper added 3 commits February 22, 2024 14:22

Explicit type annotation

bb10cd8

Merge intrinsic cases in SLPVectorizer

dba29b3

Pass FMFSource

e2bc5d3

dtcxzyw requested changes Feb 22, 2024

View reviewed changes

Remove default argument in overridden virtual functions

edb82a9

dtcxzyw approved these changes Feb 22, 2024

View reviewed changes

nikic requested changes Feb 22, 2024

View reviewed changes

agentcooper added 2 commits March 7, 2024 20:55

Remove changes to IR/ConstantFold and SLPVectorizer

60464bc

Feedback

8ede264

agentcooper requested a review from nikic March 7, 2024 20:46

nikic approved these changes Mar 14, 2024

View reviewed changes

nikic merged commit 1411452 into llvm:main Mar 15, 2024
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[IRBuilder] Fold binary intrinsics #80743

[IRBuilder] Fold binary intrinsics #80743

agentcooper commented Feb 5, 2024 •

edited

github-actions bot commented Feb 5, 2024

llvmbot commented Feb 5, 2024 •

edited

llvmbot commented Feb 5, 2024

llvmbot commented Feb 5, 2024

github-actions bot commented Feb 5, 2024 •

edited

nikic left a comment

agentcooper commented Feb 6, 2024

nikic commented Feb 6, 2024

agentcooper commented Feb 11, 2024

agentcooper commented Feb 21, 2024

dtcxzyw Feb 22, 2024

dtcxzyw Feb 22, 2024

dtcxzyw left a comment

dtcxzyw Feb 22, 2024

dtcxzyw Feb 22, 2024

dtcxzyw Feb 22, 2024

dtcxzyw Feb 22, 2024

nikic left a comment

agentcooper commented Feb 24, 2024

RKSimon commented Mar 5, 2024

nikic commented Mar 5, 2024

agentcooper commented Mar 7, 2024

agentcooper commented Mar 14, 2024

nikic left a comment

agentcooper commented Mar 14, 2024

github-actions bot commented Mar 15, 2024

		if (Call) {
		if (const auto *ConstrIntr = dyn_cast<ConstrainedFPIntrinsic>(Call)) {

	if (Call) {
	if (const auto *ConstrIntr = dyn_cast<ConstrainedFPIntrinsic>(Call)) {
	if (const auto *ConstrIntr = dyn_cast_if_present<ConstrainedFPIntrinsic>(Call)) {

	auto LVal = LC->getValueAPF();
	const APFloat &LVal = LC->getValueAPF();

	if (auto *V = Folder.FoldBinaryIntrinsic(ID, LHS, RHS, Fn->getReturnType(),
	if (Value *V = Folder.FoldBinaryIntrinsic(ID, LHS, RHS, Fn->getReturnType(),

[IRBuilder] Fold binary intrinsics #80743

[IRBuilder] Fold binary intrinsics #80743

Conversation

agentcooper commented Feb 5, 2024 • edited

github-actions bot commented Feb 5, 2024

llvmbot commented Feb 5, 2024 • edited

llvmbot commented Feb 5, 2024

llvmbot commented Feb 5, 2024

github-actions bot commented Feb 5, 2024 • edited

nikic left a comment

Choose a reason for hiding this comment

agentcooper commented Feb 6, 2024

nikic commented Feb 6, 2024

agentcooper commented Feb 11, 2024

agentcooper commented Feb 21, 2024

dtcxzyw Feb 22, 2024

Choose a reason for hiding this comment

dtcxzyw Feb 22, 2024

Choose a reason for hiding this comment

dtcxzyw left a comment

Choose a reason for hiding this comment

dtcxzyw Feb 22, 2024

Choose a reason for hiding this comment

dtcxzyw Feb 22, 2024

Choose a reason for hiding this comment

dtcxzyw Feb 22, 2024

Choose a reason for hiding this comment

dtcxzyw Feb 22, 2024

Choose a reason for hiding this comment

nikic left a comment

Choose a reason for hiding this comment

agentcooper commented Feb 24, 2024

RKSimon commented Mar 5, 2024

nikic commented Mar 5, 2024

agentcooper commented Mar 7, 2024

agentcooper commented Mar 14, 2024

nikic left a comment

Choose a reason for hiding this comment

agentcooper commented Mar 14, 2024

github-actions bot commented Mar 15, 2024

agentcooper commented Feb 5, 2024 •

edited

llvmbot commented Feb 5, 2024 •

edited

github-actions bot commented Feb 5, 2024 •

edited