[PATCH] D31951: TableGen support for parametrized register class information

[Krzysztof Parzyszek via Phabricator via llvm-commits]

kparzysz added a comment.

In https://reviews.llvm.org/D31951#731960, @asb wrote:

> One question I'd welcome a second opinion on - is the introduction of the `vi::` namespace justified?


Justified?  Good question.  I added some common functions at the top level in VariableValueType.h, and I didn't want to pollute the global name space with them, or at least wanted to make it clearer what these functions are related to.  I'm not convinced that this is an elegant solution, and I'm open to other suggestions.

Thank you for taking time to review all these patches!



================
Comment at: utils/TableGen/CodeGenDAGPatterns.cpp:495
+      if (B.empty()) {
+        TP.error("empty2a");
+        return Changed;
----------------
asb wrote:
> Is it worth having a more meaningful error message for these? Maybe "Set unexpectedly empty (ref 2a)"?
Certainly.  I will change it to something better, or at least less cryptic.  Same for the other cases in this function.

As a part of changing the type inference, I planned on generating meaningful error messages (with optional traces of what steps have been taken), but that would require more refactoring that was not directly related to the goals of this patch.  For this work I didn't spend too much time on the error reporting, as illustrated by this example.


Repository:
  rL LLVM

https://reviews.llvm.org/D31951