Skip to content

feat: Parse and verify RISCV stack operations#809

Merged
math-fehr merged 2 commits into
mainfrom
nchappe/parse-riscv-stack
Jun 10, 2026
Merged

feat: Parse and verify RISCV stack operations#809
math-fehr merged 2 commits into
mainfrom
nchappe/parse-riscv-stack

Conversation

@nchappe

@nchappe nchappe commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

This departs a bit from xdslproject/xdsl#6086 as alloca expects a value_type attribute to determine the size of the allocation, and it returns a RISCV register.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

VeIR Benchmarks

Details
Benchmark suite Current: 85367f4 Previous: 74ee571 Ratio
add-fold-worklist/create 2226000 ns (± 106330) 1853000 ns (± 23158) 1.20
add-fold-worklist/rewrite 3768000 ns (± 297353) 3362000 ns (± 53630) 1.12
add-fold-worklist-local/create 2290000 ns (± 103468) 1865000 ns (± 91759) 1.23
add-fold-worklist-local/rewrite 3044000 ns (± 38473) 2833000 ns (± 66759) 1.07
add-zero-worklist/create 2208000 ns (± 50715) 1844000 ns (± 85301) 1.20
add-zero-worklist/rewrite 2497000 ns (± 86002) 2174000 ns (± 31092) 1.15
add-zero-reuse-worklist/create 1824500 ns (± 88529) 1558000 ns (± 10320) 1.17
add-zero-reuse-worklist/rewrite 1984500 ns (± 40153) 1767000 ns (± 20550) 1.12
mul-two-worklist/create 2216000 ns (± 33731) 1855000 ns (± 83071) 1.19
mul-two-worklist/rewrite 5228000 ns (± 64213) 4729000 ns (± 61733) 1.11
add-fold-forwards/create 2191000 ns (± 76917) 1874000 ns (± 87837) 1.17
add-fold-forwards/rewrite 3018000 ns (± 27392) 2683000 ns (± 39771) 1.12
add-zero-forwards/create 2222000 ns (± 71248) 1876500 ns (± 94595) 1.18
add-zero-forwards/rewrite 1974000 ns (± 25704) 1725000 ns (± 29016) 1.14
add-zero-reuse-forwards/create 1836000 ns (± 43365) 1554000 ns (± 47788) 1.18
add-zero-reuse-forwards/rewrite 1528000 ns (± 54684) 1499000 ns (± 69831) 1.02
mul-two-forwards/create 2201000 ns (± 104610) 1853000 ns (± 86130) 1.19
mul-two-forwards/rewrite 3650000 ns (± 75145) 3230000 ns (± 97941) 1.13
add-zero-reuse-first/create 1771000 ns (± 91141) 1540000 ns (± 45075) 1.15
add-zero-reuse-first/rewrite 8000 ns (± 1278) 9000 ns (± 408) 0.89
add-zero-lots-of-reuse-first/create 1897000 ns (± 94203) 1556000 ns (± 27499) 1.22
add-zero-lots-of-reuse-first/rewrite 782000 ns (± 27004) 750000 ns (± 12582) 1.04

This comment was automatically generated by workflow using github-action-benchmark.

@regehr regehr self-requested a review June 9, 2026 19:34
Comment thread Test/RISCV_Stack/identity.mlir Outdated
@regehr

regehr commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

I'm onboard with alloca here, but I don't understand why we would want stack-specific loads and stores. In general, a pointer can refer either to the stack, or the heap, or globals -- so it's not going to be easy to actually use these in practice, except in restricted situations. I would expect that the RISC-V memory model is a flat 64-bit address space and then normal loads and stores can go anywhere inside it.

@math-fehr

Copy link
Copy Markdown
Collaborator

But let's wait for @regehr to approve it once the comments are addressed

Base automatically changed from nchappe/riscv-stack-slot-type to main June 9, 2026 22:14
@nchappe nchappe force-pushed the nchappe/parse-riscv-stack branch from b7dab82 to 5c6c869 Compare June 10, 2026 11:48
@nchappe

nchappe commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

Updated as discussed, and the stack slot type becomes unnecessary.

nchappe added 2 commits June 10, 2026 13:39
This departs a bit from xdslproject/xdsl#6086
as alloca expects a value_type attribute to determine the size of the
allocation, and it returns a RISCV register.
@nchappe nchappe force-pushed the nchappe/parse-riscv-stack branch from 5c6c869 to 85367f4 Compare June 10, 2026 12:39

@regehr regehr left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM!

@math-fehr math-fehr added this pull request to the merge queue Jun 10, 2026
Merged via the queue into main with commit 2374103 Jun 10, 2026
5 checks passed
@math-fehr math-fehr deleted the nchappe/parse-riscv-stack branch June 10, 2026 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants