[llvm] [Matrix] Don't update Changed based on Visit* return value (NFC). (PR #142487)
Jon Roelofs via llvm-commits
llvm-commits at lists.llvm.org
Mon Jun 2 14:04:14 PDT 2025
https://github.com/jroelofs updated https://github.com/llvm/llvm-project/pull/142487
>From afe1fb05426a62915dfa72354878bcb14ad66d94 Mon Sep 17 00:00:00 2001
From: Jon Roelofs <jonathan_roelofs at apple.com>
Date: Mon, 2 Jun 2025 13:59:44 -0700
Subject: [PATCH 1/2] [Matrix] Don't update Changed based on Visit* return
value (NFC).
Visit* are always modifying the IR, remove the boolean result.
Co-authored by: Florian Hahn <florian_hahn at apple.com>
---
.../Scalar/LowerMatrixIntrinsics.cpp | 67 +++++++------------
1 file changed, 26 insertions(+), 41 deletions(-)
diff --git a/llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp b/llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp
index fb5e081acf7c5..124dc54b1dba8 100644
--- a/llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp
+++ b/llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp
@@ -1056,19 +1056,24 @@ class LowerMatrixIntrinsics {
IRBuilder<> Builder(Inst);
+ const ShapeInfo &SI = ShapeMap.at(Inst);
+
if (CallInst *CInst = dyn_cast<CallInst>(Inst))
- Changed |= VisitCallInst(CInst);
+ Changed |= tryVisitCallInst(CInst);
Value *Op1;
Value *Op2;
- if (auto *BinOp = dyn_cast<BinaryOperator>(Inst))
- Changed |= VisitBinaryOperator(BinOp);
- if (auto *UnOp = dyn_cast<UnaryOperator>(Inst))
- Changed |= VisitUnaryOperator(UnOp);
if (match(Inst, m_Load(m_Value(Op1))))
- Changed |= VisitLoad(cast<LoadInst>(Inst), Op1, Builder);
+ VisitLoad(cast<LoadInst>(Inst), SI, 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), SI, Op1, Op2, Builder);
+ else if (auto *BinOp = dyn_cast<BinaryOperator>(Inst))
+ VisitBinaryOperator(BinOp, SI);
+ else if (auto *UnOp = dyn_cast<UnaryOperator>(Inst))
+ VisitUnaryOperator(UnOp, SI);
+ else
+ continue;
+ Changed = true;
}
if (ORE) {
@@ -1107,7 +1112,7 @@ class LowerMatrixIntrinsics {
}
/// Replace intrinsic calls
- bool VisitCallInst(CallInst *Inst) {
+ bool tryVisitCallInst(CallInst *Inst) {
if (!Inst->getCalledFunction() || !Inst->getCalledFunction()->isIntrinsic())
return false;
@@ -2105,49 +2110,36 @@ class LowerMatrixIntrinsics {
}
/// Lower load instructions, if shape information is available.
- bool VisitLoad(LoadInst *Inst, Value *Ptr, IRBuilder<> &Builder) {
- auto I = ShapeMap.find(Inst);
- assert(I != ShapeMap.end() &&
- "must only visit instructions with shape info");
+ void VisitLoad(LoadInst *Inst, const ShapeInfo &SI, Value *Ptr, IRBuilder<> &Builder) {
LowerLoad(Inst, Ptr, Inst->getAlign(),
- Builder.getInt64(I->second.getStride()), Inst->isVolatile(),
- I->second);
- return true;
+ Builder.getInt64(SI.getStride()), Inst->isVolatile(),
+ SI);
}
- bool VisitStore(StoreInst *Inst, Value *StoredVal, Value *Ptr,
+ void VisitStore(StoreInst *Inst, const ShapeInfo &SI, Value *StoredVal, Value *Ptr,
IRBuilder<> &Builder) {
- auto I = ShapeMap.find(StoredVal);
- 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;
+ Builder.getInt64(SI.getStride()), Inst->isVolatile(),
+ SI);
}
/// Lower binary operators, if shape information is available.
- bool VisitBinaryOperator(BinaryOperator *Inst) {
- auto I = ShapeMap.find(Inst);
- assert(I != ShapeMap.end() &&
- "must only visit instructions with shape info");
-
+ void VisitBinaryOperator(BinaryOperator *Inst, const ShapeInfo &SI) {
Value *Lhs = Inst->getOperand(0);
Value *Rhs = Inst->getOperand(1);
IRBuilder<> Builder(Inst);
- ShapeInfo &Shape = I->second;
MatrixTy Result;
- MatrixTy A = getMatrix(Lhs, Shape, Builder);
- MatrixTy B = getMatrix(Rhs, Shape, Builder);
+ MatrixTy A = getMatrix(Lhs, SI, Builder);
+ MatrixTy B = getMatrix(Rhs, SI, Builder);
assert(A.isColumnMajor() == B.isColumnMajor() &&
Result.isColumnMajor() == A.isColumnMajor() &&
"operands must agree on matrix layout");
Builder.setFastMathFlags(getFastMathFlags(Inst));
- for (unsigned I = 0; I < Shape.getNumVectors(); ++I)
+ for (unsigned I = 0; I < SI.getNumVectors(); ++I)
Result.addVector(Builder.CreateBinOp(Inst->getOpcode(), A.getVector(I),
B.getVector(I)));
@@ -2155,22 +2147,16 @@ class LowerMatrixIntrinsics {
Result.addNumComputeOps(getNumOps(Result.getVectorTy()) *
Result.getNumVectors()),
Builder);
- return true;
}
/// Lower unary operators, if shape information is available.
- bool VisitUnaryOperator(UnaryOperator *Inst) {
- auto I = ShapeMap.find(Inst);
- assert(I != ShapeMap.end() &&
- "must only visit instructions with shape info");
-
+ void VisitUnaryOperator(UnaryOperator *Inst, const ShapeInfo &SI) {
Value *Op = Inst->getOperand(0);
IRBuilder<> Builder(Inst);
- ShapeInfo &Shape = I->second;
MatrixTy Result;
- MatrixTy M = getMatrix(Op, Shape, Builder);
+ MatrixTy M = getMatrix(Op, SI, Builder);
Builder.setFastMathFlags(getFastMathFlags(Inst));
@@ -2184,14 +2170,13 @@ class LowerMatrixIntrinsics {
}
};
- for (unsigned I = 0; I < Shape.getNumVectors(); ++I)
+ for (unsigned I = 0; I < SI.getNumVectors(); ++I)
Result.addVector(BuildVectorOp(M.getVector(I)));
finalizeLowering(Inst,
Result.addNumComputeOps(getNumOps(Result.getVectorTy()) *
Result.getNumVectors()),
Builder);
- return true;
}
/// Helper to linearize a matrix expression tree into a string. Currently
>From 9f4e20556c05838b55df10ab2589985834102e35 Mon Sep 17 00:00:00 2001
From: Jon Roelofs <jonathan_roelofs at apple.com>
Date: Mon, 2 Jun 2025 14:03:59 -0700
Subject: [PATCH 2/2] clang-format
---
.../Transforms/Scalar/LowerMatrixIntrinsics.cpp | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp b/llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp
index 124dc54b1dba8..439e616254037 100644
--- a/llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp
+++ b/llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp
@@ -2110,17 +2110,16 @@ class LowerMatrixIntrinsics {
}
/// Lower load instructions, if shape information is available.
- void VisitLoad(LoadInst *Inst, const ShapeInfo &SI, Value *Ptr, IRBuilder<> &Builder) {
- LowerLoad(Inst, Ptr, Inst->getAlign(),
- Builder.getInt64(SI.getStride()), Inst->isVolatile(),
- SI);
+ void VisitLoad(LoadInst *Inst, const ShapeInfo &SI, Value *Ptr,
+ IRBuilder<> &Builder) {
+ LowerLoad(Inst, Ptr, Inst->getAlign(), Builder.getInt64(SI.getStride()),
+ Inst->isVolatile(), SI);
}
- void VisitStore(StoreInst *Inst, const ShapeInfo &SI, Value *StoredVal, Value *Ptr,
- IRBuilder<> &Builder) {
+ void VisitStore(StoreInst *Inst, const ShapeInfo &SI, Value *StoredVal,
+ Value *Ptr, IRBuilder<> &Builder) {
LowerStore(Inst, StoredVal, Ptr, Inst->getAlign(),
- Builder.getInt64(SI.getStride()), Inst->isVolatile(),
- SI);
+ Builder.getInt64(SI.getStride()), Inst->isVolatile(), SI);
}
/// Lower binary operators, if shape information is available.
More information about the llvm-commits
mailing list