- User Since
- Apr 21 2016, 3:27 AM (248 w, 19 h)
Mon, Jan 18
Dec 4 2020
Sorry, I had one more comment :) I think that should be all from my side.
Dec 3 2020
Thanks for looking into this, Oliver! This seems sensible to me, but I think @delcypher should probably have the final say.
Nov 30 2020
This seems to break several buildbots that don't build the x86 backend, e.g. http://lab.llvm.org:8011/#/builders/135/builds/90.
Could you please take a look?
Nov 23 2020
Nov 19 2020
Oct 6 2020
Sep 28 2020
Sep 24 2020
Hi Galina, those are all good comments, thanks! I think I fixed them up, does it look ok now?
Sep 21 2020
Sep 14 2020
Hi Galina, thanks for having a look!
Sep 9 2020
Great, thanks! :)
Sep 7 2020
Sep 4 2020
Thanks for trying this out!
Sep 3 2020
Sep 2 2020
Sep 1 2020
Aug 31 2020
Jul 16 2020
Thanks for the update. This looks fine to me as is, but I'll defer the final LGTM to someone that knows this area a bit better.
Jul 10 2020
Jul 8 2020
Don't you also have to set as Available/Unavailable when initializing the TLI?
Jun 25 2020
Jun 15 2020
@calixte: Does this affect the intention of the test in any way?
Um, I tried to upload a diff here but it didn't work, so I just created a new revision: https://reviews.llvm.org/D81830
Hope that's ok (if not feel free to re-upload here).
Hi! This doesn't seem to be enough. I suspect this is because the exception is thrown from one of the other threads rather than the main thread. Maybe f also needs a try-catch block? I'm going to give it a few runs and report back. EDIT: Actually I think I'll move the try-catch closer to the root of the problem, in launcher. I'll post if that worked.
Jun 11 2020
Jun 4 2020
Hi! I think this commit is causing some instability on the 10.0.1 branch on 32-bit arm: https://bugs.llvm.org/show_bug.cgi?id=46092
Can you have a look?
Jun 2 2020
May 4 2020
Hi, I like that we're looping in only one place now, but I have 2 nitpicks:
- The formatting changes are pretty distracting, I think you should commit them separately.
- The commit message should explain what the problem is and why this change fixes it. Just referencing a bug number, where there haven't even been any discussions, is not very helpful.
Jan 29 2020
Jan 28 2020
Just a few nits.
Dec 3 2019
Nov 19 2019
Nov 18 2019
Nov 15 2019
I don't see anything else wrong with this. LGTM if you rename the LE predicate.
Addressed @labath's comment (test run in progress). Thanks for having a look!
Nov 14 2019
Nov 13 2019
Regarding the change to return const, I'm not convinced that's a good idea (we actually have a clang-tidy check that warns about that). I think it would be better to either name those temporaries or use std::make_tuple instead of std::tie (whichever you prefer).
Nov 12 2019
Update test checks. Also brush it up a bit so it fits better with the rest of the tests in the file.
Nov 11 2019
Nov 5 2019
Nov 4 2019
The text itself LGTM.
Oct 31 2019
Just a few more typos and stuff.
Oct 29 2019
Thanks, looks great!
Oct 28 2019
Thanks for writing this up! I just have a few suggestions and one small bug to point out (I can't figure out how to comment on a png file, so I'll write it here).
Oct 18 2019
Thanks, LGTM then!
Oct 17 2019
Could you be a little more specific? I just ran a build with BUILD_SHARED_LIBS=ON and it seemed to work.
Oct 2 2019
This looks good to me, maybe wait a while to see if anyone else has any further comments.
Oct 1 2019
Sep 25 2019
I'm not very familiar with this area, but it seems like a sensible fix.
Sep 24 2019
I suspect 'ScalableSize' is the wrong term now; 'TypeSize' may be better. Thoughts?
Sep 16 2019
I think all the outstanding comments have been addressed. LGTM.
Sep 4 2019
Sep 2 2019
Just some drive-by suggestions :)
Aug 30 2019
Aug 1 2019
Several more general comments:
- Should all the getXSize assert when called on a scalable type? I see that for MVT::getSizeInBits and for Type::getPrimitiveSize, but not for the others. This should also be made clear in the comments for each of them.
- Great test for the IR, thanks!
- I don't see any test for the CodeGen stuff though. Is it possible to add one? (If not, maybe add the changes to EVT etc when we can actually test them).
- Ditto for TableGen (or if that's too difficult/hairy to test, just update the commit message to explain exactly why the change belongs in this patch).
Jul 31 2019
Jul 30 2019
Some people might be interested just in compiling LLVM (e.g. fetching some release sources from releases.llvm.org and building them). We should probably keep the package list here focused on that. If we want to add git at all, we should probably mention it somewhere else. I'm not sure where that would be - I suppose somewhere where it makes sense to worry about the minimum version, e.g. where the git-llvm script is concerned. Do we have any other reason to bind ourselves to a certain minimum version of git?
Jul 26 2019
Jul 23 2019
Jul 22 2019
Jul 19 2019
FWIW, I think the tests look great. Would be nice if someone more experienced with clang could also have a look though.