-
Notifications
You must be signed in to change notification settings - Fork 13.5k
[InstCombine] Add contributor guide #79007
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
Conversation
706f745
to
fdfede6
Compare
fdfede6
to
a627fac
Compare
* `m_OneUse(M)`: Check that the value only has one use, and also matches M. | ||
For example `m_OneUse(m_Add(...))`. See the next section for more | ||
information. | ||
|
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.
TODO: m_ElementwiseBitCast
. It may confuse contributors when folding bitcast-related patterns.
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 one seems pretty exotic to me, compared to the other ones.
@AtariDreams @elhewaty @ParkHanbum @SahilPatidar |
we added this:
but I sill find the look through |
what about adding when to use |
One more thing that might be nice is an example or so of successful PRs new contributors can look at to get a sense of how all these rules play out. Either way though, I think at this point the document is clearly useful and would be happy with it going up in its current form. We can always add more later. |
Done. |
Any more feedback on this? |
Make tests minimal. Only test exactly the pattern being transformed. If your | ||
original motivating case is a larger pattern that your fold enables to | ||
optimize in some non-trivial way, you may add it as well -- however, the bulk | ||
of the test coverage should be minimal. |
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.
TODO: Drop unused poison-generating flags/UB-implying attributes.
One-use checks can be performed using the `m_OneUse()` matcher, or the | ||
`V->hasOneUse()` method. | ||
|
||
### Generalization |
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.
TODO: Be aware of poison-generating flags/FMF propagation.
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.
Example: #72127
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'll go ahead and merge this so we can iterate further in-tree. |
thanks for work to this!! I was very lost when I first contributed to LLVM and this will help me a lot. Q. when I can see this document from web site llvm.org? |
Document expectations for contributions to InstCombine, especially regarding test coverage and alive2 proofs.
Document expectations for contributions to InstCombine, especially regarding test coverage and alive2 proofs.