intel / isa-l

Intelligent Storage Acceleration Library
Other
946 stars 300 forks source link

Add macos-14 (arm64) CI (and made it pass finally) #278

Closed cielavenir closed 6 months ago

cielavenir commented 7 months ago

CI passed according to https://github.com/cielavenir/isa-l/actions/runs/8114714856

cielavenir commented 7 months ago

Extended tests passed as well https://github.com/cielavenir/isa-l/actions/runs/8130749368/job/22219331109

pablodelara commented 7 months ago

HI @cielavenir. Thanks for your PR! Can you clean it up a little bit? There are a bunch of "merge" and "revert" patches that are polluting the PR. Also, since you have integrated the fix in PR #226, can you close that PR?

cielavenir commented 7 months ago

@pablodelara Squashed

cielavenir commented 7 months ago

by the way, not sure I'm noob at github, but how can I run CI without patching on: field? I'm asking because this caused pushing back and forth.

pablodelara commented 7 months ago

Thanks @cielavenir! What I meant is to keep only the commits that are adding value overall, and remove the "merge" commits. Looking at the PR, you could have 4-5 commits, each one adding/fixing some functionality. E.g.:

cielavenir commented 7 months ago

Actually is it possible to merge #226 and #280 then rebase this pull request again?

pablodelara commented 7 months ago

@cielavenir Well, those PR's are closed. We could reopen them, but still this PR should be split into several commits.

cielavenir commented 7 months ago

right please reopen and merge them

cielavenir commented 7 months ago

@pablodelara @liuqinfei Now rebased again. The thing is, #226 and #280 are obvious but avoiding x18 register should be reviewed carefully as I rely only on CI result.

pablodelara commented 6 months ago

@liuqinfei can you review this PR?

liuqinfei commented 6 months ago

yes.

---- Replied Message ---- | From | Pablo de @.> | | Date | 03/14/2024 21:19 | | To | @.> | | Cc | @.>@.> | | Subject | Re: [intel/isa-l] Add macos-14 (arm64) CI (and made it pass finally) (PR #278) |

@liuqinfei can you review this PR?

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

liuqinfei commented 6 months ago

I think the change is ok for Aarch64 platform. And, i verified this PR on kunpeng 920 (Aarch64), all the test cases pass as following.

Snipaste_2024-03-14_23-35-15
cielavenir commented 6 months ago

rebased again.

now let's see if macOS M1 extended tests pass.

pablodelara commented 6 months ago

This is merged now, thanks!