[clang] [CIR] Fix assertion with pointer offset in ternary (PR #224724)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clangir

Author: Erich Keane (erichkeane)

<details>
<summary>Changes</summary>

Ternary lowers into some blocks, so the values that come into it end up being block arguments, so there is no defining op for them.  As a result, the 'optimization' of the zero/subtract isn't really possible. THis patch uses a dyn_cast_if_present instead of a dyn_cast to make sure we don't hit that.

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


3 Files Affected:

- (modified) clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp (+2-2) 
- (modified) clang/test/CIR/CodeGen/ternary.cpp (+15) 
- (modified) clang/test/CIR/Lowering/ternary.cir (+18) 


``````````diff
diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
index 59654786d1eca..53b619556f744 100644
--- a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
+++ b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
@@ -1882,8 +1882,8 @@ static mlir::Value convertToIndexTy(mlir::ConversionPatternRewriter &rewriter,
   auto sub = dyn_cast<mlir::LLVM::SubOp>(indexOp);
   bool rewriteSub = false;
   if (sub) {
-    if (auto lhsConst =
-            dyn_cast<mlir::LLVM::ConstantOp>(sub.getLhs().getDefiningOp())) {
+    if (auto lhsConst = dyn_cast_if_present<mlir::LLVM::ConstantOp>(
+            sub.getLhs().getDefiningOp())) {
       auto lhsConstInt = mlir::dyn_cast<mlir::IntegerAttr>(lhsConst.getValue());
       if (lhsConstInt && lhsConstInt.getValue() == 0) {
         index = sub.getRhs();
diff --git a/clang/test/CIR/CodeGen/ternary.cpp b/clang/test/CIR/CodeGen/ternary.cpp
index 061794c7835bd..a7b82a84125de 100644
--- a/clang/test/CIR/CodeGen/ternary.cpp
+++ b/clang/test/CIR/CodeGen/ternary.cpp
@@ -388,3 +388,18 @@ void ternary_void(bool b) {
 // OGCG: [[FALSE]]:
 // OGCG-NEXT: call void @_Z3barv()
 // OGCG-NEXT: br
+
+void ternary_cleanup(int *p, int n, int m) {
+  p+= ((n > 0) ? n : m) - m;
+}
+// CIR-LABEL: cir.func{{.*}}@_Z15ternary_cleanupPiii
+// CIR: %[[TERNARY:.*]] = cir.ternary(%{{.*}}, true {
+// CIR: cir.sub nsw %[[TERNARY]], %{{.*}} : !s32i loc(#loc166)
+
+// LLVM-LABEL: define {{.*}}@_Z15ternary_cleanupPiii
+// LLVM: %[[TERNARY:.*]] = phi i32
+// LLVM: sub nsw i32 %[[TERNARY]], %{{.*}}
+
+// OGCG-LABEL: define {{.*}}@_Z15ternary_cleanupPiii
+// OGCG: %[[TERNARY:.*]] = phi i32
+// OGCG: sub nsw i32 %[[TERNARY]], %{{.*}}
diff --git a/clang/test/CIR/Lowering/ternary.cir b/clang/test/CIR/Lowering/ternary.cir
index 68c98f2d9cc1e..56128613a12ca 100644
--- a/clang/test/CIR/Lowering/ternary.cir
+++ b/clang/test/CIR/Lowering/ternary.cir
@@ -2,6 +2,7 @@
 // RUN: FileCheck --input-file=%t.ll -check-prefix=LLVM %s
 // REQUIRES: target={{x86_64-linux}}
 
+!s32i = !cir.int<s, 32>
 !u32i = !cir.int<u, 32>
 
 module  {
@@ -15,6 +16,23 @@ module  {
     }) : (!cir.bool) -> !u32i
     cir.return %0 : !u32i
   }
+
+  // The stride of a cir.ptr_stride is narrower than the pointer index width, so
+  // it has to be widened to i64. The widening peephole looks through a "sub 0, x"
+  // to find a unary minus, which must not assume the subtraction's LHS has a
+  // defining operation -- here it is the phi materialized for the ternary.
+  cir.func @ternary_result_as_ptr_stride(%arg0: !cir.ptr<!s32i>, %arg1: !cir.bool,
+                                         %arg2: !s32i) -> !cir.ptr<!s32i> {
+    %0 = cir.ternary(%arg1, true {
+      %a = cir.const #cir.int<1> : !s32i
+      cir.yield %a : !s32i
+    }, false {
+      cir.yield %arg2 : !s32i
+    }) : (!cir.bool) -> !s32i
+    %1 = cir.sub %0, %arg2 : !s32i
+    %2 = cir.ptr_stride %arg0, %1 : (!cir.ptr<!s32i>, !s32i) -> !cir.ptr<!s32i>
+    cir.return %2 : !cir.ptr<!s32i>
+  }
 }
 
 // LLVM-LABEL: define i32 {{.*}}@blue(

``````````

</details>


https://github.com/llvm/llvm-project/pull/224724