andwar added inline comments.
================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7458
+ llvm_unreachable("Invalid SVETypeFlag!");
+
+ case SVETypeFlags::EltTyInt8:
----------------
SjoerdMeijer wrote:
> nit: no need for the newlines here and also below?
IMHO this improves readability, but I agree that that's a matter of personal
preference :)
I'll keep it as is for the sake of consistency with `getSVEType` 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.
----------------
SjoerdMeijer wrote:
> 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.
>
Good point, updated!
================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7601
+ llvm::Type *SrcDataTy = getSVEType(TypeFlags);
+ llvm::Type *OverloadedTy = llvm::VectorType::get(
+ SVEBuiltinMemEltTy(TypeFlags), SrcDataTy->getVectorElementCount());
----------------
sdesmalen wrote:
> nit: This can use `auto`, which would make OverloadedTy a `llvm::VectorType`
Thanks, good point! I also updated `getSVEType` to return `VectorType` (so I
can use `auto` for both).
================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7602
+ llvm::Type *OverloadedTy = llvm::VectorType::get(
+ SVEBuiltinMemEltTy(TypeFlags), SrcDataTy->getVectorElementCount());
+
----------------
sdesmalen wrote:
> Just be aware that `getVectorElementCount` has been removed from Type in
> D77278, so you'll need to cast to `VectorType` and use `getElementCount`
> instead.
> (fyi - in D77596 I've made `getSVEType` actually return a VectorType so that
> cast wouldn't be needed)
I have done the same ;)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77735/new/
https://reviews.llvm.org/D77735
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
