[PATCH] D128144: [AArch64] Known bits for AArch64ISD::DUP

Mon Jun 20 10:16:52 PDT 2022

dmgreen added a comment.

> A while back I tried this and there were only a few places that relied on DAGCombine not messing with AArch64ISD::DUP in order to emit good code. I didn't push on with it as I was unsure how others would feel. Given the statement above I think I'll dig it because the easy first step is to restrict AArch64ISD::DUP to only fixed length vectors. Please let me know if you'd rather me hold off.

Yeah - that does sound like a good incremental step.

================
Comment at: llvm/include/llvm/CodeGen/TargetLowering.h:3790
+  /// target, which should not be transformed back into a BUILD_VECTOR.
+  virtual bool isTargetCanonicalConstantNode(unsigned Opc, EVT VT) const {
+    return Opc == ISD::SPLAT_VECTOR;
----------------
RKSimon wrote:
> After some testing to reuse this on X86 we might need to change this to take a SDValue Op instead.
> 
> X86 shares broadcasted constants across different vector widths (i.e. we broadcast to v8i32 and reuse it for v4i32 as well because we can freely access the bottom subvector) - which means we still have infinite loops from extract_subvector(broadcast_load(constant_pool), 0) patterns.
> 
> Not sure if any other targets does anything similar? If not we can keep this as it is for now and I might need to tweak it when x86 support gets added.
SDValue sounds good to me - the EVT was always unused anyway and was just added in case it was useful. An SDValue sounds more general than that. I'll make that change as I commit it, feel free to adjust as you need in the future.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128144/new/

https://reviews.llvm.org/D128144