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

[CodeGen] Rework MVT representation of capabilities and add type inference #726

Merged
merged 1 commit into from
Feb 14, 2024

Conversation

jrtc27
Copy link
Member

@jrtc27 jrtc27 commented Feb 10, 2024

This more closely aligns how capabilites are handled with iPTR and iN
types, introducing a new cPTR wildcard and fixed-width cN types that it
maps to, with iPTRAny being iPTR and cPTR. As a result we no longer need
to duplicate patterns for each fixed-width type in certain cases, and
the few upstream MIPS tests we've long marked as XFAIL'ed now pass.

As part of this, a few of the APIs around MVTs have been renamed from
fat pointer terminology to use capability instead. Where relatively
straightforward to do these have then been adopted throughout, but some
are pervasive and have deprecated aliases under the old names, whose
uses should be incrementally reduced.

@jrtc27
Copy link
Member Author

jrtc27 commented Feb 10, 2024

NB: I have not yet tested CheriBSD with this.

// Complement of In.
auto CompIn = [&In](MVT T) -> bool { return !In.count(T); };
namespace {
enum class TypeCat {
Copy link
Member Author

Choose a reason for hiding this comment

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

I will likely rewrite this to be less overkill with the templates. At one point I thought it was going to be a lot more complicated, but as it turns out the function really just needs an MVT and an MVT->bool.

ArrayRef<ValueTypeByHwMode> PtrTys({ValueTypeByHwMode(MVT::iFATPTRAny),
ValueTypeByHwMode(MVT::iPTR)});
return NodeToApply->UpdateNodeType(ResNo, PtrTys, TP);
// TODO: Make this less ugly
Copy link
Member Author

Choose a reason for hiding this comment

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

I intend to fix this before it gets merged

Copy link
Member

@arichardson arichardson left a comment

Choose a reason for hiding this comment

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

This looks like a huge improvement. I have never looked at those tablegen internals, so if it works that sounds good to me.

One or two of the clang format suggestions are legitimate since the shorter name now makes the line fit but most of them are quite odd suggestions...

llvm/lib/Target/Mips/MipsInstrCheri.td Show resolved Hide resolved
llvm/lib/Target/Mips/MipsInstrCheri.td Show resolved Hide resolved
…rence

This more closely aligns how capabilites are handled with iPTR and iN
types, introducing a new cPTR wildcard and fixed-width cN types that it
maps to, with iPTRAny being iPTR and cPTR. As a result we no longer need
to duplicate patterns for each fixed-width type in certain cases, and
the few upstream MIPS tests we've long marked as XFAIL'ed now pass.

As part of this, a few of the APIs around MVTs have been renamed from
fat pointer terminology to use capability instead. Where relatively
straightforward to do these have then been adopted throughout, but some
are pervasive and have deprecated aliases under the old names, whose
uses should be incrementally reduced.
Copy link
Member

@arichardson arichardson left a comment

Choose a reason for hiding this comment

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

If cheribsd still builds we should go ahead and merge this. I guess ideally we'd have CI do it but will be annoying to add.

@jrtc27
Copy link
Member Author

jrtc27 commented Feb 14, 2024

Clean builds of cheribsd-riscv64-purecap have cheribsdtest-* all passing on CHERI-QEMU and CHERI-PURECAP-QEMU kernels.

@jrtc27
Copy link
Member Author

jrtc27 commented Feb 14, 2024

FYI llvm/llvm-project#81688 for upstreaming the intersect redesign.

@jrtc27 jrtc27 merged commit 7aa7f2e into dev Feb 14, 2024
5 of 6 checks passed
@jrtc27 jrtc27 deleted the cap-mvts branch February 14, 2024 06:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants