Skip to content

Conversation

dtcxzyw
Copy link
Member

@dtcxzyw dtcxzyw commented Sep 30, 2025

Closes #161367.

In #157614, we ignored cases where OpLatticeVal might be a constant or notconstant. Directly returning the result causes a type mismatch. I apologize for the oversight in the previous code review.

This patch applies the cast op to constants. For notconstant value lattices, I'd leave it as a todo (it is similar to the constant case, except for trunc without nsw/nuw).

@dtcxzyw dtcxzyw requested review from fhahn and andjo403 September 30, 2025 17:39
@dtcxzyw dtcxzyw requested a review from nikic as a code owner September 30, 2025 17:40
@llvmbot llvmbot added llvm:analysis Includes value tracking, cost tables and constant folding llvm:transforms labels Sep 30, 2025
@llvmbot
Copy link
Member

llvmbot commented Sep 30, 2025

@llvm/pr-subscribers-llvm-analysis

Author: Yingwei Zheng (dtcxzyw)

Changes

Closes #161367.

In #157614, we ignored cases where OpLatticeVal might be a constant or notconstant. Directly returning the result causes a type mismatch. I apologize for the oversight in the previous code review.

This patch applies the cast op to constants. For notconstant value lattices, I'd leave it as a todo (it is similar to the constant case, except for trunc without nsw/nuw).


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

2 Files Affected:

  • (modified) llvm/lib/Analysis/LazyValueInfo.cpp (+17-11)
  • (added) llvm/test/Transforms/CorrelatedValuePropagation/pr161367.ll (+31)
