ctetreau added inline comments.
================
Comment at: llvm/include/llvm/Support/TypeSize.h:31-35
+namespace TypeSizeClOpt {
+/// The ScalableErrorAsWarning is a temporary measure to suppress errors from
+/// using the wrong interface.
+extern cl::opt<bool> ScalableErrorAsWarning;
+} // namespace TypeSizeClOpt
----------------
paulwalker-arm wrote:
> Why is this need here? Can it just be externs where it's accessed?
It could, but that would be annoying. It would be nice if you could just
include the header and have this symbol in scope.
Whether or not it should be convenient to use this option is a valid question
though. I'm fine either way.
================
Comment at: llvm/lib/CodeGen/ValueTypes.cpp:28
+ if (isScalableVector()) {
+ if (llvm::TypeSizeClOpt::ScalableErrorAsWarning)
+ WithColor::warning()
----------------
paulwalker-arm wrote:
> I guess related to my comment for TypeSize.h but I'm wondering if it's better
> to move all error reporting into TypeSize.cpp. For example:
> ```
> if (isScalableVector())
> reportInvalidSizeRequest("EVT::getVectorNumElements()",
> "EVT::getVectorElementCount()")
>
> reportInvalidSizeRequest(string bad_func, string good_fun) {
> #ifndef STRICT_FIXED_SIZE_VECTORS
> if (ScalableErrorAsWarning)
> warning(don't use badfunc, use good_fun instead)
> else
> #endif
> Error()
> }
> ```
I guess if we did this, then the definitions for EVT::getVectorNumElements and
the TypeSize cast to unsigned to stay in the header. Then, if
STRICT_FIXED_SIZE_VECTORS is defined, it would be easy to inline the function
body as it's all in the header, resulting in a real "pro" to defining
STRICT_FIXED_SIZE_VECTORS and having your code work with it. I'm fine either
way.
It's only two functions, and unlikely to increase to more, so you could make
the case that it's not worth bothering over.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D98856/new/
https://reviews.llvm.org/D98856
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
