Skip to content

[Matrix] Don't update Changed based on Visit* return value (NFC). #142417

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 2 commits into from
Jun 5, 2025

Conversation

fhahn
Copy link
Contributor

@fhahn fhahn commented Jun 2, 2025

Visit* are always modifying the IR, remove the boolean result.

Depends on #142416.

@fhahn fhahn requested review from jroelofs and anemet June 2, 2025 15:45
@llvmbot
Copy link
Member

llvmbot commented Jun 2, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Florian Hahn (fhahn)

Changes

Visit* are always modifying the IR, remove the boolean result.

Depends on #142416.


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

1 Files Affected:

  • (modified) llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp (+19-22)
diff --git a/llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp b/llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp
index 787e107464c0a..e6e8c30b9260a 100644
--- a/llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp
+++ b/llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp
@@ -1062,13 +1062,16 @@ class LowerMatrixIntrinsics {
       Value *Op1;
       Value *Op2;
       if (auto *BinOp = dyn_cast<BinaryOperator>(Inst))
-        Changed |= VisitBinaryOperator(BinOp);
+        VisitBinaryOperator(BinOp);
       if (auto *UnOp = dyn_cast<UnaryOperator>(Inst))
-        Changed |= VisitUnaryOperator(UnOp);
+        VisitUnaryOperator(UnOp);
       if (match(Inst, m_Load(m_Value(Op1))))
-        Changed |= VisitLoad(cast<LoadInst>(Inst), Op1, Builder);
+        VisitLoad(cast<LoadInst>(Inst), Op1, Builder);
       else if (match(Inst, m_Store(m_Value(Op1), m_Value(Op2))))
-        Changed |= VisitStore(cast<StoreInst>(Inst), Op1, Op2, Builder);
+        VisitStore(cast<StoreInst>(Inst), Op1, Op2, Builder);
+      else
+        continue;
+      Changed = true;
     }
 
     if (ORE) {
@@ -2105,34 +2108,30 @@ class LowerMatrixIntrinsics {
   }
 
   /// Lower load instructions, if shape information is available.
-  bool VisitLoad(LoadInst *Inst, Value *Ptr, IRBuilder<> &Builder) {
+  void VisitLoad(LoadInst *Inst, Value *Ptr, IRBuilder<> &Builder) {
     auto I = ShapeMap.find(Inst);
-    if (I == ShapeMap.end())
-      return false;
-
+    assert(I != ShapeMap.end() &&
+           "must only visit instructions with shape info");
     LowerLoad(Inst, Ptr, Inst->getAlign(),
               Builder.getInt64(I->second.getStride()), Inst->isVolatile(),
               I->second);
-    return true;
   }
 
-  bool VisitStore(StoreInst *Inst, Value *StoredVal, Value *Ptr,
+  void VisitStore(StoreInst *Inst, Value *StoredVal, Value *Ptr,
                   IRBuilder<> &Builder) {
     auto I = ShapeMap.find(StoredVal);
-    if (I == ShapeMap.end())
-      return false;
-
+    assert(I != ShapeMap.end() &&
+           "must only visit instructions with shape info");
     LowerStore(Inst, StoredVal, Ptr, Inst->getAlign(),
                Builder.getInt64(I->second.getStride()), Inst->isVolatile(),
                I->second);
-    return true;
   }
 
   /// Lower binary operators, if shape information is available.
-  bool VisitBinaryOperator(BinaryOperator *Inst) {
+  void VisitBinaryOperator(BinaryOperator *Inst) {
     auto I = ShapeMap.find(Inst);
-    if (I == ShapeMap.end())
-      return false;
+    assert(I != ShapeMap.end() &&
+           "must only visit instructions with shape info");
 
     Value *Lhs = Inst->getOperand(0);
     Value *Rhs = Inst->getOperand(1);
@@ -2157,14 +2156,13 @@ class LowerMatrixIntrinsics {
                      Result.addNumComputeOps(getNumOps(Result.getVectorTy()) *
                                              Result.getNumVectors()),
                      Builder);
-    return true;
   }
 
   /// Lower unary operators, if shape information is available.
-  bool VisitUnaryOperator(UnaryOperator *Inst) {
+  void VisitUnaryOperator(UnaryOperator *Inst) {
     auto I = ShapeMap.find(Inst);
-    if (I == ShapeMap.end())
-      return false;
+    assert(I != ShapeMap.end() &&
+           "must only visit instructions with shape info");
 
     Value *Op = Inst->getOperand(0);
 
@@ -2193,7 +2191,6 @@ class LowerMatrixIntrinsics {
                      Result.addNumComputeOps(getNumOps(Result.getVectorTy()) *
                                              Result.getNumVectors()),
                      Builder);
-    return true;
   }
 
   /// Helper to linearize a matrix expression tree into a string. Currently

fhahn added 2 commits June 5, 2025 16:24
Visit* are always modifying the IR, remove the boolean result.

Depends on llvm#142416.
@fhahn fhahn force-pushed the matrix-remove-return-value branch from 5569e9b to 42c110b Compare June 5, 2025 15:44
@fhahn fhahn merged commit eb83c43 into llvm:main Jun 5, 2025
9 of 10 checks passed
@fhahn fhahn deleted the matrix-remove-return-value branch June 5, 2025 16:54
llvm-sync bot pushed a commit to arm/arm-toolchain that referenced this pull request Jun 5, 2025
… (NFC). (#142417)

Visit* are always modifying the IR, remove the boolean result.

Depends on llvm/llvm-project#142416.

PR: llvm/llvm-project#142417
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants