[PATCH] D134168: [RISCV] Make preferred alignment of PointerArgs for MemIntrinsic

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:12986
+    return false;
+  MinSize = 8;
+  PrefAlign = Align(Subtarget.getXLen() / 8);
----------------
jrtc27 wrote:
> JojoR wrote:
> > jrtc27 wrote:
> > > This seems arbitrary. I'd expect at least XLEN / 8 here?
> > The caller has check this condition, set PrefAlign only the original is more smaller :)
> I don't understand this comment at all. This is about MinSize not PrefAlign, and CGP just uses this value as a threshold. Why 8? Why not XLEN / 8, or XLEN / 4 if the 8 was chosen to optimise for RV32. This needs justification.
having the same value for rv32 and rv64 kind of makes sense from a certain perspective. If the object is smaller than minsize then you're saying you're ok with "object size in bytes" number of stores if the object is align 1. What you want to allow doesn't necessarily vary with xlen.


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

https://reviews.llvm.org/D134168