[PATCH] D67572: [SVFS] The Search Vector Function System.
Johannes Doerfert via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 6 12:03:25 PST 2019
jdoerfert added inline comments.
================
Comment at: llvm/include/llvm/Analysis/TargetLibraryInfo.h:201
+
+ void fixUpVFABINames(CallInst *CI) const;
};
----------------
Documentation missing, and please do not call it "fixup", maybe "adjust" or something but not "fixup".
================
Comment at: llvm/include/llvm/Analysis/TargetLibraryInfo.h:266
return Impl->getVectorizedFunction(F, VF);
}
----------------
Why do you privatize these redirects? I would expect them to be used, thus public, or unused, thus deleted.
================
Comment at: llvm/include/llvm/Analysis/VectorUtils.h:58
+#define _LLVM_INTERNAL_TLI_ "_LLVM_INTERNAL_TLI_"
+
/// Encapsulates information needed to describe a parameter.
----------------
I generally dislike non-scoped macros. Is there a reason a `static constexpr char*` doesn't work?
I would also opt for a shorter name, "_llvm_" or "_llvm_tli_" but that is debatable.
================
Comment at: llvm/include/llvm/Analysis/VectorUtils.h:132
/// _ZGV<isa><mask><vlen><parameters>_<scalarname>[(<redirection>)].
-Optional<VFInfo> tryDemangleForVFABI(StringRef MangledName);
+Optional<VFInfo> tryDemangleForVFABI(const std::string Input);
----------------
I doubt this change is necessary. Only if you need to own the underlying data you need to replace StringRef with std::string.
================
Comment at: llvm/include/llvm/Analysis/VectorUtils.h:142
+void setVectorVariantNames(CallInst *CI,
+ const SmallSet<std::string, 8> VariantMappings);
+
----------------
I'd prefer
`void getVectorVariantNames(CallInst *CI, SmallSet<std::string, 8> &VariantMappings);`
but for sure you want a reference here:
```
void setVectorVariantNames(CallInst *CI,
const SmallSet<std::string, 8> &VariantMappings);
```
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67572/new/
https://reviews.llvm.org/D67572
More information about the llvm-commits
mailing list