Skip to content
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

Jump to bottom

WIP:When LoadStore a uint32, return value should be uint32 #222

Closed
wants to merge 1 commit into from
Closed

WIP:When LoadStore a uint32, return value should be uint32 #222

wants to merge 1 commit into from

Conversation

luyahan
Copy link
Collaborator

@luyahan luyahan commented Sep 7, 2020 •
edited

fix #215

@penguinwu penguinwu self-requested a review September 8, 2020 13:45
penguinwu
penguinwu suggested changes Sep 8, 2020
View reviewed changes
Copy link

@penguinwu penguinwu left a comment

Choose a reason for hiding this comment

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

LGTM.

But there are a few other GenAndRunTest functions that also have a similar problem of potentially returning int32/64 for uint32/64 input types. Search for string int32_t, int64_t. Please separate the floating-point version from the integer version and fix those too.

@penguinwu
Copy link

@luyahan Let's put this PR on hold (no need to address my change request). I am finishing up a pretty extensive refactoring PR for test-assembler/macro-assembler-riscv64.cc that touches the same GenAndRunTest functions that you modified here. I've incorporated the fixes (without duplicating GenAndRunTest functions for floating-point) for all similar cases in my PR. Let me merge my PR first. If the problem still persists, you can apply your fix on top of mine.

@luyahan
Copy link
Collaborator Author

luyahan commented Sep 9, 2020

@luyahan Let's put this PR on hold (no need to address my change request). I am finishing up a pretty extensive refactoring PR for test-assembler/macro-assembler-riscv64.cc that touches the same GenAndRunTest functions that you modified here. I've incorporated the fixes (without duplicating GenAndRunTest functions for floating-point) for all similar cases in my PR. Let me merge my PR first. If the problem still persists, you can apply your fix on top of mine.

That's the best !

@luyahan luyahan changed the title When LoadStore a uint32, return value should be uint32. WIP:When LoadStore a uint32, return value should be uint32. Sep 9, 2020
penguinwu pushed a commit that referenced this pull request Sep 9, 2020
Refactor test-macro-assembler-riscv64 and test-assembler-riscv64:
64aaa24 
- Share GenAndRunTest across test-macro-assembler and test-assembler tests
  so that redundant Run_xxx helper functions are removed
- Incorporated the fix in #222 w/o duplicating GenAndRunTest
- Use FOR_INT64_INPUTS defined in value-helper.h instead of defining our
  own set of corner-case values
- Cleaned-up SetParam/GetParam and use bit_cast<T> to convert between
  int and floating-point types
- Use AssembleCode to remove some boilerplate codes to build CodeObject
- Remove redundant nop (legacy of MIPS delay slot)
@penguinwu penguinwu mentioned this pull request Sep 9, 2020
Merged
ghost pushed a commit that referenced this pull request Sep 9, 2020
@obycode
Refactor test-macro-assembler-riscv64 and test-assembler-riscv64:
f85518f 
- Share GenAndRunTest across test-macro-assembler and test-assembler tests
  so that redundant Run_xxx helper functions are removed
- Incorporated the fix in #222 w/o duplicating GenAndRunTest
- Use FOR_INT64_INPUTS defined in value-helper.h instead of defining our
  own set of corner-case values
- Cleaned-up SetParam/GetParam and use bit_cast<T> to convert between
  int and floating-point types
- Use AssembleCode to remove some boilerplate codes to build CodeObject
- Remove redundant nop (legacy of MIPS delay slot)
@penguinwu
Copy link

@luyahan My PR is merged. Please try the latest from riscv64 to see if it fixes the problem you reported. If so, please close the corresponding issue.

@ghost ghost changed the title WIP:When LoadStore a uint32, return value should be uint32. WIP:When LoadStore a uint32, return value should be uint32 Sep 15, 2020
@luyahan luyahan closed this Sep 16, 2020
ghost pushed a commit that referenced this pull request Oct 15, 2020
@obycode
Refactor test-macro-assembler-riscv64 and test-assembler-riscv64:
d1717c7 
- Share GenAndRunTest across test-macro-assembler and test-assembler tests
  so that redundant Run_xxx helper functions are removed
- Incorporated the fix in #222 w/o duplicating GenAndRunTest
- Use FOR_INT64_INPUTS defined in value-helper.h instead of defining our
  own set of corner-case values
- Cleaned-up SetParam/GetParam and use bit_cast<T> to convert between
  int and floating-point types
- Use AssembleCode to remove some boilerplate codes to build CodeObject
- Remove redundant nop (legacy of MIPS delay slot)
ghost pushed a commit that referenced this pull request Nov 11, 2020
@obycode
Refactor test-macro-assembler-riscv64 and test-assembler-riscv64:
c26c96e 
- Share GenAndRunTest across test-macro-assembler and test-assembler tests
  so that redundant Run_xxx helper functions are removed
