address code review comments

Author: chenzheng1030

clang/lib/CodeGen/CGExprScalar.cpp

@@ -766,20 +766,20 @@ class ScalarExprEmitter
       CodeGenFunction::CGFPOptionsRAII FPOptsRAII(CGF, Ops.FPFeatures);
       if (LHSMatTy && RHSMatTy) {
         // Note that SME only has non-widening MOPA for float32 and float64, so
-	// only these two types have native SME matmul operations. For other
-	// types, SVE version is used. We hope that SVE version is better than
-	// default NEON or scalar version.
-	auto Ty = LHSMatTy->getElementType();
-	if (!CGF.getContext().getTargetInfo().hasFeature("sme") ||
-	    !MatrixType::isValidTypeForSME(Ty))
+        // only these two types have native SME matmul operations. For other
+        // types, SVE version is used. We hope that SVE version is better than
+        // default NEON or scalar version.
+        auto Ty = LHSMatTy->getElementType();
+        if (!CGF.getContext().getTargetInfo().hasFeature("sme") ||
+            !MatrixType::isValidTypeForSME(Ty))
           return MB.CreateMatrixMultiply(
-	      Ops.LHS, Ops.RHS, LHSMatTy->getNumRows(),
-	      LHSMatTy->getNumColumns(), RHSMatTy->getNumColumns());
+              Ops.LHS, Ops.RHS, LHSMatTy->getNumRows(),
+              LHSMatTy->getNumColumns(), RHSMatTy->getNumColumns());
         assert(isa<BuiltinType>(Ty) && "SME types should be BuiltinType.");
         return MB.CreateSMEMatrixMultiply(
-	    Ops.LHS, Ops.RHS, LHSMatTy->getNumRows(), LHSMatTy->getNumColumns(),
+            Ops.LHS, Ops.RHS, LHSMatTy->getNumRows(), LHSMatTy->getNumColumns(),
             RHSMatTy->getNumColumns(),
-	    cast<BuiltinType>(Ty)->isSignedInteger());
+            cast<BuiltinType>(Ty)->isSignedInteger());
       }
       return MB.CreateScalarMultiply(Ops.LHS, Ops.RHS);
     }
@@ -4192,7 +4192,7 @@ Value *ScalarExprEmitter::EmitAdd(const BinOpInfo &op) {
     assert(isa<BuiltinType>(Ty) && "SME types should be BuiltinType.");
     return MB.CreateSMEMatrixBinOp(
         op.LHS, op.RHS, MatTy->getNumRows(), MatTy->getNumColumns(),
-	cast<BuiltinType>(Ty)->isSignedInteger(), "add");
+        cast<BuiltinType>(Ty)->isSignedInteger(), "add");
   }
 
   if (op.Ty->isUnsignedIntegerType() &&
@@ -4349,14 +4349,14 @@ Value *ScalarExprEmitter::EmitSub(const BinOpInfo &op) {
       llvm::MatrixBuilder MB(Builder);
       CodeGenFunction::CGFPOptionsRAII FPOptsRAII(CGF, op.FPFeatures);
       auto *MatTy =
-	  cast<ConstantMatrixType>(op.E->getType().getCanonicalType());
+          cast<ConstantMatrixType>(op.E->getType().getCanonicalType());
       auto Ty = MatTy->getElementType();
       if (!CGF.getContext().getTargetInfo().hasFeature("sme") ||
           !MatrixType::isValidTypeForSME(Ty))
-	return MB.CreateSub(op.LHS, op.RHS);
+        return MB.CreateSub(op.LHS, op.RHS);
       assert(isa<BuiltinType>(Ty) && "SME types should be BuiltinType.");
       return MB.CreateSMEMatrixBinOp(
-	  op.LHS, op.RHS, MatTy->getNumRows(), MatTy->getNumColumns(),
+          op.LHS, op.RHS, MatTy->getNumRows(), MatTy->getNumColumns(),
           cast<BuiltinType>(Ty)->isSignedInteger(), "sub");
     }
 

llvm/include/llvm/IR/MatrixBuilder.h

@@ -61,7 +61,11 @@ class MatrixBuilder {
     if (!isa<LoadInst>(StoredValue))
       return Addr;
 
-    return cast<LoadInst>(StoredValue)->getPointerOperand();
+    Value *Ptr = cast<LoadInst>(StoredValue)->getPointerOperand();
+    // If Ptr is used once, its memory may be altered.
+    if (!Ptr->hasNUses(1))
+      return Addr;
+    return Ptr;
   }
 
   std::pair<Value *, Value *> splatScalarOperandIfNeeded(Value *LHS,

llvm/lib/CodeGen/MachineScheduler.cpp

@@ -196,8 +196,9 @@ static cl::opt<unsigned>
 extern cl::opt<bool> EnableMatrix;
 
 static cl::opt<unsigned>
-    BigBasicBlock("schedule-big-basic-block", cl::Hidden, cl::init(200),
-                  cl::desc("The limit to use while schedule a region "));
+    BigBBThreshold("big-basic-block-threshold", cl::Hidden, cl::init(200),
+                   cl::desc("The limit to use while schedule a region when "
+                            "matrix opertion is enabled"));
 
 // DAG subtrees must have at least this many nodes.
 static const unsigned MinSubtreeSize = 8;
@@ -642,7 +643,7 @@ void MachineSchedulerBase::scheduleRegions(ScheduleDAGInstrs &Scheduler,
       MachineBasicBlock::iterator RegionEnd = R.RegionEnd;
       unsigned NumRegionInstrs = R.NumRegionInstrs;
 
-      if (EnableMatrix && NumRegionInstrs > BigBasicBlock)
+      if (EnableMatrix && NumRegionInstrs > BigBBThreshold)
         continue;
 
       // Notify the scheduler of the region, even if we may skip scheduling

llvm/lib/CodeGen/ScheduleDAGInstrs.cpp

@@ -97,6 +97,8 @@ static cl::opt<bool> SchedPrintCycles(
 static unsigned getReductionSize() {
   // Always reduce a huge region with half of the elements, except
   // when user sets this number explicitly.
+  // When matrix operation is enabled, reduce more aggressively to reduce
+  // compile time impact.
   if (ReductionSize.getNumOccurrences() == 0) {
     if (EnableMatrix)
       return HugeRegion / 20;