Skip to content

Instruction Class Clean-Up#389

Merged
FinnWilkinson merged 24 commits intodevfrom
insn-class-clean
Feb 20, 2024
Merged

Instruction Class Clean-Up#389
FinnWilkinson merged 24 commits intodevfrom
insn-class-clean

Conversation

@FinnWilkinson
Copy link
Copy Markdown
Contributor

@FinnWilkinson FinnWilkinson commented Feb 14, 2024

Generally tidies the instruction classes and moves some common functions & member variables into the base class.

Also includes new classes in Container.hh which mitigates slowdown caused by changing sourceValues_, results_, sourceRegisters_, destinationRegisters_ to std::vector when SME was first supported. Said class has uses std::variant which contains std::array (used by default) and std::vector (used on call of makeSME()). This means most instructions can use std::array and get speed benefits, and SME instructions get the flexibility of std::vecotr (needed due to variable and large amount of operands).

Closes #355

AArch64 speeds with new Constants.hh (run on M1 MacBook Pro). TX2.yaml used for tx2 binaries, a64fx.yaml for armv8.4-a(+sve) binaries, a64fx_SME.yaml for all SME binaries. SME binaries run matmul kernel for 100 iterations on square inputs.

Binary MIPS dev time/ms dev MIPS insn-class-clean time/ms insn-class-clean
bude-armclang20.0-armv8.4-a 1.2 27033 1.4 22432
bude-armclang20.0-armv8.4-a+sve 0.87 12522 1.1 10078
bude-armclang20.0-tx2 1.2 27276 1.3 25262
bude-gcc10.3.0-armv8.4-a 1.2 27048 1.2 26280
bude-gcc10.3.0-armv8.4-a+sve 0.86 11912 0.92 11195
bude-gcc10.3.0-tx2 1.2 26344 1.3 24695
bude-gcc8.3.0-armv8.4-a 1.1 28607 1.2 26451
bude-gcc8.3.0-armv8.4-a+sve 0.86 12303 0.97 10880
bude-gcc8.3.0-tx2 1.2 27657 1.3 25550
bude-gcc9.3.0-armv8.4-a 1.1 28521 1.2 26678
bude-gcc9.3.0-armv8.4-a+sve 0.8 12520 0.94 10590
bude-gcc9.3.0-tx2 1.2 27505 1.3 26040
stream-armclang20.0-armv8.4-a 0.78 16179 1 12644
stream-armclang20.0-armv8.4-a+sve 0.82 9180 0.99 7610
stream-armclang20.0-tx2 0.64 15039 0.76 12591
stream-gcc10.3.0-armv8.4-a 0.97 15709 1.2 12856
stream-gcc10.3.0-armv8.4-a+sve 0.68 11046 0.85 8809
stream-gcc10.3.0-tx2 1 14667 1.2 13210
stream-gcc8.3.0-armv8.4-a 1 14808 1.1 13695
stream-gcc8.3.0-armv8.4-a+sve 0.68 10956 0.76 9871
stream-gcc8.3.0-tx2 0.99 15327 1.1 13459
stream-gcc9.3.0-armv8.4-a 1 14642 1.1 13574
stream-gcc9.3.0-armv8.4-a+sve 0.72 10311 0.75 9877
stream-gcc9.3.0-tx2 1 14942 1.1 13881
sme_matmul_fp32 m=n=k=64 0.72 2138 0.75 2057
sme_matmul_fp32 m=n=k=128 0.8 10099 0.85 9561
sme_matmul_fp32 m=n=k=256 0.82 59515 0.86 56645

