InstCombine AArch64 LD1/ST1 to llvm.masked.load/llvm.masked.store and LD1/ST1 to load/store when a ptrue all predicate pattern operand is present.
This allows existing IR optimizations such as dead-load removal to occur.
Paths
| Differential D113489
[AArch64][SVE] Instcombine SVE LD1/ST1 to stock LLVM IR ClosedPublic Authored by MattDevereau on Nov 9 2021, 8:06 AM.
Details Summary InstCombine AArch64 LD1/ST1 to llvm.masked.load/llvm.masked.store and LD1/ST1 to load/store when a ptrue all predicate pattern operand is present. This allows existing IR optimizations such as dead-load removal to occur.
Diff Detail
Event Timeline
Comment Actions I've had a brief look, some typographical comments. For some reason the patch has failed to apply and the tests have failed to run in CI, perhaps you could try a rebase and rerun arc diff to see if that fixes it?
MattDevereau marked 7 inline comments as done. Comment ActionsAdd CreateMaskedLoad passthru parameter
Comment Actions Regenerated broken tests MattDevereau added inline comments.
Comment Actions It looks like some auto-generation lines were added in 91eda9c30f33da6ec6da70b59a5f5da6c6397039, but the tests using CHECK-DAG were not actually auto-generated. It seems we could either cleanly auto-generate them, or we should copy the existing CHECK-DAG patterns. It seems that what is currently committed is a little wrong, if only by having a 'these tests are auto-generated' header even though they are not auto-generated. Comment Actions My current thinking is that the route forward is to update the tests without update test checks, which affects only the st1{b,h,w} files, and possibly to remove the false header implying these files were autogenerated. Comment Actions There's an issue with the a Value* being named VecPtrTy but otherwise this looks good to me. I'll leave it up to you to decide whether it's worth breaking out the usage of update_cc_test_checks.py into a separate patch. Normally this is a good thing but given the patch is ready to go and I imagine you wouldn't submit the "use update_cc_test_checks.py" patch for review I'm not sure it's worth the effort.
This revision is now accepted and ready to land.Nov 15 2021, 4:22 AM Comment Actions Renamed VecPtrTy to VecPtr in both instCombineSVEST1 and instCombineSVELD1 Closed by commit rGf526c600c043: [AArch64][SVE] Instcombine SVE LD1/ST1 to stock LLVM IR (authored by MattDevereau). · Explain WhyNov 16 2021, 3:11 AM This revision was automatically updated to reflect the committed changes. Comment Actions This caused buildbot failures which failed on the bfloat tests. Pushed commit 83727f27719d3f319f746b473ce09be7e1d99b32 to fix the issue
Revision Contents
Diff 387549 clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ld1-bfloat.c
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ld1.c
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ld1sb.c
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ld1sh.c
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ld1sw.c
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ld1ub.c
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ld1uh.c
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ld1uw.c
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_st1-bfloat.c
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_st1.c
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_st1b.c
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_st1h.c
clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_st1w.c
llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp
llvm/test/Transforms/InstCombine/AArch64/sve-intrinsic-loadstore.ll
|
Unfortunately it looks like these were not autogenerated, they used to use CHECK-DAG. This results in more changes than we expect. My thinking is that these tests are intended to be autogenerated, so we should make a clean patch which autogenerates the tests and then reapply your work on top of that.