- User Since
- Jul 30 2014, 11:40 AM (155 w, 2 d)
One more typo.
OK, you are probably right that this is not needed. Overall, there are around 200 calls to change. I will just class by class directly to the new interface.
Add missing conversion class.
Yes, this is a total hack. The underlying bugs are:
Can you add -fno-vectorize and try again?
Or even better use test/create_ll.sh?
Thanks for the test case. I have one additional comment. Otherwise, i agree with Siddharth's comments.
LGTM! Very nice!
Thu, Jul 20
Wed, Jul 19
Nice. Is there any reason this is still marked as WIP? Sould I already have a look.
Also mark this as requesting changes to make sure this is not in my review queue any more.
Amazing. Can you add a test case?
- Can you check if the constructors miss some isl cleanups.
- There are more isl print output changes
Tue, Jul 18
Great! Thanks for working on this!
Nice. I will have a look immediately. I am also at ETH tomorrow in case you wanna chat.
Can you maybe document that splitEdge is supposed to only be called on nodes that are part of the dominator tree.
[PPCGCodeGeneration] Generate invariant loads before trying to generate IR.
Seems inconsistent, I get for the following input:
Mon, Jul 17
Add Sebastian as reviewer.
This looks good to me! Thank you.
LGTM. Thank you Sebastian!
That looks great so far.
@Meinersbur: If you find time, would be great to check if the unit tests are still broken under windows.
@tarunr : Do you plan to commit this patch or do you need somebody to commit this patch for you?
Thanks for pushing forward here. One thing I would like to check if the scalars that are private are passed into the kernel or not. AFAIU ppcg is not passing such "private" scalars to the kernel, but Polly-ACC still does this today. Can you check if/what is going on here?
Sun, Jul 16
And one more:
It seems adding an "early" EP is not so easy. In this case, let's just leave it and start testing with DeLICM and co enabled. If they are not yet up-to what we need, we anyhow should report bugs. Feel free to commit this as soon as the prerequsites are in.
Very cool to see this working!
Mark this as "request changes" to move this out of my "to-review" queue.
Instead of adding more content to the Release notes, we can then directly link to the isl-documentation section. By not being part of the release notes, this information will also be preserved for the future.
Sat, Jul 15
This looks good to me
Amazing. I just pushed this out! Good work.
OK. Then we should highlight this in *bold* at an obvious place of our documentation.
OK, can you share (and maybe document) what you misunderstood? Others likely make similar mistakes, so it is good to have this documented and explained.
Right, now the InvalidDomain is indeed consumed. Which is why using get() is not right. Does this make sense? Is there something specific we can or should explain better?
Fri, Jul 14
@singam-sanjay can you have a final look and commit it if you are OK with this patch.
LGTM from my side.
Nice. I mark this as requesting changes, to make sure it pops up again after the test case has been updated!
Yes, I can change this.
Switch to firstname.lastname@example.org
@grosser-test: Another comment, this time with grosser-test's primary email address set to my gmail account.
Added Manuel as reviewer to see if he gets notified.