Skip to content

[CIR][ThroughMLIR] Add support for almabench and fbench from SingleSource test suite. #1562

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

goxul
Copy link
Contributor

@goxul goxul commented Apr 14, 2025

From the SingleSource test suite, almabench had a missing ATan2 implemention, whereas fbench had missing asin support. This PR adds support for both.

  1. A lowering for ATan2 is added via ThroughMLIR.
  2. NYI for Asin changed with the appropriate invocation.

@goxul
Copy link
Contributor Author

goxul commented Apr 14, 2025

With this, both the tests pass.

/Documents/test-suite/build main % llvm-lit --timeout=60 -v SingleSource/Benchmarks/Misc/fbench.test SingleSource/Benchmarks/CoyoteBench/almabench.test 
llvm-lit: /Users/gokulnathp/Documents/llvm-project/llvm/utils/lit/lit/main.py:72: note: The test suite configuration requested an individual test timeout of 0 seconds but a timeout of 60 seconds was requested on the command line. Forcing timeout to be 60 seconds.
-- Testing: 2 tests, 2 workers --
PASS: test-suite :: SingleSource/Benchmarks/Misc/fbench.test (1 of 2)
********** TEST 'test-suite :: SingleSource/Benchmarks/Misc/fbench.test' RESULTS **********
compile_time: 0.0675 
exec_time: 0.4681 
hash: "d677e99cc79e1f469b0f24b59dcb1e2a" 
link_time: 0.0615 
size: 50568 
size.__bss: 1216 
size.__common: 4 
size.__const: 64 
size.__cstring: 201 
size.__data: 4 
size.__eh_frame: 176 
size.__got: 88 
size.__stubs: 132 
size.__text: 2336 
size.__unwind_info: 96 
**********
PASS: test-suite :: SingleSource/Benchmarks/CoyoteBench/almabench.test (2 of 2)
********** TEST 'test-suite :: SingleSource/Benchmarks/CoyoteBench/almabench.test' RESULTS **********
compile_time: 0.0670 
exec_time: 1.6678 
hash: "e8b207d5b71848a2b5d279ae6563ad28" 
link_time: 0.0726 
size: 34168 
size.__const: 4906 
size.__eh_frame: 264 
size.__got: 72 
size.__stubs: 96 
size.__text: 4028 
size.__unwind_info: 112 
**********

Testing Time: 1.88s

Total Discovered Tests: 2
  Passed: 2 (100.00%)

Not sure if this enables some more test cases as well.

@goxul goxul changed the title [CIR][LowerMLIR] Add support for almabench and fbench from SingleSource test suite. [CIR][ThroughMLIR] Add support for almabench and fbench from SingleSource test suite. Apr 14, 2025
@@ -4802,6 +4803,7 @@ def FMinimumOp : BinaryFPToFPBuiltinOp<"fminimum", "MinimumOp">;
def FModOp : BinaryFPToFPBuiltinOp<"fmod", "FRemOp">;
def PowOp : BinaryFPToFPBuiltinOp<"pow", "PowOp">;


Copy link
Member

Choose a reason for hiding this comment

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

No need for an extra newline here

@@ -293,6 +293,22 @@ class CIRUnaryMathOpLowering : public mlir::OpConversionPattern<CIROp> {
}
};

class CIRATan2OpLowering : public mlir::OpConversionPattern<cir::ATan2Op> {
Copy link
Member

Choose a reason for hiding this comment

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

While here, please add lowering and a test for direct LLVM lowering.

@bcardosolopes
Copy link
Member

Thanks for working on this @goxul.

Please change the title to be about the actual adding of ATan2, etc. You can add more information about the benchmarks in the description. Question: I wasn't expecting all these benchmarks to be working in the throughMLIR path, are you sure this is what you are running? Anyways, if direct LLVM also is missing this lowering, please go ahead and add it as part of this PR.

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.

2 participants