[PATCH] D134087: [TableGen] Track reference locations of Records/RecordVals

Sat Sep 24 20:01:29 PDT 2022

jpienaar accepted this revision.
jpienaar added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: zero9178.

In D134087#3804630 <https://reviews.llvm.org/D134087#3804630>, @lattner wrote:

>> The problem that I ran into was that a lot of stuff kind of requires that the Inits get folded and are uniqued, which would no longer be the case if we put locations on them. From there this patch arose, because I didn't want to broach the "rewrite the world" just to get better locations.
>
> Aha, I didn't remember that, yes I agree that you cannot unique things if they carry locations.  If that is inherent to how tblgen works, then ok.

Would it be possible to do something like unique/have equivalence set representative have union of all locations and unique excluding location?

I haven't looked at innards to see if the above possible, this change doesn't seem too invasive and the results are very useful to produce better error messages too. This is already an improvement in location tracking looking at the changed test results, so LG from me.

================
Comment at: llvm/lib/TableGen/TGParser.cpp:2538
+          auto RecClasses = RecTy->getClasses();
+          if (RecClasses.size() == 1) {
+            if (auto *RV = RecClasses[0]->getValue(FieldName))
----------------
Why only if singleton?

================
Comment at: llvm/lib/TableGen/TGParser.h:203
                 ArrayRef<unsigned> BitList, Init *V,
-                bool AllowSelfAssignment = false);
+                bool AllowSelfAssignment = false, bool OverrideDefLoc = true);
   bool AddSubClass(Record *Rec, SubClassReference &SubClass);
----------------
What does OverrideDefLoc mean?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134087