[flang-commits] [PATCH] D127805: Bitwise comparison intrinsics

[Tarun Prabhu via Phabricator via flang-commits]

tarunprabhu marked 2 inline comments as done.
tarunprabhu added inline comments.


================
Comment at: flang/lib/Evaluate/fold-logical.cpp:63
   } else if (name == "bge" || name == "bgt" || name == "ble" || name == "blt") {
-    static_assert(std::is_same_v<Scalar<LargestInt>, BOZLiteralConstant>);
-    // Arguments do not have to be of the same integer type. Convert all
-    // arguments to the biggest integer type before comparing them to
-    // simplify.
-    for (int i{0}; i <= 1; ++i) {
-      if (auto *x{UnwrapExpr<Expr<SomeInteger>>(args[i])}) {
-        *args[i] = AsGenericExpr(
-            Fold(context, ConvertToType<LargestInt>(std::move(*x))));
-      } else if (auto *x{UnwrapExpr<BOZLiteralConstant>(args[i])}) {
-        *args[i] = AsGenericExpr(Constant<LargestInt>{std::move(*x)});
-      }
-    }
-    auto fptr{&Scalar<LargestInt>::BGE};
-    if (name == "bge") { // done in fptr declaration
-    } else if (name == "bgt") {
-      fptr = &Scalar<LargestInt>::BGT;
-    } else if (name == "ble") {
-      fptr = &Scalar<LargestInt>::BLE;
-    } else if (name == "blt") {
-      fptr = &Scalar<LargestInt>::BLT;
-    } else {
-      common::die("missing case to fold intrinsic function %s", name.c_str());
-    }
-    return FoldElementalIntrinsic<T, LargestInt, LargestInt>(context,
-        std::move(funcRef),
-        ScalarFunc<T, LargestInt, LargestInt>(
-            [&fptr](const Scalar<LargestInt> &i, const Scalar<LargestInt> &j) {
-              return Scalar<T>{std::invoke(fptr, i, j)};
-            }));
+    // The arguments to these intrinsics can be of different types. In that
+    // case, the shorter of the two would need to be zero-extended to match
----------------
klausler wrote:
> What about calls to BGE & al. that must be folded at compilation time in order to process a constant expression in a declaration?
Do they need to be folded before lowering to FIR? The way this is implemented, the constants get folded during lowering from FIR to LLVM-IR.


================
Comment at: flang/lib/Evaluate/fold-logical.cpp:63
   } else if (name == "bge" || name == "bgt" || name == "ble" || name == "blt") {
-    static_assert(std::is_same_v<Scalar<LargestInt>, BOZLiteralConstant>);
-    // Arguments do not have to be of the same integer type. Convert all
-    // arguments to the biggest integer type before comparing them to
-    // simplify.
-    for (int i{0}; i <= 1; ++i) {
-      if (auto *x{UnwrapExpr<Expr<SomeInteger>>(args[i])}) {
-        *args[i] = AsGenericExpr(
-            Fold(context, ConvertToType<LargestInt>(std::move(*x))));
-      } else if (auto *x{UnwrapExpr<BOZLiteralConstant>(args[i])}) {
-        *args[i] = AsGenericExpr(Constant<LargestInt>{std::move(*x)});
-      }
-    }
-    auto fptr{&Scalar<LargestInt>::BGE};
-    if (name == "bge") { // done in fptr declaration
-    } else if (name == "bgt") {
-      fptr = &Scalar<LargestInt>::BGT;
-    } else if (name == "ble") {
-      fptr = &Scalar<LargestInt>::BLE;
-    } else if (name == "blt") {
-      fptr = &Scalar<LargestInt>::BLT;
-    } else {
-      common::die("missing case to fold intrinsic function %s", name.c_str());
-    }
-    return FoldElementalIntrinsic<T, LargestInt, LargestInt>(context,
-        std::move(funcRef),
-        ScalarFunc<T, LargestInt, LargestInt>(
-            [&fptr](const Scalar<LargestInt> &i, const Scalar<LargestInt> &j) {
-              return Scalar<T>{std::invoke(fptr, i, j)};
-            }));
+    // The arguments to these intrinsics can be of different types. In that
+    // case, the shorter of the two would need to be zero-extended to match
----------------
tarunprabhu wrote:
> klausler wrote:
> > What about calls to BGE & al. that must be folded at compilation time in order to process a constant expression in a declaration?
> Do they need to be folded before lowering to FIR? The way this is implemented, the constants get folded during lowering from FIR to LLVM-IR.
Oh, sorry, I misunderstood your comment. I only tested against constant expressions, but not in declarations. The latter, indeed, doesn't work. 


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

https://reviews.llvm.org/D127805