- User Since
- Dec 14 2019, 8:54 PM (40 w, 21 h)
Fri, Sep 18
It's a bit odd to see the loop ordering of a *lowering conversion* change because the output improves performance with a specific backend/compiler and for specific reasons. Moreover, that way, this would keep changing/evolving and be sensitive to common downstream paths and sensitive to LLVM's opt pipeline. In the absence of any target / scheduling info, the order to choose is typically expected to be just the most intuitive / canonical and shouldn't keep changing. You need an optimization mechanism if you need a better one.
Thu, Sep 17
@ftynse comments from the previous round sound good to me too. Looks good to commit from my side once those are addressed.
Looks good, thanks!
Wed, Sep 16
Please add a test case that exercises this. Thanks. It's not fully clear one could just ignore all uses that aren't CallOps. Please also add a comment.
Tue, Sep 15
constructTiledSetHyper... appears to have been moved but not mentioned in the commit summary as an NFC move. Has it been updated in some way?
Trigger harbormaster build check.
These build failures appear unrelated.
Mon, Sep 14
Thanks for fixing the issues.
Suggestions for tweaking the information printed welcome. This change does provide more information than just "<block argument>" but still not enough (as we aren't identifying its Block in any way).
I think we definitely need a test for the builder API here to make sure it's working as intended. Please see the other builder API tests for reference in test/EDSC/builder-api-test.cpp.
Sun, Sep 13
Great to see this. Some minor comments.
Thu, Sep 10
I find the commit summary and the title a bit confusing. Isn't this extending standard dialect cast ops to work with vectors of those types? It's not that vector types are being cast to int/fp.
Wed, Sep 9
Please mark the commit 'NFC' (in the title). Please also include the previous commit title in the commit summary and a part of it in the current title.
I chatted with Nicolas about this earlier today, and he was okay with this idea.
Thanks for contributing this! Please state at the end of the commit summary as to which methods were just moved as a result of this revision (since there appear to be methods that were purely moved to allow reuse with constant tile size API).
Tue, Sep 8
Thanks for adding this option.
Sat, Sep 5
Fri, Sep 4
Nit: commit summary: to -> due to / during the
Thu, Sep 3
Thanks very much for introducing this. Some initial superficial comments.
Wed, Sep 2
Great to see the region branch op interface being used for this. Some minor comments.
Tue, Sep 1
I'd reword the commit summary a bit to replace "Introduce" - instead, something like "Make use of ..."
Sun, Aug 30
Sat, Aug 29
Please fix the 80 column spillovers. Although this "Returns if ..." construction is used pervasively and isn't actually incorrect, it can be a bit confusing. "if this ..." was being used as a noun for the bool value.
Thu, Aug 27
I've committed this after taking care of the typo, a whitespace error, and typos in the commit title/summary.
Wed, Aug 26
Tue, Aug 25
@AlexEichenberger Let me know if you are able to commit or if you'd like me to commit this for you.
Some more minor comments - mostly on documentation and comments. Can you please fix?