- User Since
- Mar 4 2023, 1:11 AM (12 w, 6 d)
May 2 2023
Together with my observation in D148316 and the new tests, this looks good.
Apr 28 2023
Apr 27 2023
@dmgreen Thanks for your review. Could you please merge this with "Lawrence Benson <email@example.com>".
Apr 26 2023
Added check for illegal types. I could not get this check to actually fire. The type is legalized before the bitcast in the bitcast lowering case, and it is legalized before the
store becomes a truncating store. So in both cases other checks prevent bad things from happening. But it is probably still fine to keep it as a defensive check in case things
Apr 22 2023
Address a few review comments.
Apr 19 2023
I dug into this a bit, and your vector compare sing bit patch does the right thing here. But the problem is unrelated and a bit annoying. At the time the sign_extend_inreg that we added is combined (to potentially remove it), there is a build_vector with (1 << vectorElementSize) - 1 bits (to negate via xor). But ComputeNumSignBits breaks here. The vector has 32-bit constants but we only have 8-bit vectors. So it detects 24 sign bits and then kinda gives up. So without changing code in SelectionDAG::ComputeNumSignBits or the negation of vector entries, there is no way to correctly determine the sign bits.
Apr 18 2023
Apr 14 2023
Apr 12 2023
Add test for float vector. This required a single-line change to convert the VecVT to an integer vector for sign-extend to work.
Apr 11 2023
Thanks for your effort in reviewing this patch. I think this solution is nicer than my original approach.
Apr 10 2023
Changed approach as suggested by @dmgreen. We now use an explicit sign-extend and ignore the vector compare. The sign-extend is removed in later steps if there is a vector compare,
so there is no overhead. This change allows us to determine the original type in more cases, as we can detect both SETCC and TRUNC.
Apr 6 2023
Addressed some review comments.
Apr 3 2023
@dmgreen I can split this into two patches. I'll remove the truncate store part and only focus on the bitcast for now.
Mar 31 2023
Changed large parts of where this conversion takes place.
Mar 29 2023
Thanks. Could you please use "Lawrence Benson <firstname.lastname@example.org>".
@fhahn is there anything missing from your side? If not, could one of you or @dmgreen merge this? As this is my first patch, I obviously do not have push access :)
Mar 25 2023
@Sp00ph Your example would not be optimized. The issue with that example is: how is a bitcast to i1 defined? The current logic in LLVM uses the least significant bit. But this trick does not work in that case, as we use bits 0 to n for lanes 0 to n, so we only use the least significant one for lane 0. If we have a comparison, we know that all bits are 1 or all bits are 0, so the least significant one is equal to all others. Without a comparison, we could shift the least significant bit and then do the rest, but that would need an extra instruction. Maybe this could be added in a follow-up? I'm happy to discuss options here. The current approach is a bit more conservative.
Mar 24 2023
@david-arm @dmgreen I rebased this into a single commit. I hope the changes are shown correctly now. All changes here are new, I did not modify any tests. I guess this was just shown incorrectly because I used multiple commits.
Rebase onto main
An honest attempt to do a rebase with arc :)
Mar 22 2023
@dmgreen Okay, I rebased onto main and submited via arc. I hope this did the right thing, but it doesn't look broken. I guess my mistake in the other patch was to create a merge commit instead of rebasing. For future reference, I'll just always squash into in commit.
Mar 21 2023
@dmgreen Thanks for the feedback. I've addressed the comments you made. I've renamed the method, as it does not require an AND anymore.
Add a path without AND mask.
Apply the bitcast to truncating stores.
Mar 17 2023
Add tests for exit cases and extend to v8i8.
Sorry, this was meant to be an update to: https://reviews.llvm.org/D146212, not a new diff.
Mar 16 2023
@t.p.northover As this is my first patch submitted to LLVM, this is just a short ping to check if there is something that I have missed or forgotten to do. I'm not yet familiar with the procedure.