- User Since
- Apr 21 2016, 3:27 AM (259 w, 4 d)
Tue, Mar 23
Mon, Mar 22
I think the issue might be here: https://github.com/llvm/llvm-project/blob/7515e81e8c58ca07ac5fede7149634c0dfacae8a/flang/CMakeLists.txt#L52
We seem to be using CLANG_DIR directly, but for LLVM_DIR and MLIR_DIR we build absolute paths if needed.
@awarzynski, does this sound correct to you?
Thu, Mar 18
This seems fine to me, but maybe @awarzynski can also have a look, since he knows the details of the new driver build better :)
Wed, Mar 17
Mar 5 2021
Feb 9 2021
LGTM as far as GlobalISel is concerned.
Feb 5 2021
Seems fine from a GlobalISel perspective, and I'm guessing the stack clashing details will be reviewed in the SelectionDAG patch which adds the test.
Can you please upload with context? :) Thanks!
Jan 28 2021
Jan 26 2021
Jan 25 2021
Jan 18 2021
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).