Add integer exponent code sample to JitBuilder #6652
Conversation
sudip-unb
requested review from
mstoodle,
charliegracie and
Leonardo2718
as code owners
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thank you for supporting the project, and congratulations on your first contribution! A project committer will shortly review your contribution. In the mean time, if you haven't had a chance please skim over the contribution guidelines which all pull requests must adhere to. If the ECA pull request check fails, have a look at the instructions for signing the ECA in the legal considerations section.
If you run into any problems our community will be happy to assist you in any way we can. There are a number of recommended ways to interact with the community. We encourage you to ask questions, or drop by to say hello.
sudip-unb
force-pushed
the
jitbuilder-samplecode
branch
2 times, most recently
from
afd9f9e
to
6d23609
Compare
There was a problem hiding this comment.
Thank you for your contribution, Sudip, it's great to see more code samples being added for JitBuilder!
Please put your commit title into the active voice, e.g. "Add JitBuilder power function sample:"
There are two sign-offs in your commit (you only need one), and please fix the typo in your commit summary: "jirbuilder".
I think your commit summary could include a bit more information about how the sample works, i.e. "calculates integer powers by iterating over the exponent" or something like that.
| OMR::JitBuilder::TypeDictionary types; | ||
|
|
||
| printf("Step 3: compile method builder\n"); | ||
| PowerMethod PowerMethod(&types); |
There was a problem hiding this comment.
shouldn't the variable have a different name than the class? i.e. PowerMethod powerMethod(&types);
sudip-unb
force-pushed
the
jitbuilder-samplecode
branch
4 times, most recently
from
f894443
to
49c9cb8
Compare
sudip-unb
requested review from
fjeremic,
0xdaryl and
vijaysun-omr
as code owners
sudip-unb
force-pushed
the
jitbuilder-samplecode
branch
from
49c9cb8
to
98c9c1b
Compare
sudip-unb
changed the title
|
Hi @sudip-unb, the code looks good now. Would you please add a bit more information to the commit message on what the code does and (high level) how it computes the result as I suggested earlier? As an example, please see this commit: 3112c66 . |
|
Hi @mstoodle, |
|
Hi @sudip-unb I see the updated description in the PR, but the commit message still looks like: i.e. it has only a subject and is otherwise empty... |
sudip-unb
force-pushed
the
jitbuilder-samplecode
branch
2 times, most recently
from
a17680c
to
f11c27c
Compare
|
@mstoodle I have updated the commit message. |
sudip-unb
requested review from
mstoodle
and removed request for
Leonardo2718,
charliegracie,
fjeremic,
0xdaryl and
vijaysun-omr
sudip-unb
changed the title
|
Just so it doesn't look like this one has been left idling (should have documented this back in mid September): I reached out to @sudip-unb directly to explain what it is I'm looking for in the commit message. Once that change is made, I think this PR is good to go. |
sudip-unb
force-pushed
the
jitbuilder-samplecode
branch
from
f11c27c
to
c014458
Compare
|
Sorry, I had missed the commit message change that was pushed 4 weeks ago :( . This PR look good to me now. I've asked @sudip-unb to rebase onto the latest, at which point I'll run more tests and merge it (assuming they pass :) ). |
sudip-unb
force-pushed
the
jitbuilder-samplecode
branch
from
c014458
to
e826323
Compare
|
jenkins build all |
|
@mstoodle rebase is done and all the checks are fine, however, the build shows some issues. |
Provide a new JitBuilder sample code that demonstrates how to compute power(a,n) using the DoWhileLoop construct to multiply a by itself n times (n is an integer). Signed-off-by: Sudip Chatterjee <sudip.chatterjee@unb.ca>
sudip-unb
force-pushed
the
jitbuilder-samplecode
branch
from
e826323
to
5efc4b4
Compare
|
jenkins build all |
|
The failure looks like #6571 which has been failing for a while now ( :( ). I'm approving and will merge. Thanks @sudip-unb ! |
Provide a new JitBuilder sample code that demonstrates how to compute power(a,n) using the DoWhileLoop construct to multiply a by itself n times (n is an integer). This code sample will be included in the set of default code samples that runs when OMR_JITBUILDERTEST is enabled in cmake builds.