Details
- Reviewers
sdesmalen efriedma - Commits
- rGfebeaf94a806: [llvm][SVE] IR intrinsic for LD1RO.
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Event Timeline
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp | ||
---|---|---|
11901 | Note: I am tempted to merge performLD1ROCombine and performLD1RQCombine in a single function, as they only differ in the ISD node being used to replace the intrinsic. Let me know if this makes sense, I'll update the patch. |
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp | ||
---|---|---|
11901 | Yes, that makes sense. |
Some nits, but LGTM otherwise.
llvm/include/llvm/IR/IntrinsicsAArch64.td | ||
---|---|---|
1308 | super nit: move this under ld1rq? | |
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp | ||
1461 | super nit: same suggestions as above. | |
llvm/lib/Target/AArch64/AArch64ISelLowering.h | ||
251 | super nit: move under ld1rq. | |
llvm/test/CodeGen/AArch64/sve-intrinsics-ld1ro.ll | ||
4 | perhaps somewhat unrelated, but I see that llvm/test/CodeGen/AArch64/sve-intrinsics-loads.ll contains the ld1rq tests. (Oddly enough that file also contains tests for ldnt1, but nothing else) |
Remove the bfloat test case, as bfloat support will land
separately. This brings the patch at the same state when it was
approved, so there is no need to reapprove it, I will just submit it.
llvm/test/CodeGen/AArch64/sve-intrinsics-ld1ro.ll | ||
---|---|---|
4 | I'd rather keep the ld1ro and ld1rq tests separate. I see more value in having more smaller test files than fewer big tests files, it make it easier to me to isolate implementation, reviews and possible failures. |
super nit: move this under ld1rq?