This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Move Shift Ones instructions from Zbb to Zbp to match 0.93 bitmanip spec.
ClosedPublic

Authored by craig.topper on Jan 13 2021, 10:15 PM.

Details

Summary

It's not really clear in the spec that these are in Zbp now, but
that's what I've gather from previous commits to the spec. I've
file an issue to get it documented properly.

Diff Detail

Event Timeline

craig.topper created this revision.Jan 13 2021, 10:15 PM
craig.topper requested review of this revision.Jan 13 2021, 10:15 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 13 2021, 10:15 PM
Herald added a subscriber: MaskRay. · View Herald Transcript
lenary resigned from this revision.Jan 14 2021, 9:22 AM

Changes look good but, like you, I can't see that these instructions are in that section (or any section for that matter). We can keep an eye on the issue you filed and approve once it's confirmed?

Changes look good but, like you, I can't see that these instructions are in that section (or any section for that matter). We can keep an eye on the issue you filed and approve once it's confirmed?

My hope is to get the 0.93 move into LLVM 12. Zbb is marked frozen in the 0.93 spec and does not include these. So I'd at least like them out of Zbb. Would it be better to just remove them until they have a home?

My hope is to get the 0.93 move into LLVM 12. Zbb is marked frozen in the 0.93 spec and does not include these. So I'd at least like them out of Zbb. Would it be better to just remove them until they have a home?

Mm yes, if Zbb is frozen and doesn't include them then I agree they need to go somewhere. Removing them sounds a bit drastic. If the release branch will be created in 6 days, how long can we delay the decision?

asb added a comment.Jan 21 2021, 7:58 AM

My hope is to get the 0.93 move into LLVM 12. Zbb is marked frozen in the 0.93 spec and does not include these. So I'd at least like them out of Zbb. Would it be better to just remove them until they have a home?

Mm yes, if Zbb is frozen and doesn't include them then I agree they need to go somewhere. Removing them sounds a bit drastic. If the release branch will be created in 6 days, how long can we delay the decision?

We could backport the patch after the branch. The release manager has been very accommodating in the past, but we should do what we can to avoid generating unnecessary work. It's a shame there's no answer on the issue yet https://github.com/riscv/riscv-bitmanip/issues/105

I think making the assumption that they probably meant to move to Zbp should be fine.

frasercrmck accepted this revision.Jan 22 2021, 6:47 AM
In D94652#2512628, @asb wrote:

I think making the assumption that they probably meant to move to Zbp should be fine.

Agreed

This revision is now accepted and ready to land.Jan 22 2021, 6:47 AM
asb accepted this revision.Jan 22 2021, 9:04 AM
This revision was landed with ongoing or failed builds.Jan 22 2021, 12:51 PM
This revision was automatically updated to reflect the committed changes.