[PATCH] D98856: Always emit error for wrong interfaces to scalable vectors, unless cmdline flag is passed.

[Paul Walker via Phabricator via cfe-commits]

paulwalker-arm added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5052-5055
+  if (isa<BackendJobAction>(JA)) {
+    CmdArgs.push_back("-mllvm");
+    CmdArgs.push_back("-treat-scalable-fixed-error-as-warning");
+  }
----------------
Are there any concerns related to LTO here? Could we live with LTO triggering errors for the invalid uses?  Part of me thinks this is reasonable given the clang exception is more about ensuring we can continue active development until we're ready to press the "it's supported" switch.


================
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
----------------
Why is this need here? Can it just be externs where it's accessed?


================
Comment at: llvm/lib/CodeGen/ValueTypes.cpp:28
+  if (isScalableVector()) {
+    if (llvm::TypeSizeClOpt::ScalableErrorAsWarning)
+      WithColor::warning()
----------------
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()
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98856