dialects: (riscv) make get_constant_value agnostic

[osmanyasar05]

Breaking down PR #5589, this PR makes the get_constant_value agnostic of input type.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.26%. Comparing base (2cb4ced) to head (f67782d).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #5648   +/-   ##
=======================================
  Coverage   86.26%   86.26%           
=======================================
  Files         403      403           
  Lines       56797    56827   +30     
  Branches     6544     6554   +10     
=======================================
+ Hits        48994    49021   +27     
+ Misses       6277     6275    -2     
- Partials     1526     1531    +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[superlopuh]

Oh actually, can you please add a test that checks that one of the patterns that uses this new functionality actually works with a 64-bit immediate?

[superlopuh]

I'm not sure why all the other changes were made, let's revert them and add unit tests for get_constant_value specifically?

[osmanyasar05]

I'm not sure why all the other changes were made, let's revert them and add unit tests for get_constant_value specifically?

without those changes if the immediate is rv64, the pattern would incorrectly rewrite it using rv32.LiOp though?

[superlopuh]

This still opens a new can of worms, and would need testing separately, how about we add asserts in the patterns that the type returned is i32 and address updating the patterns later?

[osmanyasar05]

i just left the patterns as they were and added the tests.

[superlopuh]

Thank you @osmanyasar05! I'm sorry about the churn, but I meant the unit test that tests this helper specifically: test_get_constant_value in tests/dialects/test_riscv.py. Could you please expand it to test for rv64.li? We can then take a look at the filecheck tests in a subsequent PR.

[osmanyasar05]

Thank you @osmanyasar05! I'm sorry about the churn, but I meant the unit test that tests this helper specifically: test_get_constant_value in tests/dialects/test_riscv.py. Could you please expand it to test for rv64.li? We can then take a look at the filecheck tests in a subsequent PR.

ah ok, no worries, sorry for the misunderstanding. I deleted the unit tests, so we can add them in the next pr, and added the test you mentioned!