Skip to content

Fix Android build failure in InferIntRangeCommon #96154

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 20, 2024

Conversation

RoboTux
Copy link
Contributor

@RoboTux RoboTux commented Jun 20, 2024

As of today, Android's libcxx is missing C++17's std::function's CTAD
added in e1eabcd. This leads to
InferIntRangeCommon.cpp to fail to compile. This commit makes the
template parameter of std::function in that function explicit, therefore
avoiding CTAD. While LLVM/MLIR's requirement is C++17, the rest of the
code builds fine so hopefully this is acceptable.

As of today, Android's libcxx is missing C++17's std::function's CTAD
added in e1eabcd. This leads to
InferIntRangeCommon.cpp to fail to compile. This commit makes the
template parameter of std::function in that function explicit, therefore
avoiding CTAD. While LLVM/MLIR's requirement is C++17, the rest of the
code builds fine so hopefully this is acceptable.

Change-Id: I78517af440e8b8bcca62f5176e8e3aa437a0d314
@llvmbot llvmbot added the mlir label Jun 20, 2024
@llvmbot
Copy link
Member

llvmbot commented Jun 20, 2024

@llvm/pr-subscribers-mlir

Author: Thomas Preud'homme (RoboTux)

Changes

As of today, Android's libcxx is missing C++17's std::function's CTAD
added in e1eabcd. This leads to
InferIntRangeCommon.cpp to fail to compile. This commit makes the
template parameter of std::function in that function explicit, therefore
avoiding CTAD. While LLVM/MLIR's requirement is C++17, the rest of the
code builds fine so hopefully this is acceptable.


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

1 Files Affected:

  • (modified) mlir/lib/Interfaces/Utils/InferIntRangeCommon.cpp (+18-16)
diff --git a/mlir/lib/Interfaces/Utils/InferIntRangeCommon.cpp b/mlir/lib/Interfaces/Utils/InferIntRangeCommon.cpp
index 5b8d35e7bd519..ca3631d53bda9 100644
--- a/mlir/lib/Interfaces/Utils/InferIntRangeCommon.cpp
+++ b/mlir/lib/Interfaces/Utils/InferIntRangeCommon.cpp
@@ -35,6 +35,8 @@ using namespace mlir;
 /// constants and returns std::nullopt on overflow.
 using ConstArithFn =
     function_ref<std::optional<APInt>(const APInt &, const APInt &)>;
+using ConstArithStdFn =
+    std::function<std::optional<APInt>(const APInt &, const APInt &)>;
 
 /// Compute op(minLeft, minRight) and op(maxLeft, maxRight) if possible,
 /// If either computation overflows, make the result unbounded.
