sparcians / mavis

RISC-V Decoder
Apache License 2.0
1 stars 5 forks source link

makeInstDirectory does not support vector masks or any other special fields #19

Open Shubhf opened 4 weeks ago

Shubhf commented 4 weeks ago

I have tried to make changes for https://github.com/sparcians/mavis/issues/15 Pls suggest changes

klingaard commented 4 weeks ago

@kathlenemagnus, @bdutro am I missing something?

Shubhf commented 4 weeks ago

Ok I ll check this out and get back to you.

On Tue, 20 Aug 2024 at 7:17 PM, Knute @.***> wrote:

@kathlenemagnus https://github.com/kathlenemagnus, @bdutro https://github.com/bdutro am I missing something?

— Reply to this email directly, view it on GitHub https://github.com/sparcians/mavis/pull/19#issuecomment-2298907855, or unsubscribe https://github.com/notifications/unsubscribe-auth/A5WMIMSVSXJPFX7WBUWEBS3ZSNCIVAVCNFSM6AAAAABMZNHXE6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEOJYHEYDOOBVGU . You are receiving this because you authored the thread.Message ID: @.***>

kathlenemagnus commented 3 weeks ago

How about adding a new constructor type for the ExtractorDirectInfo class that takes in a map of special fields and their values? See ExtractorDirectInfo.h#L242C1-L276C7

Actually now that I look at this more closely, I do see a constructor that we could use:

ExtractorDirectInfo(const InstructionUniqueID uid, const RegListType &sources, const RegListType &dests,
                        const ValueListType& specials) :
        ExtractorDirectBase(uid), sources_(sources), dests_(dests), specials_(specials)
    {}

@Shubhf can you add a test for a vector add with a mask vm to see if this constructor will work?

Shubhf commented 3 weeks ago

yeah thats a good idea .. i ll try to implement it

On Wed, Aug 21, 2024 at 11:52 PM Kathlene Magnus @.***> wrote:

How about adding a new constructor type for the ExtractorDirectInfo class that takes in a map of special fields and their values? See ExtractorDirectInfo.h#L242C1-L276C7 https://github.com/sparcians/mavis/blob/fe4a557091d5322ded4e090f10fca1e9e0b1704a/mavis/ExtractorDirectInfo.h#L242C1-L276C7

Actually now that I look at this more closely, I do see a constructor that we could use:

ExtractorDirectInfo(const InstructionUniqueID uid, const RegListType &sources, const RegListType &dests, const ValueListType& specials) : ExtractorDirectBase(uid), sources(sources), dests(dests), specials_(specials) {}

@Shubhf https://github.com/Shubhf can you add a test for a vector add with a mask vm to see if this constructor will work?

— Reply to this email directly, view it on GitHub https://github.com/sparcians/mavis/pull/19#issuecomment-2302699125, or unsubscribe https://github.com/notifications/unsubscribe-auth/A5WMIMXF2BJDYCNTWVLE55TZSTLGDAVCNFSM6AAAAABMZNHXE6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMBSGY4TSMJSGU . You are receiving this because you were mentioned.Message ID: @.***>

Shubhf commented 2 weeks ago

@kathlenemagnus As you suggested to add a test vector with a mask "vm" to test to verify "ExtractorDirectInfo" , which will initialise "ExtractDirectInfo" with special fields including vm and validate that the mask is correctly set.

void test_vector_add_with_mask_vm() { ExtractorDirectInfo ex_info(123, {1, 2}, {3}, {{mavis::ExtractorIF::SpecialField::VM, 0}}); auto specials = ex_info.getSpecialFields(); assert(specials.at(mavis::ExtractorIF::SpecialField::VM) == 0); }

I have thought about this function suggest necessary changes