-
Notifications
You must be signed in to change notification settings - Fork 42
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
Conversation
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 { |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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
There was a problem hiding this 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...
…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.
There was a problem hiding this 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.
Clean builds of cheribsd-riscv64-purecap have cheribsdtest-* all passing on CHERI-QEMU and CHERI-PURECAP-QEMU kernels. |
FYI llvm/llvm-project#81688 for upstreaming the intersect redesign. |
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.