- User Since
- Oct 10 2019, 2:40 AM (172 w, 3 d)
May 11 2022
May 9 2022
The patch has been submitted. Somehow this revision wasn't closed automatically
May 6 2022
I've refactored the switch statement as proposed.
May 5 2022
I've changed the name for the unit test
May 4 2022
I've changed the code to only explicitely check for splats of 1, rather
than all forms of all active predicates.
I also added the requested negative unit test. Note that this doesn't
actually guard the code that we just changed, because the to.bool is
lowered before the from.bool, so the from.bool has not yet been lowered
to reinterpret cast at this point. I kept the unit test anyway,
because this is a valid case to be tested.
Just adding [SVE] to the commit message
Apr 14 2022
The CHECK lines are now autogenerated.
@ABataev , I had a look at the various patches you pointed. That looks like a proper piece of work! Given how much change is going to happen, i don't think it is worth pushing the fix i am proposing today.
Further reduction of the unit test
Apr 12 2022
@vporpo I was just starting to look at removing the loop-related code. That's a good point, i keep thinking about SLP in the context of loops, but it shouldn't be just about loops. The various costs seem completely different, it might require some extra work to reproduce the exact case without the loop control code.
I have simplified the unit tests.
As far as this patch goes, both tests were checking the same
behaviour, so I have only kept one. The complex data structures
and unnecesary attributes have been removed, and the IR has been reordered
to make the patterns clearer. Let me know if further reduction
Apr 11 2022
Jan 25 2022
Adding test cases for float to integer bitcasts
Jan 24 2022
Oct 5 2021
Sorry for the delay, i had a couple of concurrent tasks.
Sep 29 2021
The issue was detected on Linux, AArch64.
Sep 8 2021
Thanks Malhar. The patch looks good to me
Just one small comment. Otherwise the patch looks good to me
Sep 7 2021
Sep 6 2021
I quite like the way it looks with the code moved from the vectorizer to loop access analysis. I'll do a more in-depth review after the unit tests are fixed, but i left a couple of simple comments for now.
Aug 24 2021
Thanks Malhar. Code looks good to me. I'll need to take another look at the unit tests.
I'm going away for a week, so either someone else picks up the review in the meantime, or we can resume working on this when I come back.
Aug 23 2021
Thanks for the cleanup on the unit tests. It's clearer, it helps to start digging into the details of what they do.
Aug 20 2021
Thank you for the clarifications!
I see now that the llvm.loop.parallel_accesses metadata can't be used in the way that we wanted. We have also come back to the original IR and found that there was a more appropriate way to tackle our issue. So I'll close this review. I'll also add a bit more information about our case below, in case you're interested.
Aug 18 2021
Could I have some more information with regards to the proposed changes?
Thanks for the review. I started looking at the metadata definition and previous messages from the mailing list. It seems you're right, and the metadata doesn't give the information i thought it did.
I'd love to get some confirmation, if someone knows for sure that this is an abuse of this particular metadata.
I'll come back to the IR of the original test case, and see if there some other information i should have used.
The build error seems unrelated to the this patch. There is another build next to mine, for a different patch, with the same unit test failing.