- User Since
- Jan 23 2017, 12:28 AM (209 w, 2 d)
Tue, Jan 19
Tue, Jan 12
NFC. Usage of Optional. Rebased onto committed D93709 .
Mon, Jan 11
Title typo: 'isntert'
Fri, Jan 8
NEC. Rebased. Use variable name Opcode instead of OC.
Can we accept this patch? In the end, it is a strict improvement over the existing one.. and it is "just" a unittest..
This is one of those patches that has the potential to linger on without review for 1Y+ .. let me know if there is anything i can do to get this reviewed, as it really helps with bringing good vector architecture (long SIMD) support to LLVM. Thanks.
We want somebody that has reviewed VP patches before to review the VP bits of this (getters, really). @kaz7 takes care of the VE parts. Thanks!
This now depends on D93687 - the build_vector lowering causes introduces more instances of insert_vector_elt.
- Rebased onto main.
- insert_vector_elt tests trigger vbrd code path because lowering of build_vector to insert_vector_elt is gone (D93759).
Tue, Jan 5
|& -> 2>&1 |
- Hopefully attached signext to all args in tests this time
- Adressed comments
You might want to fix the typo in the title before committing this.
- Use bool4 in docs.
- Use signext for arguments in tests.
- No more than 80 chars in VEInstrPatternVec.td file.
"Stuff" is unspecific. How about we go by "[VE] SJLJ isel" or something along those lines for the title?
Mon, Jan 4
- NFC. Rebased.
- Fixed custom lowering code for packed vectors and updated tests.
- Removed unrelated BUILD_VECTOR custom lowering to insert/extract elt.
NFC. Use OC variable name consistently.
There should be test for this. You could use:
Dec 23 2020
FYI i have tested this against check-all with clang and all (including experimental) targets enabled.
In case you were wondering, the ISD helper functions are stapled to this VE patch to make a nice testable unit.
Done. Please base your patches on the upstream main branch in the future. That makes it much easier to commit patches with Arcanist.
NFC. Removed stray comment.
Dec 22 2020
Dec 21 2020
Thanks! Do you need me to commit this on your behalf?
That's why i am proposing to initially use these bundles only for the VP intrinsics and keep the constrained fp intrinsics in place. When we are sure that all optimizations use the proper abstractions, we can move to replace constrained fp intrinsics with the bundled version instead.
Dec 18 2020
Added VPModuleComplete - test that all registered VP intrinsics are declared in the testing module.
Ping. Where do we stand with this patch, can we LGTM or are there comments?
Probably. With this patch standalone, we can only do "catch-22 testing": all uses of the bundle are illegal. We can extend that once the VP floating-point intrinsics are upstream that actually support these bundles.
Dec 17 2020
- Fixed a failing tests (there is now two more op bundles)
nfc, comment cleanup.
- NFC. Fixed spelling, typos.
Dec 15 2020
Dec 14 2020
Thanks for splitting this off!
LGTM with nits.
Dec 11 2020
Just this one thing, else, LGTM.
LGTM. Can we upstream the script that generates the intrinsic declarations? We don't need to run it in the build but it should be upstream so people can re-generate and understand the intrinsics.
Dec 10 2020
AFAIK all comments were addressed. Could you have another look at/LGTM this?
Dec 9 2020
Thanks! One nit, else good to go.