[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