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:
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.