This patch adds intrinsic definitions and SDNodes for predicated load/store/gather/scatter, based on the work done in D57504.
Thanks Hussain for this patch!
For now I have a couple of minor comments, I will take a deeper look later.
Also, I don't quite understand yet how VP handles (or plans to handle) specifying alignment parameter. Does it need VPBuilder (not to be confused with VPlan VPBuilder!) for that? Would it make sense to add the Alignment parameter to VP load/store/scatter/gather intrinsics (much like their masked counterparts) for now, unless there is already a way to specify that?
Better to use DefaultAttrsIntrinsic? (Ditto for other load, gather and scatter)
minor style nit: A blank line above TODO. Perhaps reposition it to above the scope where it applies.
We should use the align(N) attribute on pointers to convey the alignment. That's what we do in the LLVM for SX-Aurora stack (https://github.com/sx-aurora-dev/llvm-project/blob/683f41073689262a8a493cce7caba13bbccea56c/llvm/test/CodeGen/VE/Packed/vp_load_packed.ll#L80) . I'd say that the alignment parameter in the pre-existing masked.load/store simply predates that attribute.
And yes, setting alignment is something that the VPBuilder or what's-it-called-after-bikeshedding would take care of for you.
Can we override clang-format here? The previous code had a more consistent structure.
It kind of feels like maybe there should be a VPMemIntrinsic and a VPStoreIntrinsic subclasses that contain these methods? Then you can use the more common isa, cast, dyn_cast and not have Optional return values.
Doesn't the getValue() below already assert if there is no value?
Just some minor comments. I think Craig's suggestion of mem subclasses sounds like it's worth investigating.
Alternatively, I suppose, it could be a switch statement (aside from the last two)? Presumably that'd be optimized well enough by compilers?
Inconsistent documentation style, e.g. return the and return The, and use of commas vs forward slashes.
Also, is clang-tidy correct here? No uppercase letter on static methods?
How some the loads are IntrReadMem but the stores aren't IntrWriteMem?
typo: cannonical -> canonical
Purely a matter of style but this is slightly shorter and (I think) a little clearer:
if (auto PtrParamOpt = GetMemoryPointerParamPos(getIntrinsicID())) return getArgOperand(PtrParamOpt.getValue()); return nullptr;