diff --git a/llvm/lib/Analysis/LazyValueInfo.cpp b/llvm/lib/Analysis/LazyValueInfo.cpp
index 6fb28072afe46..0e5bc481383a0 100644
--- a/llvm/lib/Analysis/LazyValueInfo.cpp
+++ b/llvm/lib/Analysis/LazyValueInfo.cpp
@@ -1632,19 +1632,25 @@ LazyValueInfoImpl::getEdgeValueLocal(Value *Val, BasicBlock *BBFrom,
                 *getValueFromCondition(Usr->getOperand(0), Condition,
                                        isTrueDest, /*UseBlockValue*/ false);
 
-            if (!OpLatticeVal.isConstantRange())
-              return OpLatticeVal;
+            if (OpLatticeVal.isConstantRange()) {
+              const unsigned ResultBitWidth =
+                  Usr->getType()->getScalarSizeInBits();
+              if (auto *Trunc = dyn_cast<TruncInst>(Usr))
+                return ValueLatticeElement::getRange(
+                    OpLatticeVal.getConstantRange().truncate(
+                        ResultBitWidth, Trunc->getNoWrapKind()));
 
-            const unsigned ResultBitWidth =
-                Usr->getType()->getScalarSizeInBits();
-            if (auto *Trunc = dyn_cast<TruncInst>(Usr))
               return ValueLatticeElement::getRange(
-                  OpLatticeVal.getConstantRange().truncate(
-                      ResultBitWidth, Trunc->getNoWrapKind()));
-
-            return ValueLatticeElement::getRange(
-                OpLatticeVal.getConstantRange().castOp(
-                    cast<CastInst>(Usr)->getOpcode(), ResultBitWidth));
+                  OpLatticeVal.getConstantRange().castOp(
+                      cast<CastInst>(Usr)->getOpcode(), ResultBitWidth));
+            }
+            if (OpLatticeVal.isConstant()) {
+              Constant *C = OpLatticeVal.getConstant();
+              if (auto *CastC = ConstantFoldCastOperand(
+                      cast<CastInst>(Usr)->getOpcode(), C, Usr->getType(), DL))
+                return ValueLatticeElement::get(CastC);
+            }
+            return ValueLatticeElement::getOverdefined();
           } else {
             // If one of Val's operand has an inferred value, we may be able to
             // infer the value of Val.
diff --git a/llvm/test/Transforms/CorrelatedValuePropagation/pr161367.ll b/llvm/test/Transforms/CorrelatedValuePropagation/pr161367.ll
new file mode 100644
index 0000000000000..346eaeaec72c1
--- /dev/null
+++ b/llvm/test/Transforms/CorrelatedValuePropagation/pr161367.ll
@@ -0,0 +1,31 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 6
+; RUN: opt < %s -passes=correlated-propagation -S | FileCheck %s
+
+; Make sure that we apply trunc to the edge value of %x.
+@g = external global i8
+
+define i16 @pr161367(i64 %x) {
+; CHECK-LABEL: define i16 @pr161367(
+; CHECK-SAME: i64 [[X:%.*]]) {
+; CHECK-NEXT:  [[ENTRY:.*]]:
+; CHECK-NEXT:    [[TRUNC:%.*]] = trunc i64 [[X]] to i16
+; CHECK-NEXT:    [[EXITCOND:%.*]] = icmp eq i64 [[X]], sub (i64 0, i64 ptrtoint (ptr @g to i64))
+; CHECK-NEXT:    br i1 [[EXITCOND]], label %[[EXIT:.*]], label %[[ELSE:.*]]
+; CHECK:       [[ELSE]]:
+; CHECK-NEXT:    br label %[[EXIT]]
+; CHECK:       [[EXIT]]:
+; CHECK-NEXT:    [[RET:%.*]] = phi i16 [ trunc (i64 sub (i64 0, i64 ptrtoint (ptr @g to i64)) to i16), %[[ENTRY]] ], [ 0, %[[ELSE]] ]
+; CHECK-NEXT:    ret i16 [[RET]]
+;
+entry:
+  %trunc = trunc i64 %x to i16
+  %exitcond = icmp eq i64 %x, sub (i64 0, i64 ptrtoint (ptr @g to i64))
+  br i1 %exitcond, label %exit, label %else
+
+else:
+  br label %exit
+
+exit:
+  %ret = phi i16 [ %trunc, %entry ], [ 0, %else ]
+  ret i16 %ret
+}

@llvmbot
Copy link
Member

llvmbot commented Sep 30, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Yingwei Zheng (dtcxzyw)

Changes

Closes #161367.

In #157614, we ignored cases where OpLatticeVal might be a constant or notconstant. Directly returning the result causes a type mismatch. I apologize for the oversight in the previous code review.

This patch applies the cast op to constants. For notconstant value lattices, I'd leave it as a todo (it is similar to the constant case, except for trunc without nsw/nuw).


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

2 Files Affected:

  • (modified) llvm/lib/Analysis/LazyValueInfo.cpp (+17-11)
  • (added) llvm/test/Transforms/CorrelatedValuePropagation/pr161367.ll (+31)
diff --git a/llvm/lib/Analysis/LazyValueInfo.cpp b/llvm/lib/Analysis/LazyValueInfo.cpp
index 6fb28072afe46..0e5bc481383a0 100644
--- a/llvm/lib/Analysis/LazyValueInfo.cpp
+++ b/llvm/lib/Analysis/LazyValueInfo.cpp
@@ -1632,19 +1632,25 @@ LazyValueInfoImpl::getEdgeValueLocal(Value *Val, BasicBlock *BBFrom,
                 *getValueFromCondition(Usr->getOperand(0), Condition,
                                        isTrueDest, /*UseBlockValue*/ false);
 
-            if (!OpLatticeVal.isConstantRange())
-              return OpLatticeVal;
+            if (OpLatticeVal.isConstantRange()) {
+              const unsigned ResultBitWidth =
+                  Usr->getType()->getScalarSizeInBits();
+              if (auto *Trunc = dyn_cast<TruncInst>(Usr))
+                return ValueLatticeElement::getRange(
+                    OpLatticeVal.getConstantRange().truncate(
+                        ResultBitWidth, Trunc->getNoWrapKind()));
 
-            const unsigned ResultBitWidth =
-                Usr->getType()->getScalarSizeInBits();
-            if (auto *Trunc = dyn_cast<TruncInst>(Usr))
               return ValueLatticeElement::getRange(
-                  OpLatticeVal.getConstantRange().truncate(
-                      ResultBitWidth, Trunc->getNoWrapKind()));
-
-            return ValueLatticeElement::getRange(
-                OpLatticeVal.getConstantRange().castOp(
-                    cast<CastInst>(Usr)->getOpcode(), ResultBitWidth));
+                  OpLatticeVal.getConstantRange().castOp(
+                      cast<CastInst>(Usr)->getOpcode(), ResultBitWidth));
+            }
+            if (OpLatticeVal.isConstant()) {
+              Constant *C = OpLatticeVal.getConstant();
+              if (auto *CastC = ConstantFoldCastOperand(
+                      cast<CastInst>(Usr)->getOpcode(), C, Usr->getType(), DL))
+                return ValueLatticeElement::get(CastC);
+            }
+            return ValueLatticeElement::getOverdefined();
           } else {
             // If one of Val's operand has an inferred value, we may be able to
             // infer the value of Val.
diff --git a/llvm/test/Transforms/CorrelatedValuePropagation/pr161367.ll b/llvm/test/Transforms/CorrelatedValuePropagation/pr161367.ll
new file mode 100644
index 0000000000000..346eaeaec72c1
--- /dev/null
+++ b/llvm/test/Transforms/CorrelatedValuePropagation/pr161367.ll
@@ -0,0 +1,31 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 6
+; RUN: opt < %s -passes=correlated-propagation -S | FileCheck %s
+
+; Make sure that we apply trunc to the edge value of %x.
+@g = external global i8
+
+define i16 @pr161367(i64 %x) {
+; CHECK-LABEL: define i16 @pr161367(
+; CHECK-SAME: i64 [[X:%.*]]) {
+; CHECK-NEXT:  [[ENTRY:.*]]:
+; CHECK-NEXT:    [[TRUNC:%.*]] = trunc i64 [[X]] to i16
+; CHECK-NEXT:    [[EXITCOND:%.*]] = icmp eq i64 [[X]], sub (i64 0, i64 ptrtoint (ptr @g to i64))
+; CHECK-NEXT:    br i1 [[EXITCOND]], label %[[EXIT:.*]], label %[[ELSE:.*]]
+; CHECK:       [[ELSE]]:
+; CHECK-NEXT:    br label %[[EXIT]]
+; CHECK:       [[EXIT]]:
+; CHECK-NEXT:    [[RET:%.*]] = phi i16 [ trunc (i64 sub (i64 0, i64 ptrtoint (ptr @g to i64)) to i16), %[[ENTRY]] ], [ 0, %[[ELSE]] ]
+; CHECK-NEXT:    ret i16 [[RET]]
+;
+entry:
+  %trunc = trunc i64 %x to i16
+  %exitcond = icmp eq i64 %x, sub (i64 0, i64 ptrtoint (ptr @g to i64))
+  br i1 %exitcond, label %exit, label %else
+
+else:
+  br label %exit
+
+exit:
+  %ret = phi i16 [ %trunc, %entry ], [ 0, %else ]
+  ret i16 %ret
+}

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

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

LGTM

@andjo403
Copy link
Contributor

sorry for that miss and thanks for fixing

For notconstant value lattices, I'd leave it as a todo (it is similar to the constant case, except for trunc without nsw/nuw).

What is the todo? I tought the notconstant was the constantRange case that is already there or is there something more that I have missed?

@nikic
Copy link
Contributor

nikic commented Sep 30, 2025

@andjo403 There is a separate lattice value for != cases that can't be expressed as a range. Mostly used for != null.

@dtcxzyw dtcxzyw merged commit d62776d into llvm:main Oct 1, 2025
12 checks passed
@dtcxzyw dtcxzyw deleted the fix-161367 branch October 1, 2025 06:20
mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Oct 3, 2025
Closes llvm#161367.

In llvm#157614, we ignored cases
where OpLatticeVal might be a constant or notconstant. Directly
returning the result causes a type mismatch. I apologize for the
oversight in the previous code review.

This patch applies the cast op to constants. For notconstant value
lattices, I'd leave it as a todo (it is similar to the constant case,
except for trunc without nsw/nuw).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm:analysis Includes value tracking, cost tables and constant folding llvm:transforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CVP] Assertion `getType() == V->getType() && "All operands to PHI node must be the same type as the PHI node!"' failed.
4 participants