Skip to content

[Layout] Utilizing IsEqual instead of StructuralEqual #1073

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.

Sign up for GitHub

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
Oct 20, 2025
Merged

[Layout] Utilizing IsEqual instead of StructuralEqual #1073

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/transform/layout_inference.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ class BufferUseDefCollector : public IRVisitorWithAnalyzer {
}
}
// If already in map, ensure they are structurally equal
ICHECK(StructuralEqual()(layout, layout_map[buffer]))
ICHECK(layout->IsEqual(layout_map[buffer].get()))
<< "Get different layout for " << buffer
<< "\n current layout: " << layout->DebugOutput()
<< "\n previous layout: " << layout_map[buffer]->DebugOutput();
Comment on lines +162 to 165
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 20, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Good improvement using specialized equality.

Replacing StructuralEqual() with layout->IsEqual() is appropriate here, as it leverages domain-specific equality logic for Layout objects rather than generic structural comparison.

To ensure consistency across the codebase, verify that all layout-to-layout comparisons use IsEqual() rather than StructuralEqual():

🏁 Script executed:

#!/bin/bash
# Description: Find potential layout equality comparisons that might need updating.
# Search for StructuralEqual usage with layout-related patterns.

echo "=== Searching for StructuralEqual() calls that might involve Layout objects ==="
rg -n -C3 'StructuralEqual\(\).*layout' --type=cpp

echo -e "\n=== Searching for layout variables being compared ==="
rg -n -C3 'layout.*StructuralEqual|StructuralEqual.*layout' --type=cpp

Length of output: 6136

🏁 Script executed:

#!/bin/bash
# Find Layout class definition and IsEqual method
echo "=== Searching for Layout class and IsEqual method ==="
rg -n -A5 'class Layout' --type=cpp | head -40

echo -e "\n=== Searching for IsEqual method in Layout ==="
rg -n -B2 -A8 'IsEqual\s*\(' --type=cpp --type=h | grep -A10 -B2 Layout | head -50

echo -e "\n=== Checking types in src/op/copy.cc around line 1468 ==="
sed -n '1450,1485p' src/op/copy.cc

echo -e "\n=== Checking if shared_layout is a Layout type ==="
rg -n -B10 'shared_layout' src/op/copy.cc | grep -E 'shared_layout|Layout|auto' | head -20

Length of output: 8292

🏁 Script executed:

#!/bin/bash
# Find Layout class definition and IsEqual method
echo "=== Searching for Layout class and IsEqual method ==="
rg -n -A5 'class Layout' --type=cpp --type=h | head -60

echo -e "\n=== Checking types in src/op/copy.cc around line 1468 ==="
sed -n '1450,1485p' src/op/copy.cc

echo -e "\n=== Checking variable declarations in src/op/copy.cc ==="
rg -n -B15 'shared_layout.*=' src/op/copy.cc | grep -A2 -B2 'shared_layout' | head -40

Length of output: 6418

🏁 Script executed:

#!/bin/bash
# Check what linear_layout is
echo "=== Finding linear_layout definition ==="
rg -n -B5 'linear_layout' src/op/copy.cc | head -30

echo -e "\n=== Finding makeQuarterBankSwizzleLayout return type ==="
rg -n -A3 'makeQuarterBankSwizzleLayout' src/op/copy.cc | head -20

echo -e "\n=== Finding makeQuarterBankSwizzleLayout function definition ==="
rg -n -B2 -A5 'makeQuarterBankSwizzleLayout\s*\(' --type=cpp --type=h | grep -A5 'Layout\|makeQuarterBank'

Length of output: 6703

🏁 Script executed:

#!/bin/bash
# Search for all StructuralEqual usage to identify any remaining Layout comparisons
echo "=== All StructuralEqual calls ==="
rg -n 'StructuralEqual\(\)' --type=cpp --type=h | grep -v test | grep -v '\.o:' | head -50

echo -e "\n=== Check if there are any other layout comparisons we missed ==="
rg -n -B2 'shared_layout.*StructuralEqual|StructuralEqual.*shared_layout' src/op/copy.cc

Length of output: 3596

Incomplete conversion: Layout comparisons in src/op/copy.cc still use StructuralEqual().

The change at line 162 replaces StructuralEqual() with layout->IsEqual(), but this conversion was not applied consistently across the codebase. Six Layout-to-Layout comparisons in src/op/copy.cc still use the generic StructuralEqual():

  • Line 1468: shared_layout vs linear_layout
  • Lines 1477, 1481, 1486, 1491: shared_layout vs swizzle layout results (quarter/half/full bank patterns)
  • Lines 1835, 1839, 1843: similar comparisons in a second function

Update these to use shared_layout->IsEqual(other_layout.get()) for consistency with the specialized equality logic.

🤖 Prompt for AI Agents 
In src/transform/layout_inference.cc around lines 162-165 the code was updated
to use layout->IsEqual(...) but similar Layout-to-Layout comparisons in
src/op/copy.cc remain using StructuralEqual(); update each listed comparison to
call the specialized IsEqual on the shared layout. Specifically, replace
occurrences like shared_layout->StructuralEqual(linear_layout.get()) or
shared_layout->StructuralEqual(swizzle_layout.get()) with
shared_layout->IsEqual(linear_layout.get()) and
shared_layout->IsEqual(swizzle_layout.get()) respectively at the noted locations
(around lines 1468, 1477, 1481, 1486, 1491 and around 1835, 1839, 1843); ensure
you call IsEqual on the left-hand shared_layout pointer and pass the other
layout.get() as the argument to preserve semantics.

Expand Down