- User Since
- Dec 12 2019, 1:37 PM (80 w, 13 h)
Fri, Jun 11
Oh, don't mind me, LGTM!
IMHO, the documentation is fine if the readier gets some hint that they should look at it :P Sth like:
bool BestPossible = !C.hasValue(); bool SecondBest = isa_and_nonnull<UndefValue>(*C);
might do the trick. Like, they will either get it, or they will see that they don't get it and they'll check the doc. But I don't know if you like the style, maybe too verbose.
Thu, Jun 10
LGTM although I've missed a lot of progress in the Attributor so definitely wait for other reviewers if you want an acceptance to move it forward :)
Mon, Jun 7
Oct 29 2020
Remove unnecessary text
Oct 28 2020
That might be too big of a footnote and it might even be more suitable to be added in the phi doc in the LangRef. Nevertheless, it feels complete and valuable now :)
Oct 26 2020
Oct 21 2020
Shortened comments and moved a more detailed explanation in loop terminology/LCSSA doc.
Some resources that might help:
Oct 20 2020
Oct 19 2020
Oct 5 2020
Sep 29 2020
It would be part of the LoopSimplify normal form, ensured by the preheader and dedicated exit blocks. Otherwise, it's more a concept for CFGs than loops.
Btw, do you think it's good to desribe what is a critical edge in this doc ? I think it's mentioned quite often in anything loop-related.
I think the document is more useful as a reference than a text-book reading.
Sep 28 2020
What do you think of the revision? Does it clarify things? Is it more understandable?
Thanks for taking the time to write this! I think it will help, especially beginners. I remember when I first read the loop terminology, some things were not obvious that are cleared here (e.g. why do people use backedge taken count).
Sep 24 2020
Sep 23 2020
Sep 22 2020
Sep 19 2020
@baziotis Are you going to update this diff?
Aug 28 2020
Aug 27 2020
Jul 30 2020
Jul 28 2020
- Added doc.
One proposal by a friend of mine, GeorgeLS, is that we can make a wrapper in which instead of passing a bool, we pass a BitWord.
And then we XOR with this BitWord every BitWord we read from Bits. So, the passed BitWord acts conditionally. If we want the "unset" version, we pass ~0.
If we want the "set" version, we pass 0.
Jul 27 2020
Apart from that, now that I'm seeing it again, in LAA, we probably want to move a lot of vectors out of loops and just reset() them.
Jul 26 2020
Instead of accumulating in AccessInfos, couldn't we just reiterate the alias set (if we didn't bail out)?
Jul 17 2020
Jul 14 2020
The code is quite good and clean. Most comments are on the doc so that we can make it good and clean too and move to the full patch.
Jul 13 2020
A nit comment is to please start comments with caps.
Jul 8 2020
Jul 7 2020
Nit comments, I haven't yet digested the whole patch.
First of all, great patch. Reading the description and the referenced diff, I can't find an explanation about what this patch tries to do, it would be good if you added that. My assumption is:
Jul 3 2020
I partially agree but also IMHO empty() is confusing enough so that introducing isInnermost/Outermost has benefit that outweighs the cost. A Loop can contain both BBs and loops which is different than DominatorTree AFAIK (which can only contain BBs).
This is a big source of confusion when one comes across it in the code (and as I said, personally I assumed it had to do with the BBs).
Jul 2 2020
Thanks for the reviews, I'm updating.
Jun 30 2020
I created the review instead of just committing to discuss that it may be a good idea to even rename this e.g. to "isInnermost()".
With "empty()", it's not clear if the implementation is:
return getSubLoops().empty() or return !getNumBlocks() (or something else)
May 28 2020
May 26 2020
May 25 2020
Ping :) Should I commit?
May 15 2020
Thanks for the comments!
Apr 11 2020
Apr 4 2020
Differential Review: https://reviews.llvm.org/D71960/
- Final update
Committed but of course I forgot again to add the differential revision. I don't why I continuously do that, sorry. :)
I'll add it to the commit in Phabricator and close this when the commit is ready.
Apr 2 2020
Mar 31 2020
No, problem, thanks for the review and the info!
Mar 30 2020
- Addressed SCEV behavior in loop-variant expressions.
Mar 29 2020
- Various typos.
- Added def-use chain info as a footnote since its used twice. Also a footnote about LCSSA construction (I consider it interesting but in a footnote because it may be irrelevant in the main text).
- Added some info about SCEV.
Personally I don't have problem with big files, sometimes they're more easy to use. But I definitely agree that we should start thinking about structure and splitting into files can help.