Skip to content

cpu-o3: fix pc nullptr for storeDataUop #485

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

Open
Rrraaaeee wants to merge 1 commit into
base: xs-dev
Choose a base branch
from
Open

cpu-o3: fix pc nullptr for storeDataUop #485

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: 2 additions & 0 deletions src/arch/riscv/isa/vector/base/vector_mem.temp.isa
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,8 @@ def template VlWholeConstructor {{
%(class_name)s::%(class_name)s(ExtMachInst _machInst)
: %(base_class)s("%(mnemonic)s", _machInst, %(op_class)s)
{
%(set_reg_idx_arr)s;

Comment on lines +426 to +427

@coderabbitai coderabbitai Bot Jan 9, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's understand the file structure and find all macro constructors
cat -n src/arch/riscv/isa/vector/base/vector_mem.temp.isa | head -50

Repository: OpenXiangShan/GEM5

Length of output: 1383

🏁 Script executed:

# Check the VlWholeConstructor around line 426-427
sed -n '400,450p' src/arch/riscv/isa/vector/base/vector_mem.temp.isa | cat -n

Repository: OpenXiangShan/GEM5

Length of output: 1598

🏁 Script executed:

# Check VsWholeConstructor (lines 1186-1208)
sed -n '1180,1210p' src/arch/riscv/isa/vector/base/vector_mem.temp.isa | cat -n

Repository: OpenXiangShan/GEM5

Length of output: 1055

🏁 Script executed:

# Search for all macro-level constructors in the file
rg -n 'def constructor.*:' src/arch/riscv/isa/vector/base/vector_mem.temp.isa | head -20

Repository: OpenXiangShan/GEM5

Length of output: 44

🏁 Script executed:

# Search for set_reg_idx_arr across the file to see where it appears
rg -n 'set_reg_idx_arr' src/arch/riscv/isa/vector/base/vector_mem.temp.isa

Repository: OpenXiangShan/GEM5

Length of output: 458

🏁 Script executed:

# Find all constructor definitions to map line numbers to constructors
rg -n 'def template.*Constructor|def template.*Declare' src/arch/riscv/isa/vector/base/vector_mem.temp.isa

Repository: OpenXiangShan/GEM5

Length of output: 987

🏁 Script executed:

# Get context around each set_reg_idx_arr occurrence to identify the constructor
for line in 117 351 426 459 560 688 821 1051 1163 1221 1288 1366; do
  echo "=== Line $line ==="
  sed -n "$((line-10)),$((line+2))p" src/arch/riscv/isa/vector/base/vector_mem.temp.isa | grep -E 'def template|class|::|%(class_name)s::' | head -3
done

Repository: OpenXiangShan/GEM5

Length of output: 2633

🏁 Script executed:

# More detailed context for the key comparisons
echo "=== Around line 426 (VlWholeConstructor claim) ==="
sed -n '415,435p' src/arch/riscv/isa/vector/base/vector_mem.temp.isa | cat -n

echo -e "\n=== Around line 1221 (likely near VsWholeConstructor) ==="
sed -n '1210,1230p' src/arch/riscv/isa/vector/base/vector_mem.temp.isa | cat -n

Repository: OpenXiangShan/GEM5

Length of output: 1623

🏁 Script executed:

# Let me verify which macro constructors have set_reg_idx_arr
# Check each macro constructor template

echo "=== VleConstructor (starts at line 45) ==="
sed -n '45,105p' src/arch/riscv/isa/vector/base/vector_mem.temp.isa | grep -n 'set_reg_idx_arr\|^%%'

echo -e "\n=== VleffConstructor (starts at line 310) ==="
sed -n '310,338p' src/arch/riscv/isa/vector/base/vector_mem.temp.isa | grep -n 'set_reg_idx_arr\|^%%'

echo -e "\n=== VlStrideConstructor (starts at line 504) ==="
sed -n '504,547p' src/arch/riscv/isa/vector/base/vector_mem.temp.isa | grep -n 'set_reg_idx_arr\|^%%'

echo -e "\n=== VlIndexConstructor (starts at line 623) ==="
sed -n '623,674p' src/arch/riscv/isa/vector/base/vector_mem.temp.isa | grep -n 'set_reg_idx_arr\|^%%'

echo -e "\n=== VseConstructor (starts at line 758) ==="
sed -n '758,809p' src/arch/riscv/isa/vector/base/vector_mem.temp.isa | grep -n 'set_reg_idx_arr\|^%%'

echo -e "\n=== VlmConstructor (starts at line 1003) ==="
sed -n '1003,1039p' src/arch/riscv/isa/vector/base/vector_mem.temp.isa | grep -n 'set_reg_idx_arr\|^%%'

echo -e "\n=== VsmConstructor (starts at line 1116) ==="
sed -n '1116,1151p' src/arch/riscv/isa/vector/base/vector_mem.temp.isa | grep -n 'set_reg_idx_arr\|^%%'

echo -e "\n=== VsWholeConstructor (starts at line 1186) ==="
sed -n '1186,1209p' src/arch/riscv/isa/vector/base/vector_mem.temp.isa | grep -n 'set_reg_idx_arr\|^%%'

echo -e "\n=== VsStrideConstructor (starts at line 1239) ==="
sed -n '1239,1275p' src/arch/riscv/isa/vector/base/vector_mem.temp.isa | grep -n 'set_reg_idx_arr\|^%%'

echo -e "\n=== VsIndexConstructor (starts at line 1311) ==="
sed -n '1311,1352p' src/arch/riscv/isa/vector/base/vector_mem.temp.isa | grep -n 'set_reg_idx_arr\|^%%'

Repository: OpenXiangShan/GEM5

Length of output: 538

🏁 Script executed:

# Let's directly check the content of each macro constructor more carefully
echo "=== VlWholeConstructor (lines 421-447) ==="
sed -n '421,447p' src/arch/riscv/isa/vector/base/vector_mem.temp.isa | cat -n

echo -e "\n=== VsWholeConstructor (lines 1186-1209) ==="
sed -n '1186,1209p' src/arch/riscv/isa/vector/base/vector_mem.temp.isa | cat -n

Repository: OpenXiangShan/GEM5

Length of output: 1820

🏁 Script executed:

# Check a few other macro constructors
echo "=== VleConstructor (lines 45-105) ==="
sed -n '45,105p' src/arch/riscv/isa/vector/base/vector_mem.temp.isa | cat -n

echo -e "\n=== VseConstructor (lines 758-809) ==="
sed -n '758,809p' src/arch/riscv/isa/vector/base/vector_mem.temp.isa | cat -n

Repository: OpenXiangShan/GEM5

Length of output: 4822

🏁 Script executed:

# Let me check the remaining macro constructors to be thorough
echo "=== VleffConstructor (lines 310-338) ==="
sed -n '310,338p' src/arch/riscv/isa/vector/base/vector_mem.temp.isa | cat -n

echo -e "\n=== VlStrideConstructor (lines 504-547) ==="
sed -n '504,547p' src/arch/riscv/isa/vector/base/vector_mem.temp.isa | cat -n

echo -e "\n=== VlIndexConstructor (lines 623-674) ==="
sed -n '623,674p' src/arch/riscv/isa/vector/base/vector_mem.temp.isa | cat -n

echo -e "\n=== VlmConstructor (lines 1003-1039) ==="
sed -n '1003,1039p' src/arch/riscv/isa/vector/base/vector_mem.temp.isa | cat -n

Repository: OpenXiangShan/GEM5

Length of output: 6280

🏁 Script executed:

echo "=== VsmConstructor (lines 1116-1151) ==="
sed -n '1116,1151p' src/arch/riscv/isa/vector/base/vector_mem.temp.isa | cat -n

echo -e "\n=== VsStrideConstructor (lines 1239-1275) ==="
sed -n '1239,1275p' src/arch/riscv/isa/vector/base/vector_mem.temp.isa | cat -n

echo -e "\n=== VsIndexConstructor (lines 1311-1352) ==="
sed -n '1311,1352p' src/arch/riscv/isa/vector/base/vector_mem.temp.isa | cat -n

Repository: OpenXiangShan/GEM5

Length of output: 4639

Pattern inconsistency: Why only VlWholeConstructor among macro constructors?

This change adds %(set_reg_idx_arr)s; initialization to the VlWholeConstructor macro-level constructor, making it the only macro constructor with this initialization. Verified observations:

  1. VsWholeConstructor (lines 1186-1209, the store equivalent) lacks this initialization
  2. All other macro constructors (VleConstructor, VleffConstructor, VseConstructor, VlStrideConstructor, VsStrideConstructor, VlIndexConstructor, VlmConstructor, VsmConstructor, VsIndexConstructor) lack this initialization
  3. All micro-level constructors have this initialization (e.g., line 459 for VlWholeMicro)

Since macro constructors create micro-ops but don't directly perform register operations, clarify:

  • Why does the macro-level constructor need register array initialization when no other macro constructor does?
  • Why is VlWholeConstructor treated differently from its store counterpart VsWholeConstructor?
🤖 Prompt for AI Agents 
In @src/arch/riscv/isa/vector/base/vector_mem.temp.isa around lines 426 - 427,
The VlWholeConstructor macro was changed to include %(set_reg_idx_arr)s; making
it inconsistent with other macro constructors and its store counterpart
VsWholeConstructor; revert this addition from VlWholeConstructor (or
alternatively add the same %(set_reg_idx_arr)s; to all other macro constructors
including VsWholeConstructor, VleConstructor, VleffConstructor, VseConstructor,
VlStrideConstructor, VsStrideConstructor, VlIndexConstructor, VlmConstructor,
VsmConstructor, VsIndexConstructor) so macro-level constructors remain
consistent and only micro-level constructors (e.g., VlWholeMicro and other micro
ops) perform register-array initialization.

size_t NFIELDS = machInst.nf + 1;
eew = 8;
StaticInstPtr microop;
Expand Down
Loading