[PATCH] D77735: [SveEmitter] Implement builtins for gathers/scatters

Sjoerd Meijer via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Apr 20 06:25:32 PDT 2020


SjoerdMeijer accepted this revision.
SjoerdMeijer added a reviewer: efriedma.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.

This is a big patch, but looks reasonable to me.



================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7451
+  }
+  llvm_unreachable("Unknown MemEltType");
+}
----------------
nit: to be consistent, do this in the default clause?


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7458
+    llvm_unreachable("Invalid SVETypeFlag!");
+
+  case SVETypeFlags::EltTyInt8:
----------------
nit: no need for the newlines here and also below?


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7555
+
+  // From ACLE we always get <n x 16 x i1>. This might be incompatible with
+  // the actual type being loaded. Cast accordingly.
----------------
nit: can you rephrase this comment a bit I.e. the "From ACLE we always get ..." is a bit confusing I think. You want to say that this is how the ACLE defines this, but the IR looks different. You can also specify which bit is different, because that was not immediately obvious to me. 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77735





More information about the cfe-commits mailing list