The patch covers both register/register and register/immediate addressing modes.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
- Build Status
Buildable 46453 Build 48921: arc lint + arc unit
Event Timeline
The patch now is ready to review. It covers only the contiguous prefetches. I will create a separate revision for the gather prefetches.
llvm/include/llvm/IR/IntrinsicsAArch64.td | ||
---|---|---|
1238–1240 | Ideally, I would have liked to define this as something like: def int_aarch64_sve_prf : Intrinsic<[], [PointerTo<llvm_anyvector_ty>, LLVMScalarOrSameVectorWidth<0, llvm_i1_ty>, llvm_i32_ty], [IntrArgMemOnly]>; For some definition of PointerTo, but I am not aware of anything in the current intrinsic generator that can be used for that. |
nit: in the title:
s/continuous/contiguous/
LGTM otherwise
llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp | ||
---|---|---|
4414 ↗ | (On Diff #245526) | nit: Any reason you're using an anonymous namespace over static for these functions? |
4424 ↗ | (On Diff #245526) | nit: you can fold this into the condition above. |
4432 ↗ | (On Diff #245526) | instead of 128, can you use AArch64::SVEBitsPerBlock? |
llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp | ||
---|---|---|
4414 ↗ | (On Diff #245526) | I should have really used static to adhere to the style of the rest of the file... Done. |
llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp | ||
---|---|---|
4443 ↗ | (On Diff #245961) | [nit] corresponding? |
4491 ↗ | (On Diff #245961) | What's this check for? Wouldn't some hard-coded null-value be more idiomatic? |
llvm/test/CodeGen/AArch64/sve-intrinsics-contiguous-prefetches.ll | ||
100 ↗ | (On Diff #245961) | Would it make sense to add checks for what's expected instead? |
llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp | ||
---|---|---|
4491 ↗ | (On Diff #245961) | I think EVT() is there to simulate a nullptr, see http://llvm.org/doxygen/ValueTypes_8h_source.html: struct EVT { private: MVT V = MVT::INVALID_SIMPLE_VALUE_TYPE; Type *LLVMTy = nullptr; public: constexpr EVT() = default; Essentially, if statement checks that the MemVT is not { MVT::INVALID_SIMPLE_VALUE_TYPE, nullptr}. |
llvm/test/CodeGen/AArch64/sve-intrinsics-contiguous-prefetches.ll | ||
100 ↗ | (On Diff #245961) | I guess you are right. We could miss a failure in the addressing mode generation if we have (at the same time) a failure in the rendering of the immediate constant... quite a remote possibility, which I am sure it will be caught in time in some other tests. So I think we can leave it as it is. |
Ideally, I would have liked to define this as something like:
For some definition of PointerTo, but I am not aware of anything in the current intrinsic generator that can be used for that.