- Incorporated the fix in #222 w/o duplicating GenAndRunTest
- Use FOR_INT64_INPUTS defined in value-helper.h instead of defining our
  own set of corner-case values
- Cleaned-up SetParam/GetParam and use bit_cast<T> to convert between
  int and floating-point types
- Use AssembleCode to remove some boilerplate codes to build CodeObject
- Remove redundant nop (legacy of MIPS delay slot)
luyahan pushed a commit to luyahan/v8 that referenced this pull request Nov 30, 2020
@luyahan
Refactor test-macro-assembler-riscv64 and test-assembler-riscv64:
a0e338d 
- Share GenAndRunTest across test-macro-assembler and test-assembler tests
  so that redundant Run_xxx helper functions are removed
- Incorporated the fix in riscv-collab#222 w/o duplicating GenAndRunTest
- Use FOR_INT64_INPUTS defined in value-helper.h instead of defining our
  own set of corner-case values
- Cleaned-up SetParam/GetParam and use bit_cast<T> to convert between
  int and floating-point types
- Use AssembleCode to remove some boilerplate codes to build CodeObject
- Remove redundant nop (legacy of MIPS delay slot)
ghost pushed a commit that referenced this pull request Dec 3, 2020
@obycode
Refactor test-macro-assembler-riscv64 and test-assembler-riscv64:
cc330c8 
- Share GenAndRunTest across test-macro-assembler and test-assembler tests
  so that redundant Run_xxx helper functions are removed
- Incorporated the fix in #222 w/o duplicating GenAndRunTest
- Use FOR_INT64_INPUTS defined in value-helper.h instead of defining our
  own set of corner-case values
- Cleaned-up SetParam/GetParam and use bit_cast<T> to convert between
  int and floating-point types
- Use AssembleCode to remove some boilerplate codes to build CodeObject
- Remove redundant nop (legacy of MIPS delay slot)
ghost pushed a commit that referenced this pull request Dec 9, 2020
@obycode
Refactor test-macro-assembler-riscv64 and test-assembler-riscv64:
4007132 
- Share GenAndRunTest across test-macro-assembler and test-assembler tests
  so that redundant Run_xxx helper functions are removed
- Incorporated the fix in #222 w/o duplicating GenAndRunTest
- Use FOR_INT64_INPUTS defined in value-helper.h instead of defining our
  own set of corner-case values
- Cleaned-up SetParam/GetParam and use bit_cast<T> to convert between
  int and floating-point types
- Use AssembleCode to remove some boilerplate codes to build CodeObject
- Remove redundant nop (legacy of MIPS delay slot)
ghost pushed a commit that referenced this pull request Jan 13, 2021
@obycode
Refactor test[-macro]-assembler-riscv64
1a3e5e4 
- Share GenAndRunTest across test-macro-assembler and test-assembler
  tests so that redundant Run_xxx helper functions are removed
- Incorporated the fix in #222 w/o duplicating GenAndRunTest
- Use FOR_INT64_INPUTS defined in value-helper.h instead of defining our
  own set of corner-case values
- Cleaned-up SetParam/GetParam and use bit_cast<T> to convert between
  int and floating-point types
- Use AssembleCode to remove some boilerplate codes to build CodeObject
- Remove redundant nop (legacy of MIPS delay slot)
ghost pushed a commit that referenced this pull request Jan 26, 2021
@obycode
Refactor test[-macro]-assembler-riscv64
48e0069 
- Share GenAndRunTest across test-macro-assembler and test-assembler
  tests so that redundant Run_xxx helper functions are removed
- Incorporated the fix in #222 w/o duplicating GenAndRunTest
- Use FOR_INT64_INPUTS defined in value-helper.h instead of defining our
  own set of corner-case values
- Cleaned-up SetParam/GetParam and use bit_cast<T> to convert between
  int and floating-point types
- Use AssembleCode to remove some boilerplate codes to build CodeObject
- Remove redundant nop (legacy of MIPS delay slot)
luyahan pushed a commit to luyahan/v8 that referenced this pull request Jan 27, 2021
@luyahan
Refactor test[-macro]-assembler-riscv64
79ab938 
- Share GenAndRunTest across test-macro-assembler and test-assembler
  tests so that redundant Run_xxx helper functions are removed
- Incorporated the fix in riscv-collab#222 w/o duplicating GenAndRunTest
- Use FOR_INT64_INPUTS defined in value-helper.h instead of defining our
  own set of corner-case values
