[PATCH] D78756: [SveEmitter] Add builtins for svreinterpret

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7880
+    return Builder.CreateBitCast(Val, Ty);
+  }
+
----------------
sdesmalen wrote:
> efriedma wrote:
> > I'm vaguely suspicious this might be wrong for big-endian targets.  I mean, this isn't unreasonable, but users might be surprised if svreinterpret isn't a no-op.
> For SVE the loads and stores (svld1 and svst1) are all endian safe, so no special consideration needs to be taken for big endian targets.
> 
> The ACLE specifies that:
> > The svreinterpret functions simply reinterpret a vector of one type as a vector of another type, without changing any of the bits.
"bitcast" is specified to mean "reinterpret the bits like a store+load".  On big-endian NEON (and, I assume, SVE), that isn't a no-op.  See http://llvm.org/docs/BigEndianNEON.html .

I mean, if the answer here is "yes, svreinterpret is supposed to lower to a REV", then that's fine.  But I'd like to see some explciit acknowledgement that that's intentional.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78756