@FinnWilkinson FinnWilkinson added code consistency Cleaning up existing code to align with rest of project 0.9.6 Part of SimEng Release 0.9.6 labels Feb 14, 2024
@FinnWilkinson FinnWilkinson self-assigned this Feb 14, 2024
@FinnWilkinson FinnWilkinson linked an issue Feb 14, 2024 that may be closed by this pull request
Comment thread src/include/simeng/arch/aarch64/Instruction.hh Outdated
Comment thread src/include/simeng/arch/aarch64/Instruction.hh Outdated
Comment thread src/include/simeng/arch/aarch64/Instruction.hh Outdated
Comment thread src/include/simeng/arch/riscv/Instruction.hh Outdated
Comment thread src/include/simeng/arch/riscv/Instruction.hh Outdated
Comment thread src/lib/arch/riscv/Instruction.cc Outdated
Comment thread src/include/simeng/arch/aarch64/Container.hh Outdated
Comment thread src/include/simeng/arch/aarch64/Container.hh Outdated
Comment thread src/include/simeng/arch/aarch64/Container.hh Outdated
Comment thread src/include/simeng/arch/aarch64/Container.hh Outdated
Comment thread src/include/simeng/arch/aarch64/Container.hh Outdated
Comment thread src/include/simeng/arch/aarch64/Container.hh Outdated
Comment thread src/include/simeng/arch/aarch64/Container.hh Outdated
Comment thread src/lib/arch/aarch64/Instruction.cc Outdated
Comment thread src/lib/arch/aarch64/Instruction_decode.cc Outdated
Comment thread src/include/simeng/arch/aarch64/Container.hh Outdated
Copy link
Copy Markdown
Contributor

@ABenC377 ABenC377 left a comment

Choose a reason for hiding this comment

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

Looks good other than a comment typo

Comment thread src/include/simeng/arch/aarch64/Container.hh Outdated
Copy link
Copy Markdown
Contributor

@JosephMoore25 JosephMoore25 left a comment

Choose a reason for hiding this comment

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

Looks good, and yields impressive speedups. Happy to approve once other's comments have been handled, but nothing extra that I could find.

Comment thread src/include/simeng/arch/aarch64/Container.hh Outdated
JosephMoore25
JosephMoore25 previously approved these changes Feb 16, 2024
Copy link
Copy Markdown
Contributor

@JosephMoore25 JosephMoore25 left a comment

Choose a reason for hiding this comment

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

Nice work. I like the change to using just operandContainer.

Comment thread src/include/simeng/arch/aarch64/Instruction.hh
Comment thread src/include/simeng/arch/aarch64/Container.hh
ABenC377
ABenC377 previously approved these changes Feb 19, 2024
@FinnWilkinson FinnWilkinson dismissed stale reviews from ABenC377 and JosephMoore25 via e0c402b February 20, 2024 10:16
ABenC377
ABenC377 previously approved these changes Feb 20, 2024
Copy link
Copy Markdown
Contributor

@ABenC377 ABenC377 left a comment

Choose a reason for hiding this comment

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

👏🏻

Comment thread src/include/simeng/arch/aarch64/operandContainer.hh
Comment thread test/unit/aarch64/OperandContainerTest.cc Outdated
ABenC377
ABenC377 previously approved these changes Feb 20, 2024
JosephMoore25
JosephMoore25 previously approved these changes Feb 20, 2024
Copy link
Copy Markdown
Contributor

@JosephMoore25 JosephMoore25 left a comment

Choose a reason for hiding this comment

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

Looks good. A shame we couldn't get isInstruction cleaner, but I'll have a think for next release.

@FinnWilkinson FinnWilkinson dismissed stale reviews from JosephMoore25 and ABenC377 via 7f75799 February 20, 2024 13:07
dANW34V3R
dANW34V3R previously approved these changes Feb 20, 2024
jj16791
jj16791 previously approved these changes Feb 20, 2024
JosephMoore25
JosephMoore25 previously approved these changes Feb 20, 2024
Comment thread src/include/simeng/arch/aarch64/Instruction.hh Outdated
Comment thread src/include/simeng/arch/aarch64/Instruction.hh Outdated
@FinnWilkinson FinnWilkinson dismissed stale reviews from JosephMoore25 and jj16791 via f1d2dc4 February 20, 2024 16:46
@FinnWilkinson FinnWilkinson merged commit c5fb7c6 into dev Feb 20, 2024
@FinnWilkinson FinnWilkinson mentioned this pull request Feb 20, 2024
@FinnWilkinson FinnWilkinson deleted the insn-class-clean branch February 21, 2024 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

0.9.6 Part of SimEng Release 0.9.6 code consistency Cleaning up existing code to align with rest of project

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Clean Up Instruction Class

6 participants