- Cleaned-up SetParam/GetParam and use bit_cast<T> to convert between
  int and floating-point types
- Use AssembleCode to remove some boilerplate codes to build CodeObject
- Remove redundant nop (legacy of MIPS delay slot)
luyahan pushed a commit to luyahan/v8 that referenced this pull request Feb 1, 2021
@luyahan
Refactor test[-macro]-assembler-riscv64
32802e7 
- Share GenAndRunTest across test-macro-assembler and test-assembler
  tests so that redundant Run_xxx helper functions are removed
- Incorporated the fix in riscv-collab#222 w/o duplicating GenAndRunTest
- Use FOR_INT64_INPUTS defined in value-helper.h instead of defining our
  own set of corner-case values
- Cleaned-up SetParam/GetParam and use bit_cast<T> to convert between
  int and floating-point types
- Use AssembleCode to remove some boilerplate codes to build CodeObject
- Remove redundant nop (legacy of MIPS delay slot)
ghost pushed a commit that referenced this pull request Feb 1, 2021
@obycode
Refactor test[-macro]-assembler-riscv64
bbdbb4b 
- Share GenAndRunTest across test-macro-assembler and test-assembler
  tests so that redundant Run_xxx helper functions are removed
- Incorporated the fix in #222 w/o duplicating GenAndRunTest
- Use FOR_INT64_INPUTS defined in value-helper.h instead of defining our
  own set of corner-case values
- Cleaned-up SetParam/GetParam and use bit_cast<T> to convert between
  int and floating-point types
- Use AssembleCode to remove some boilerplate codes to build CodeObject
- Remove redundant nop (legacy of MIPS delay slot)
ghost pushed a commit that referenced this pull request Feb 3, 2021
@obycode
Refactor test[-macro]-assembler-riscv64
572b17c 
- Share GenAndRunTest across test-macro-assembler and test-assembler
  tests so that redundant Run_xxx helper functions are removed
- Incorporated the fix in #222 w/o duplicating GenAndRunTest
- Use FOR_INT64_INPUTS defined in value-helper.h instead of defining our
  own set of corner-case values
- Cleaned-up SetParam/GetParam and use bit_cast<T> to convert between
  int and floating-point types
- Use AssembleCode to remove some boilerplate codes to build CodeObject
- Remove redundant nop (legacy of MIPS delay slot)
luyahan pushed a commit to luyahan/v8 that referenced this pull request Feb 7, 2021
@luyahan
Refactor test[-macro]-assembler-riscv64
1bea40e 
- Share GenAndRunTest across test-macro-assembler and test-assembler
  tests so that redundant Run_xxx helper functions are removed
- Incorporated the fix in riscv-collab#222 w/o duplicating GenAndRunTest
- Use FOR_INT64_INPUTS defined in value-helper.h instead of defining our
  own set of corner-case values
- Cleaned-up SetParam/GetParam and use bit_cast<T> to convert between
  int and floating-point types
- Use AssembleCode to remove some boilerplate codes to build CodeObject
- Remove redundant nop (legacy of MIPS delay slot)
luyahan pushed a commit to luyahan/v8 that referenced this pull request Feb 9, 2021
@luyahan
Refactor test[-macro]-assembler-riscv64
729afc3 
- Share GenAndRunTest across test-macro-assembler and test-assembler
  tests so that redundant Run_xxx helper functions are removed
- Incorporated the fix in riscv-collab#222 w/o duplicating GenAndRunTest
- Use FOR_INT64_INPUTS defined in value-helper.h instead of defining our
  own set of corner-case values
- Cleaned-up SetParam/GetParam and use bit_cast<T> to convert between
  int and floating-point types
- Use AssembleCode to remove some boilerplate codes to build CodeObject
- Remove redundant nop (legacy of MIPS delay slot)
ghost pushed a commit that referenced this pull request Feb 9, 2021
@obycode
Refactor test[-macro]-assembler-riscv64
e60f023 
- Share GenAndRunTest across test-macro-assembler and test-assembler
  tests so that redundant Run_xxx helper functions are removed
- Incorporated the fix in #222 w/o duplicating GenAndRunTest
- Use FOR_INT64_INPUTS defined in value-helper.h instead of defining our
  own set of corner-case values
- Cleaned-up SetParam/GetParam and use bit_cast<T> to convert between
  int and floating-point types
- Use AssembleCode to remove some boilerplate codes to build CodeObject
- Remove redundant nop (legacy of MIPS delay slot)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lazyparser lazyparser lazyparser left review comments

@penguinwu penguinwu penguinwu requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[QEMU] === cctest/test-assembler-riscv64/RISCV_UTEST_swlwu === failed
3 participants
@luyahan @penguinwu @lazyparser