Skip to content

[MLIR][Transforms] Correct block sorting utils name (NFC) #92558

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 1 commit into from
May 17, 2024

Conversation

Dinistro
Copy link
Contributor

This commit renames the name of the block sorting utility function to getBlocksSortedByDominance. A topological order is not defined on a general directed graph, so the previous name did not make sense.

This commit renames the name of the block sorting utility function to
`getBlocksSortedByDominance`.
@llvmbot
Copy link
Member

llvmbot commented May 17, 2024

@llvm/pr-subscribers-openacc
@llvm/pr-subscribers-mlir-core
@llvm/pr-subscribers-flang-openmp
@llvm/pr-subscribers-mlir-openacc

@llvm/pr-subscribers-mlir-llvm

Author: Christian Ulmann (Dinistro)

Changes

This commit renames the name of the block sorting utility function to getBlocksSortedByDominance. A topological order is not defined on a general directed graph, so the previous name did not make sense.


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

6 Files Affected:

  • (modified) mlir/include/mlir/Transforms/RegionUtils.h (+3-2)
  • (modified) mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp (+1-1)
  • (modified) mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp (+26-28)
  • (modified) mlir/lib/Target/LLVMIR/ModuleTranslation.cpp (+1-1)
  • (modified) mlir/lib/Transforms/Mem2Reg.cpp (+1-1)
  • (modified) mlir/lib/Transforms/Utils/RegionUtils.cpp (+1-1)
diff --git a/mlir/include/mlir/Transforms/RegionUtils.h b/mlir/include/mlir/Transforms/RegionUtils.h
index 192ff71384059..f65d0d44eef42 100644
--- a/mlir/include/mlir/Transforms/RegionUtils.h
+++ b/mlir/include/mlir/Transforms/RegionUtils.h
@@ -87,8 +87,9 @@ LogicalResult eraseUnreachableBlocks(RewriterBase &rewriter,
 LogicalResult runRegionDCE(RewriterBase &rewriter,
                            MutableArrayRef<Region> regions);
 
-/// Get a topologically sorted list of blocks of the given region.
-SetVector<Block *> getTopologicallySortedBlocks(Region &region);
+/// Get a list of blocks that is sorted according to dominance. This sort is
+/// stable.
+SetVector<Block *> getBlocksSortedByDominance(Region &region);
 
 } // namespace mlir
 
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp
index b964d1c082b20..eeda245ce969f 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp
@@ -392,7 +392,7 @@ static LogicalResult convertDataOp(acc::DataOp &op,
   llvm::BasicBlock *endDataBlock = llvm::BasicBlock::Create(
       ctx, "acc.end_data", builder.GetInsertBlock()->getParent());
 
-  SetVector<Block *> blocks = getTopologicallySortedBlocks(op.getRegion());
+  SetVector<Block *> blocks = getBlocksSortedByDominance(op.getRegion());
   for (Block *bb : blocks) {
     llvm::BasicBlock *llvmBB = moduleTranslation.lookupBlock(bb);
     if (bb->isEntryBlock()) {
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index a7294632d6667..aa3c516b0c5d6 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -198,7 +198,7 @@ static llvm::BasicBlock *convertOmpOpRegions(
 
   // Convert blocks one by one in topological order to ensure
   // defs are converted before uses.
-  SetVector<Block *> blocks = getTopologicallySortedBlocks(region);
+  SetVector<Block *> blocks = getBlocksSortedByDominance(region);
   for (Block *bb : blocks) {
     llvm::BasicBlock *llvmBB = moduleTranslation.lookupBlock(bb);
     // Retarget the branch of the entry block to the entry block of the
@@ -2146,40 +2146,38 @@ getFirstOrLastMappedMemberPtr(mlir::omp::MapInfoOp mapInfo, bool first) {
   llvm::SmallVector<size_t> indices(shape[0]);
   std::iota(indices.begin(), indices.end(), 0);
 
-  llvm::sort(
-      indices.begin(), indices.end(), [&](const size_t a, const size_t b) {
-        auto indexValues = indexAttr.getValues<int32_t>();
-        for (int i = 0;
-             i < shape[1];
-             ++i) {
-          int aIndex = indexValues[a * shape[1] + i];
-          int bIndex = indexValues[b * shape[1] + i];
+  llvm::sort(indices.begin(), indices.end(),
+             [&](const size_t a, const size_t b) {
+               auto indexValues = indexAttr.getValues<int32_t>();
+               for (int i = 0; i < shape[1]; ++i) {
+                 int aIndex = indexValues[a * shape[1] + i];
+                 int bIndex = indexValues[b * shape[1] + i];
 
-          if (aIndex == bIndex)
-            continue;
+                 if (aIndex == bIndex)
+                   continue;
 
-          if (aIndex != -1 && bIndex == -1)
-            return false;
+                 if (aIndex != -1 && bIndex == -1)
+                   return false;
 
-          if (aIndex == -1 && bIndex != -1)
-            return true;
+                 if (aIndex == -1 && bIndex != -1)
+                   return true;
 
-          // A is earlier in the record type layout than B
-          if (aIndex < bIndex)
-            return first;
+                 // A is earlier in the record type layout than B
+                 if (aIndex < bIndex)
+                   return first;
 
-          if (bIndex < aIndex)
-            return !first;
-        }
+                 if (bIndex < aIndex)
+                   return !first;
+               }
 
-        // Iterated the entire list and couldn't make a decision, all elements
-        // were likely the same. Return false, since the sort comparator should
-        // return false for equal elements.
-        return false;
-      });
+               // Iterated the entire list and couldn't make a decision, all
+               // elements were likely the same. Return false, since the sort
+               // comparator should return false for equal elements.
+               return false;
+             });
 
-    return llvm::cast<mlir::omp::MapInfoOp>(
-          mapInfo.getMembers()[indices.front()].getDefiningOp());
+  return llvm::cast<mlir::omp::MapInfoOp>(
+      mapInfo.getMembers()[indices.front()].getDefiningOp());
 }
 
 /// This function calculates the array/pointer offset for map data provided
diff --git a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
index 669b95a9c6a5b..21e7c0b50d2a4 100644
--- a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
@@ -1320,7 +1320,7 @@ LogicalResult ModuleTranslation::convertOneFunction(LLVMFuncOp func) {
 
   // Then, convert blocks one by one in topological order to ensure defs are
   // converted before uses.
-  auto blocks = getTopologicallySortedBlocks(func.getBody());
+  auto blocks = getBlocksSortedByDominance(func.getBody());
   for (Block *bb : blocks) {
     CapturingIRBuilder builder(llvmContext);
     if (failed(convertBlockImpl(*bb, bb->isEntryBlock(), builder,
diff --git a/mlir/lib/Transforms/Mem2Reg.cpp b/mlir/lib/Transforms/Mem2Reg.cpp
index e096747741c0a..e2e240ad865ce 100644
--- a/mlir/lib/Transforms/Mem2Reg.cpp
+++ b/mlir/lib/Transforms/Mem2Reg.cpp
@@ -517,7 +517,7 @@ getOrCreateBlockIndices(BlockIndexCache &blockIndexCache, Region *region) {
     return it->second;
 
   DenseMap<Block *, size_t> &blockIndices = it->second;
-  SetVector<Block *> topologicalOrder = getTopologicallySortedBlocks(*region);
+  SetVector<Block *> topologicalOrder = getBlocksSortedByDominance(*region);
   for (auto [index, block] : llvm::enumerate(topologicalOrder))
     blockIndices[block] = index;
   return blockIndices;
diff --git a/mlir/lib/Transforms/Utils/RegionUtils.cpp b/mlir/lib/Transforms/Utils/RegionUtils.cpp
index e25867b527b71..192f59b353295 100644
--- a/mlir/lib/Transforms/Utils/RegionUtils.cpp
+++ b/mlir/lib/Transforms/Utils/RegionUtils.cpp
@@ -837,7 +837,7 @@ LogicalResult mlir::simplifyRegions(RewriterBase &rewriter,
                  mergedIdenticalBlocks);
 }
 
-SetVector<Block *> mlir::getTopologicallySortedBlocks(Region &region) {
+SetVector<Block *> mlir::getBlocksSortedByDominance(Region &region) {
   // For each block that has not been visited yet (i.e. that has no
   // predecessors), add it to the list as well as its successors.
   SetVector<Block *> blocks;

@llvmbot
Copy link
Member

llvmbot commented May 17, 2024

@llvm/pr-subscribers-mlir

Author: Christian Ulmann (Dinistro)

Changes

This commit renames the name of the block sorting utility function to getBlocksSortedByDominance. A topological order is not defined on a general directed graph, so the previous name did not make sense.


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

6 Files Affected:

  • (modified) mlir/include/mlir/Transforms/RegionUtils.h (+3-2)
  • (modified) mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp (+1-1)
  • (modified) mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp (+26-28)
  • (modified) mlir/lib/Target/LLVMIR/ModuleTranslation.cpp (+1-1)
  • (modified) mlir/lib/Transforms/Mem2Reg.cpp (+1-1)
  • (modified) mlir/lib/Transforms/Utils/RegionUtils.cpp (+1-1)
diff --git a/mlir/include/mlir/Transforms/RegionUtils.h b/mlir/include/mlir/Transforms/RegionUtils.h
index 192ff71384059..f65d0d44eef42 100644
--- a/mlir/include/mlir/Transforms/RegionUtils.h
+++ b/mlir/include/mlir/Transforms/RegionUtils.h
@@ -87,8 +87,9 @@ LogicalResult eraseUnreachableBlocks(RewriterBase &rewriter,
 LogicalResult runRegionDCE(RewriterBase &rewriter,
                            MutableArrayRef<Region> regions);
 
-/// Get a topologically sorted list of blocks of the given region.
-SetVector<Block *> getTopologicallySortedBlocks(Region &region);
+/// Get a list of blocks that is sorted according to dominance. This sort is
+/// stable.
+SetVector<Block *> getBlocksSortedByDominance(Region &region);
 
 } // namespace mlir
 
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp
index b964d1c082b20..eeda245ce969f 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp
@@ -392,7 +392,7 @@ static LogicalResult convertDataOp(acc::DataOp &op,
   llvm::BasicBlock *endDataBlock = llvm::BasicBlock::Create(
       ctx, "acc.end_data", builder.GetInsertBlock()->getParent());
 
-  SetVector<Block *> blocks = getTopologicallySortedBlocks(op.getRegion());
+  SetVector<Block *> blocks = getBlocksSortedByDominance(op.getRegion());
   for (Block *bb : blocks) {
     llvm::BasicBlock *llvmBB = moduleTranslation.lookupBlock(bb);
     if (bb->isEntryBlock()) {
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index a7294632d6667..aa3c516b0c5d6 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -198,7 +198,7 @@ static llvm::BasicBlock *convertOmpOpRegions(
 
   // Convert blocks one by one in topological order to ensure
   // defs are converted before uses.
-  SetVector<Block *> blocks = getTopologicallySortedBlocks(region);
+  SetVector<Block *> blocks = getBlocksSortedByDominance(region);
   for (Block *bb : blocks) {
     llvm::BasicBlock *llvmBB = moduleTranslation.lookupBlock(bb);
     // Retarget the branch of the entry block to the entry block of the
@@ -2146,40 +2146,38 @@ getFirstOrLastMappedMemberPtr(mlir::omp::MapInfoOp mapInfo, bool first) {
   llvm::SmallVector<size_t> indices(shape[0]);
   std::iota(indices.begin(), indices.end(), 0);
 
-  llvm::sort(
-      indices.begin(), indices.end(), [&](const size_t a, const size_t b) {
-        auto indexValues = indexAttr.getValues<int32_t>();
-        for (int i = 0;
-             i < shape[1];
-             ++i) {
-          int aIndex = indexValues[a * shape[1] + i];
-          int bIndex = indexValues[b * shape[1] + i];
+  llvm::sort(indices.begin(), indices.end(),
+             [&](const size_t a, const size_t b) {
+               auto indexValues = indexAttr.getValues<int32_t>();
+               for (int i = 0; i < shape[1]; ++i) {
+                 int aIndex = indexValues[a * shape[1] + i];
+                 int bIndex = indexValues[b * shape[1] + i];
 
-          if (aIndex == bIndex)
-            continue;
+                 if (aIndex == bIndex)
+                   continue;
 
-          if (aIndex != -1 && bIndex == -1)
-            return false;
+                 if (aIndex != -1 && bIndex == -1)
+                   return false;
 
-          if (aIndex == -1 && bIndex != -1)
-            return true;
+                 if (aIndex == -1 && bIndex != -1)
+                   return true;
 
-          // A is earlier in the record type layout than B
-          if (aIndex < bIndex)
-            return first;
+                 // A is earlier in the record type layout than B
+                 if (aIndex < bIndex)
+                   return first;
 
-          if (bIndex < aIndex)
-            return !first;
-        }
+                 if (bIndex < aIndex)
+                   return !first;
+               }
 
-        // Iterated the entire list and couldn't make a decision, all elements
-        // were likely the same. Return false, since the sort comparator should
-        // return false for equal elements.
-        return false;
-      });
+               // Iterated the entire list and couldn't make a decision, all
+               // elements were likely the same. Return false, since the sort
+               // comparator should return false for equal elements.
+               return false;
+             });
 
-    return llvm::cast<mlir::omp::MapInfoOp>(
-          mapInfo.getMembers()[indices.front()].getDefiningOp());
+  return llvm::cast<mlir::omp::MapInfoOp>(
+      mapInfo.getMembers()[indices.front()].getDefiningOp());
 }
 
 /// This function calculates the array/pointer offset for map data provided
diff --git a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
index 669b95a9c6a5b..21e7c0b50d2a4 100644
--- a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
@@ -1320,7 +1320,7 @@ LogicalResult ModuleTranslation::convertOneFunction(LLVMFuncOp func) {
 
   // Then, convert blocks one by one in topological order to ensure defs are
   // converted before uses.
-  auto blocks = getTopologicallySortedBlocks(func.getBody());
+  auto blocks = getBlocksSortedByDominance(func.getBody());
   for (Block *bb : blocks) {
     CapturingIRBuilder builder(llvmContext);
     if (failed(convertBlockImpl(*bb, bb->isEntryBlock(), builder,
diff --git a/mlir/lib/Transforms/Mem2Reg.cpp b/mlir/lib/Transforms/Mem2Reg.cpp
index e096747741c0a..e2e240ad865ce 100644
--- a/mlir/lib/Transforms/Mem2Reg.cpp
+++ b/mlir/lib/Transforms/Mem2Reg.cpp
@@ -517,7 +517,7 @@ getOrCreateBlockIndices(BlockIndexCache &blockIndexCache, Region *region) {
     return it->second;
 
   DenseMap<Block *, size_t> &blockIndices = it->second;
-  SetVector<Block *> topologicalOrder = getTopologicallySortedBlocks(*region);
+  SetVector<Block *> topologicalOrder = getBlocksSortedByDominance(*region);
   for (auto [index, block] : llvm::enumerate(topologicalOrder))
     blockIndices[block] = index;
   return blockIndices;
diff --git a/mlir/lib/Transforms/Utils/RegionUtils.cpp b/mlir/lib/Transforms/Utils/RegionUtils.cpp
index e25867b527b71..192f59b353295 100644
--- a/mlir/lib/Transforms/Utils/RegionUtils.cpp
+++ b/mlir/lib/Transforms/Utils/RegionUtils.cpp
@@ -837,7 +837,7 @@ LogicalResult mlir::simplifyRegions(RewriterBase &rewriter,
                  mergedIdenticalBlocks);
 }
 
-SetVector<Block *> mlir::getTopologicallySortedBlocks(Region &region) {
+SetVector<Block *> mlir::getBlocksSortedByDominance(Region &region) {
   // For each block that has not been visited yet (i.e. that has no
   // predecessors), add it to the list as well as its successors.
   SetVector<Block *> blocks;

@llvmbot
Copy link
Member

llvmbot commented May 17, 2024

@llvm/pr-subscribers-mlir-openmp

Author: Christian Ulmann (Dinistro)

Changes

This commit renames the name of the block sorting utility function to getBlocksSortedByDominance. A topological order is not defined on a general directed graph, so the previous name did not make sense.


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

6 Files Affected:

  • (modified) mlir/include/mlir/Transforms/RegionUtils.h (+3-2)
  • (modified) mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp (+1-1)
  • (modified) mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp (+26-28)
  • (modified) mlir/lib/Target/LLVMIR/ModuleTranslation.cpp (+1-1)
  • (modified) mlir/lib/Transforms/Mem2Reg.cpp (+1-1)
  • (modified) mlir/lib/Transforms/Utils/RegionUtils.cpp (+1-1)
diff --git a/mlir/include/mlir/Transforms/RegionUtils.h b/mlir/include/mlir/Transforms/RegionUtils.h
index 192ff71384059..f65d0d44eef42 100644
--- a/mlir/include/mlir/Transforms/RegionUtils.h
+++ b/mlir/include/mlir/Transforms/RegionUtils.h
@@ -87,8 +87,9 @@ LogicalResult eraseUnreachableBlocks(RewriterBase &rewriter,
 LogicalResult runRegionDCE(RewriterBase &rewriter,
                            MutableArrayRef<Region> regions);
 
-/// Get a topologically sorted list of blocks of the given region.
-SetVector<Block *> getTopologicallySortedBlocks(Region &region);
+/// Get a list of blocks that is sorted according to dominance. This sort is
+/// stable.
+SetVector<Block *> getBlocksSortedByDominance(Region &region);
 
 } // namespace mlir
 
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp
index b964d1c082b20..eeda245ce969f 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenACC/OpenACCToLLVMIRTranslation.cpp
@@ -392,7 +392,7 @@ static LogicalResult convertDataOp(acc::DataOp &op,
   llvm::BasicBlock *endDataBlock = llvm::BasicBlock::Create(
       ctx, "acc.end_data", builder.GetInsertBlock()->getParent());
 
-  SetVector<Block *> blocks = getTopologicallySortedBlocks(op.getRegion());
+  SetVector<Block *> blocks = getBlocksSortedByDominance(op.getRegion());
   for (Block *bb : blocks) {
     llvm::BasicBlock *llvmBB = moduleTranslation.lookupBlock(bb);
     if (bb->isEntryBlock()) {
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index a7294632d6667..aa3c516b0c5d6 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -198,7 +198,7 @@ static llvm::BasicBlock *convertOmpOpRegions(
 
   // Convert blocks one by one in topological order to ensure
   // defs are converted before uses.
-  SetVector<Block *> blocks = getTopologicallySortedBlocks(region);
+  SetVector<Block *> blocks = getBlocksSortedByDominance(region);
   for (Block *bb : blocks) {
     llvm::BasicBlock *llvmBB = moduleTranslation.lookupBlock(bb);
     // Retarget the branch of the entry block to the entry block of the
@@ -2146,40 +2146,38 @@ getFirstOrLastMappedMemberPtr(mlir::omp::MapInfoOp mapInfo, bool first) {
   llvm::SmallVector<size_t> indices(shape[0]);
   std::iota(indices.begin(), indices.end(), 0);
 
-  llvm::sort(
-      indices.begin(), indices.end(), [&](const size_t a, const size_t b) {
-        auto indexValues = indexAttr.getValues<int32_t>();
-        for (int i = 0;
-             i < shape[1];
-             ++i) {
-          int aIndex = indexValues[a * shape[1] + i];
-          int bIndex = indexValues[b * shape[1] + i];
+  llvm::sort(indices.begin(), indices.end(),
+             [&](const size_t a, const size_t b) {
+               auto indexValues = indexAttr.getValues<int32_t>();
+               for (int i = 0; i < shape[1]; ++i) {
+                 int aIndex = indexValues[a * shape[1] + i];
+                 int bIndex = indexValues[b * shape[1] + i];
 
-          if (aIndex == bIndex)
-            continue;
+                 if (aIndex == bIndex)
+                   continue;
 
-          if (aIndex != -1 && bIndex == -1)
-            return false;
+                 if (aIndex != -1 && bIndex == -1)
+                   return false;
 
-          if (aIndex == -1 && bIndex != -1)
-            return true;
+                 if (aIndex == -1 && bIndex != -1)
+                   return true;
 
-          // A is earlier in the record type layout than B
-          if (aIndex < bIndex)
-            return first;
+                 // A is earlier in the record type layout than B
+                 if (aIndex < bIndex)
+                   return first;
 
-          if (bIndex < aIndex)
-            return !first;
-        }
+                 if (bIndex < aIndex)
+                   return !first;
+               }
 
-        // Iterated the entire list and couldn't make a decision, all elements
-        // were likely the same. Return false, since the sort comparator should
-        // return false for equal elements.
-        return false;
-      });
+               // Iterated the entire list and couldn't make a decision, all
+               // elements were likely the same. Return false, since the sort
+               // comparator should return false for equal elements.
+               return false;
+             });
 
-    return llvm::cast<mlir::omp::MapInfoOp>(
-          mapInfo.getMembers()[indices.front()].getDefiningOp());
+  return llvm::cast<mlir::omp::MapInfoOp>(
+      mapInfo.getMembers()[indices.front()].getDefiningOp());
 }
 
 /// This function calculates the array/pointer offset for map data provided
diff --git a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
index 669b95a9c6a5b..21e7c0b50d2a4 100644
--- a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
@@ -1320,7 +1320,7 @@ LogicalResult ModuleTranslation::convertOneFunction(LLVMFuncOp func) {
 
   // Then, convert blocks one by one in topological order to ensure defs are
   // converted before uses.
-  auto blocks = getTopologicallySortedBlocks(func.getBody());
+  auto blocks = getBlocksSortedByDominance(func.getBody());
   for (Block *bb : blocks) {
     CapturingIRBuilder builder(llvmContext);
     if (failed(convertBlockImpl(*bb, bb->isEntryBlock(), builder,
diff --git a/mlir/lib/Transforms/Mem2Reg.cpp b/mlir/lib/Transforms/Mem2Reg.cpp
index e096747741c0a..e2e240ad865ce 100644
--- a/mlir/lib/Transforms/Mem2Reg.cpp
+++ b/mlir/lib/Transforms/Mem2Reg.cpp
@@ -517,7 +517,7 @@ getOrCreateBlockIndices(BlockIndexCache &blockIndexCache, Region *region) {
     return it->second;
 
   DenseMap<Block *, size_t> &blockIndices = it->second;
-  SetVector<Block *> topologicalOrder = getTopologicallySortedBlocks(*region);
+  SetVector<Block *> topologicalOrder = getBlocksSortedByDominance(*region);
   for (auto [index, block] : llvm::enumerate(topologicalOrder))
     blockIndices[block] = index;
   return blockIndices;
diff --git a/mlir/lib/Transforms/Utils/RegionUtils.cpp b/mlir/lib/Transforms/Utils/RegionUtils.cpp
index e25867b527b71..192f59b353295 100644
--- a/mlir/lib/Transforms/Utils/RegionUtils.cpp
+++ b/mlir/lib/Transforms/Utils/RegionUtils.cpp
@@ -837,7 +837,7 @@ LogicalResult mlir::simplifyRegions(RewriterBase &rewriter,
                  mergedIdenticalBlocks);
 }
 
-SetVector<Block *> mlir::getTopologicallySortedBlocks(Region &region) {
+SetVector<Block *> mlir::getBlocksSortedByDominance(Region &region) {
   // For each block that has not been visited yet (i.e. that has no
   // predecessors), add it to the list as well as its successors.
   SetVector<Block *> blocks;

Copy link
Contributor

@gysit gysit left a comment

Choose a reason for hiding this comment

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

LGTM!

@Dinistro Dinistro merged commit e919df5 into main May 17, 2024
11 of 12 checks passed
@Dinistro Dinistro deleted the users/dinistro/rename-block-sorting branch May 17, 2024 15:36
Dinistro added a commit that referenced this pull request May 17, 2024
This commit fixes a breakage introduced by changing the name of the
block sorting function.

Related PR: #92558
@Dinistro
Copy link
Contributor Author

This broke the MLIR build and was fixed in 9e39a0c

This was broken because the branch of this PR did not yet include the commit from #90448 which introduced the usage of the renamed function. This is yet another example why a pre-merge CI pipeline would be preferable.

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