@@ -182,16 +184,16 @@ mlir::intrange::inferAdd(ArrayRef<ConstantIntRanges> argRanges,
                          OverflowFlags ovfFlags) {
   const ConstantIntRanges &lhs = argRanges[0], &rhs = argRanges[1];
 
-  std::function uadd = [=](const APInt &a,
-                           const APInt &b) -> std::optional<APInt> {
+  ConstArithStdFn uadd = [=](const APInt &a,
+                             const APInt &b) -> std::optional<APInt> {
     bool overflowed = false;
     APInt result = any(ovfFlags & OverflowFlags::Nuw)
                        ? a.uadd_sat(b)
                        : a.uadd_ov(b, overflowed);
     return overflowed ? std::optional<APInt>() : result;
   };
-  std::function sadd = [=](const APInt &a,
-                           const APInt &b) -> std::optional<APInt> {
+  ConstArithStdFn sadd = [=](const APInt &a,
+                             const APInt &b) -> std::optional<APInt> {
     bool overflowed = false;
     APInt result = any(ovfFlags & OverflowFlags::Nsw)
                        ? a.sadd_sat(b)
@@ -215,16 +217,16 @@ mlir::intrange::inferSub(ArrayRef<ConstantIntRanges> argRanges,
                          OverflowFlags ovfFlags) {
   const ConstantIntRanges &lhs = argRanges[0], &rhs = argRanges[1];
 
-  std::function usub = [=](const APInt &a,
-                           const APInt &b) -> std::optional<APInt> {
+  ConstArithStdFn usub = [=](const APInt &a,
+                             const APInt &b) -> std::optional<APInt> {
     bool overflowed = false;
     APInt result = any(ovfFlags & OverflowFlags::Nuw)
                        ? a.usub_sat(b)
                        : a.usub_ov(b, overflowed);
     return overflowed ? std::optional<APInt>() : result;
   };
-  std::function ssub = [=](const APInt &a,
-                           const APInt &b) -> std::optional<APInt> {
+  ConstArithStdFn ssub = [=](const APInt &a,
+                             const APInt &b) -> std::optional<APInt> {
     bool overflowed = false;
     APInt result = any(ovfFlags & OverflowFlags::Nsw)
                        ? a.ssub_sat(b)
@@ -247,16 +249,16 @@ mlir::intrange::inferMul(ArrayRef<ConstantIntRanges> argRanges,
                          OverflowFlags ovfFlags) {
   const ConstantIntRanges &lhs = argRanges[0], &rhs = argRanges[1];
 
-  std::function umul = [=](const APInt &a,
-                           const APInt &b) -> std::optional<APInt> {
+  ConstArithStdFn umul = [=](const APInt &a,
+                             const APInt &b) -> std::optional<APInt> {
     bool overflowed = false;
     APInt result = any(ovfFlags & OverflowFlags::Nuw)
                        ? a.umul_sat(b)
                        : a.umul_ov(b, overflowed);
     return overflowed ? std::optional<APInt>() : result;
   };
-  std::function smul = [=](const APInt &a,
-                           const APInt &b) -> std::optional<APInt> {
+  ConstArithStdFn smul = [=](const APInt &a,
+                             const APInt &b) -> std::optional<APInt> {
     bool overflowed = false;
     APInt result = any(ovfFlags & OverflowFlags::Nsw)
                        ? a.smul_sat(b)
@@ -565,16 +567,16 @@ mlir::intrange::inferShl(ArrayRef<ConstantIntRanges> argRanges,
 
   // The signed/unsigned overflow behavior of shl by `rhs` matches a mul with
   // 2^rhs.
-  std::function ushl = [=](const APInt &l,
-                           const APInt &r) -> std::optional<APInt> {
+  ConstArithStdFn ushl = [=](const APInt &l,
+                             const APInt &r) -> std::optional<APInt> {
     bool overflowed = false;
     APInt result = any(ovfFlags & OverflowFlags::Nuw)
                        ? l.ushl_sat(r)
                        : l.ushl_ov(r, overflowed);
     return overflowed ? std::optional<APInt>() : result;
   };
-  std::function sshl = [=](const APInt &l,
-                           const APInt &r) -> std::optional<APInt> {
+  ConstArithStdFn sshl = [=](const APInt &l,
+                             const APInt &r) -> std::optional<APInt> {
     bool overflowed = false;
     APInt result = any(ovfFlags & OverflowFlags::Nsw)
                        ? l.sshl_sat(r)

@RoboTux RoboTux requested review from ubfx and krzysz00 June 20, 2024 09:58
Copy link
Contributor

@krzysz00 krzysz00 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fine, go ahead

@RoboTux RoboTux merged commit 93ce8e1 into main Jun 20, 2024
9 checks passed
@RoboTux RoboTux deleted the users/thopre01/avoid_std_function_ctad branch June 20, 2024 13:44
AlexisPerry pushed a commit to llvm-project-tlp/llvm-project that referenced this pull request Jul 9, 2024
As of today, Android's libcxx is missing C++17's std::function's CTAD
added in e1eabcd. This leads to
InferIntRangeCommon.cpp to fail to compile. This commit makes the
template parameter of std::function in that function explicit, therefore
avoiding CTAD. While LLVM/MLIR's requirement is C++17, the rest of the
code builds fine so hopefully this is acceptable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants